Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed Adafruit_GFX font size limitation - the 8-bit signed values prevented large fonts from working #448

Closed
wants to merge 2 commits into from

Conversation

bitbank2
Copy link
Contributor

This is a simple change which allows the fontconverter and Adafruit GFX font logic to work with fonts larger than about 90 pt. The original code used signed 8-bit values for the character offsets and yAdvance. These overflow when working with fonts past about 90 points. There is no downside to this fix; adding a few bytes to the size of the GFXglyph structure won't negatively impact anything.

@bitbank2
Copy link
Contributor Author

No idea what the problem is because it just shows "error 1". Useless.

@BillyDonahue
Copy link
Contributor

BillyDonahue commented Jun 6, 2024

adding a few bytes to the size of the GFXglyph structure won't negatively impact anything.

I disagree with that. Let's say there are 100 glyphs in a font.

This change adds 5 bytes per glyph, so 500 bytes to that font, even for apps that will never work with huge fonts.
The Atmega328 target only has 32kB total PROGMEM space.

People pack their apps pretty tightly into that 32kB PROGMEM space. If they have a few fonts the cost will be multiplied again.

We may disagree as to whether a 2-ish % overhead is significant, but it's not correct to flatly state that there's no downside.
It's a tradeoff.

Comment on lines +4 to +5
CFLAGS = -Wall -I/usr/local/include/freetype2 -I/usr/include/freetype2 -I/usr/include -I/opt/homebrew/include/freetype2
LIBS = -L/opt/homebrew/lib -lfreetype
Copy link
Contributor

@BillyDonahue BillyDonahue Jun 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These paths apply to your machine.

The right way to figure out where freetype2 is, would be to have the Makefile invoke and save the output of:
freetype-config --libs and freetype-config --cflags.

On my machine, I get different answers.

$ freetype-config --libs
-L/opt/homebrew/opt/freetype/lib -lfreetype
$ freetype-config --cflags
-I/opt/homebrew/opt/freetype/include/freetype2 -I/opt/homebrew/opt/libpng/include/libpng16

But Makefile tweaks are a different PR I think.

@BillyDonahue
Copy link
Contributor

BillyDonahue commented Jun 6, 2024

No idea what the problem is because it just shows "error 1". Useless.

The details are in the failure report:

https://github.com/adafruit/Adafruit-GFX-Library/actions/runs/9206826415/job/25325537261?pr=448

Your code needs be in a state that running clang-format on it would produce no changes.

Widening the types pushed the // comments over to the right by 1 character, and clang-format adjusts the position of // comments so that the // on adjacent source lines form a straight vertical line.

@bitbank2 bitbank2 closed this Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants