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

Assert occurs when imgui_freetype is used. #5829

Closed
wants to merge 2 commits into from
Closed

Assert occurs when imgui_freetype is used. #5829

wants to merge 2 commits into from

Conversation

nkari82
Copy link

@nkari82 nkari82 commented Oct 28, 2022

Skip with spacebar (codepoint 32) or only advanceX for glyphs that cannot create image data.

@ocornut
Copy link
Owner

ocornut commented Oct 28, 2022

Hello,

Thanks for the PR but we cannot merge code without understanding a problem.
Please provide a repro that triggers the assert and please explain the problem you are trying to solve.

@nkari82
Copy link
Author

nkari82 commented Oct 28, 2022

Some fonts crash with pack_rect.was_packed set to 0.

Instead, in the case of pack_rect.was_packed 0, we fixed this problem by adding only advanceX of the glyph data without image data.

@ocornut
Copy link
Owner

ocornut commented Oct 28, 2022

Some fonts crash with pack_rect.was_packed set to 0.

Please be specific and provide fonts/size/ranges information.

@nkari82
Copy link
Author

nkari82 commented Oct 28, 2022

Some fonts crash with pack_rect.was_packed set to 0.

Please be specific and provide fonts/size/ranges information.

NanumSquareB/18/GlyphRangesKorean

Same information.
#5788 (comment)

ocornut added a commit that referenced this pull request Jan 4, 2023
… prevent large amount of glyphs from being packed correctly. (#5788, #5829)

This seemingly innocuous change sursingly had very large side-effects of completly breaking packing for the test font mentioned in above issue. Not even sure why tbh. New code matches what stb_truetype's stbtt_PackBegin() does.
@ocornut
Copy link
Owner

ocornut commented Jan 4, 2023

Hello,
I scratched my head about this for a while, as there was something fishy.
Stepping through the packing procedure and comparing both variants (stb and freetype packers) I found the fix: 9150c23
It's a surprisingly innocent-looking fix but has very large effects on packing. I think the sentinel logic of rectpack may have been affected accidentally by this, but I am not sure. Anyhow, which this minor fix it seems like both #5788 and #5829 are correctly fixed.
Thanks for reporting this strange issue!

--stbrp_init_target(&pack_context, atlas->TexWidth, TEX_HEIGHT_MAX, pack_nodes.Data, pack_nodes.Size);
++stbrp_init_target(&pack_context, atlas->TexWidth - atlas->TexGlyphPadding, TEX_HEIGHT_MAX - atlas->TexGlyphPadding, pack_nodes.Data, pack_nodes.Size);

@ocornut ocornut closed this Jan 4, 2023
@ocornut ocornut added the bug label Jan 4, 2023
ocornut added a commit that referenced this pull request Jan 4, 2023
… prevent large amount of glyphs from being packed correctly. (#5788, #5829)

This seemingly innocuous change sursingly had very large side-effects of completly breaking packing for the test font mentioned in above issue. Not even sure why tbh. New code matches what stb_truetype's stbtt_PackBegin() does.
kjblanchard pushed a commit to kjblanchard/imgui that referenced this pull request May 5, 2023
… prevent large amount of glyphs from being packed correctly. (ocornut#5788, ocornut#5829)

This seemingly innocuous change sursingly had very large side-effects of completly breaking packing for the test font mentioned in above issue. Not even sure why tbh. New code matches what stb_truetype's stbtt_PackBegin() does.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants