Skip to content

korean packing rect size multiply 2#5788

Closed
seokwoongchoi wants to merge 2 commits intoocornut:masterfrom
seokwoongchoi:master
Closed

korean packing rect size multiply 2#5788
seokwoongchoi wants to merge 2 commits intoocornut:masterfrom
seokwoongchoi:master

Conversation

@seokwoongchoi
Copy link
Copy Markdown

@seokwoongchoi seokwoongchoi commented Oct 18, 2022 •
edited
Loading

Some koreans seems to be a little bigger.
So if we use freetype, it is necessary to increase the width of the correct when packing.

Changes:

  • When you start freetype packing, rects[i].w *= 2

NanumSquareB_ttf.zip

@seokwoongchoi seokwoongchoi reopened this Oct 18, 2022
@seokwoongchoi seokwoongchoi reopened this Oct 18, 2022
@ocornut
Copy link
Copy Markdown
Owner

ocornut commented Oct 18, 2022

Hello, thanks for the PR.
Please provide a reproduction to show the issue so we can look into a solution.

@seokwoongchoi
Copy link
Copy Markdown
Author

seokwoongchoi commented Oct 27, 2022 •
edited
Loading

i've tried find better solution. but couldn't find it.
my test case are

  • using icon/merged fonts
  • using asian fonts(korean)
  • using freetype builder
    `
            ImFontConfig config;
            config.FontDataOwnedByAtlas = false;
            config.MergeMode = false;
            float fontSize = 18.0f;

            {
                const ImWchar* ranges = io.Fonts->GetGlyphRangesKorean();
                font_ = io.Fonts->AddFontFromMemoryTTF((void*)NanumSquareB_ttf, sizeof(NanumSquareB_ttf), fontSize, &config, ranges);
            }

            config.MergeMode = true;
            config.DstFont = font_;


            {
                ImWchar ranges[] = { ICON_MIN, ICON_MAX, 0 };
                io.Fonts->AddFontFromMemoryTTF((void*)s_iconsTtf, sizeof(s_iconsTtf), fontSize - 3.0f, &config, ranges);
            }

font.zip

@seokwoongchoi seokwoongchoi reopened this Oct 27, 2022
@nkari82 nkari82 mentioned this pull request Oct 28, 2022
Closed
ocornut added a commit that referenced this pull request Jan 4, 2023
@ocornut
imgui_freetype: fixed a packing issue which in some occurrences would…
eaa2a77 
… 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
Copy Markdown
Owner

ocornut commented Jan 4, 2023 •
edited
Loading

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
@ocornut
imgui_freetype: fixed a packing issue which in some occurrences would…
9150c23 
… 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.
maztheman pushed a commit to maztheman/imgui that referenced this pull request Mar 10, 2025
@ocornut
imgui_freetype: fixed a packing issue which in some occurrences would…
0edfe3e 
… 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

Reviewers

No reviews

Assignees

No one assigned

Labels

bug font/text

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@seokwoongchoi @ocornut