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

Add SDF HintingMode #1966

Merged
merged 11 commits into from
Jun 23, 2024
Merged

Add SDF HintingMode #1966

merged 11 commits into from
Jun 23, 2024

Conversation

Labrium
Copy link
Contributor

@Labrium Labrium commented Sep 2, 2023

I was having trouble building this with the prebuilt FreeType macOS framework, I'm not sure why, so I had to build it myself to make sure this built correctly.

@slime73
Copy link
Member

slime73 commented Sep 10, 2023

I tried to apply and use this change on Windows with the default font (love.graphics.newFont(14, "sdf")) but I get this error at runtime:

main.lua:1: TrueType Font glyph error: FT_Glyph_To_Bitmap failed (0x13)

0x13 is FT_Err_Cannot_Render_Glyph. The version of FreeType in megasource's 12.x branch is 2.12.0. Does it need a newer version than that to work, or is there some other problem?

@Labrium
Copy link
Contributor Author

Labrium commented Sep 11, 2023

That's the main problem I was having with the prebuilt love-apple-dependencies on macOS. After building it myself from the official FreeType 2.12.0 source instead of from the megasource, it worked with no errors.

From FreeType's changelog they state that SDF support is available since 2.11.0, is there a reason the megasource's FreeType could be different from the official source?

@slime73
Copy link
Member

slime73 commented Sep 11, 2023

It's the same source code. Maybe it's an opt-in compile time feature when building the library? If that's the case and it's not enabled by default in Linux distros, this gets a lot more complicated to support because we can't guarantee that it's available in every copy of a given love version.

Speaking of Linux distros, I think Ubuntu 20.04 LTS may not provide 2.11+.

@Labrium
Copy link
Contributor Author

Labrium commented Sep 11, 2023

I've investigated it further, and it looks like the FT_Err_Cannot_Render_Glyph error is being caused by this commit.

Commenting out lines 303-309 in src/sdf/ftsdfrend.c makes it work correctly (restoring the same functionality as 2.11.0):

    /* the rows and pitch must be valid after presetting the */
    /* bitmap using outline                                  */
    //if ( !bitmap->rows || !bitmap->pitch )
    //{
    //  FT_ERROR(( "ft_sdf_render: failed to preset bitmap\n" ));

    //  error = FT_THROW( Cannot_Render_Glyph );
    //  goto Exit;
    //}

Would it be possible and/or favorable to modify the FreeType source within the megasource (assuming this small change does not cause any other problems)?

Also, I don't see how supporting Ubuntu 20.04 is relevant to this issue since the 12.x branch of the megasource already includes FreeType 2.12.0.

@slime73
Copy link
Member

slime73 commented Sep 11, 2023

Commenting out lines 303-309 in src/sdf/ftsdfrend.c

Yeah that'd do it, love creates a glyph of the space character in Font constructors. It might be possible to modify TrueTypeRasterizer.cpp so it detects glyphs which would have invisible/zero-size bitmaps and has an alternate codepath that doesn't try rasterizing those. Or sdf support could only be turned on at runtime when a newer version of freetype with that fix is used - although I'm hesitant about doing that because of the versions used in package managers.

Would it be possible and/or favorable to modify the FreeType source within the megasource (assuming this small change does not cause any other problems)?

I don't think that's a good idea, it adds a burden for us to maintain the patch and it would only apply to one platform out of 5 (see below).

Also, I don't see how supporting Ubuntu 20.04 is relevant to this issue since the 12.x branch of the megasource already includes FreeType 2.12.0.

As I said (and is said in the readmes), megasource is primarily for Windows. It's not used for official builds on other platforms. Linux AppImages use a different build process, and Linux distro-maintained love packages use shared system dependencies which are not under our control at all.

Copy link
Member

@slime73 slime73 left a comment

Choose a reason for hiding this comment

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

It's working for me in Windows now, thanks!

I think the hinting stuff still needs to be figured out, but aside from that it looks good. It's too bad Freetype doesn't have an option for a more advanced SDF technique since single-value SDFs can have some noticeable visual issues around corners of letters.

src/modules/font/freetype/TrueTypeRasterizer.cpp Outdated Show resolved Hide resolved
@slime73
Copy link
Member

slime73 commented Feb 10, 2024

The 12.0-development branch has been merged into main now, you'll need to change the base branch of this PR to main as well.

@slime73
Copy link
Member

slime73 commented Jun 16, 2024

Any update on this? I can take it over if needed.

@slime73 slime73 changed the base branch from 12.0-development to main June 22, 2024 19:55
@Labrium
Copy link
Contributor Author

Labrium commented Jun 22, 2024

Sorry for the long delay, I forgot this wasn't finished. I can take care of the rest of the changes.

@Labrium
Copy link
Contributor Author

Labrium commented Jun 22, 2024

It looks like mono hinting and sdf options cannot be used together since they use different rendermodes. If this is a problem, I can do further testing to see what can be done about it.

@slime73
Copy link
Member

slime73 commented Jun 22, 2024

It looks like mono hinting and sdf options cannot be used together since they use different rendermodes.

I think that makes sense, you could make it cause an error (via love::Exception) if both are set in the constructor.

@Labrium
Copy link
Contributor Author

Labrium commented Jun 23, 2024

It looks like there was no error needed to fix the rendermode problem.

Copy link
Member

@slime73 slime73 left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@slime73 slime73 merged commit 2e1e48b into love2d:main Jun 23, 2024
7 checks passed
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