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

Better fonts rendering / allow to use e.g. FreeType #618

Closed
Vuhdo opened this issue Apr 29, 2016 · 26 comments
Closed

Better fonts rendering / allow to use e.g. FreeType #618

Vuhdo opened this issue Apr 29, 2016 · 26 comments

Comments

@Vuhdo
Copy link

Vuhdo commented Apr 29, 2016

Let me start by saying, Dear ImGui rocks!
In fact, it's so great that I literally want to use it more than I really need to :)

That said, I have some really annoying problem - mediocre text visuals.
Stb_truetype, the underlying font rendering library, is simple and convenient,
but not that great comparing to, say, FreeType's hinting rasterizer.

Proggy font (the built-in one) is crisp, but it's essentially a bitmap font which
doesn't imply anti-aliased rasterizing.
It's a fixed-width font which really looks like some 'programmer's art', that
might be a problem or not (my artists will make fun of me for sure).
Moreover, it contains only basic Latin glyphs which is really a problem in
some environments (for example, we badly need to render russian texts in our tools).

I've tried to use a number of popular outline fonts (Droids, Microsoft's SegoeUI, etc.).
They all look blurry, at least when using lower pixel sizes.
And that is understandable, stb_truetype does no hinting, only anti-alising.

What I propose is to add some interface so a user could plug-in his own font rasterizer.
This way we can more or less easily substitute stb_truetype with FreeType without bloating the library.

One minor feature which I think can be easily implemented is to add shadows to texts.
Another one is to add glyph outlines, but I bet it's too difficult without using an advanced glyph
rasterizer and complicating the library's code.

What do you think?

-- Alexey

@ocornut
Copy link
Owner

ocornut commented Apr 29, 2016

Hello,

Obviously the purpose of Proggy is to provide a default embedded-in-source-code font for quick tools. It isn't meant to cover everything.

I don't know what you mean by "lower pixel sizes", although I guess at 12- it won't look amazing. Though you can increase subsampling and it'll look a little better. stb_truetype also doesn't do sub-pixels.

This is a good idea but I won't have time to do it myself, I don't have the resources and it isn't a development priority for me. If you want to work on it'd be great. The function to replace is ImFontAtlas::Build(). Interestingly we don't even need to change code in ImGui core, one could have an external Build function that would work with same input and same outputs and it would work. If you call that before GetTexDataAs.. functions then Build() won't be called. Tho it might be sane/beneficial to introduce some more explicit option in ImGui as well, it could work without.

Another possibility that would be more work but also be beneficial to larger number of people would be to improve stb_truetype.

@ocornut ocornut changed the title Better fonts rendering Better fonts rendering / allow to use e.g. FreeType Apr 29, 2016
@Vuhdo
Copy link
Author

Vuhdo commented Apr 30, 2016

Omar,

Here are my preliminary results, using rather problematic SegoeUI font at 15px:

Built-in rasterizer:
stb_truetype

FreeType's (auto) hinting rasterizer:
freetype

Stb_truetype renders using default ImFontConfig settings.
FreeType uses auto-hinting.
Note the differences in size due to FreeType's hinting (and probably my bugs being hacking here and there all over the place).

@hypernewbie
Copy link

Looks awesome!
I also think robust font rendering is ImGui's weakest point.

@ocornut
Copy link
Owner

ocornut commented May 1, 2016

Looking nice!
Did you do it by reimplementing ImFontAtlas::Build() as suggested? Curious to see the code there!

PS: The sizing/spacing difference seems like a bug and unrelated to auto-hinting. On some pairs it might feel better because it might compensates for the lack of kerning but generally it doesn't look right. As a side note, kerning supports would in theory be nice but in practice I don't know how to do it without affecting performances a lot (text size calculation is a major bottleneck). We could add kerning as an optional per-font option by duplicating some of the ImFont code, however some code here and there (code in InputText) would need to be reworked to be kerning friendly, aka not processing characters in pairs. Gut feeling is that we can ignore kerning for now, rasterizing quality is a much more valuable benefit as you are pointing out.

@Vuhdo
Copy link
Author

Vuhdo commented May 3, 2016

Omar,
Yes, just ImFontAtlas::Build() as you suggested.

The fork with my experiments is here: https://github.com/Vuhdo/imgui
Only the dx11 example is working.
Probably lots of bugs. FreeType and text layout are new to me :)
I would be really grateful If you will find the time to dig through the changes and express your opinion.

@Vuhdo
Copy link
Author

Vuhdo commented May 3, 2016

Probably should be posted in screenshots thread, but since it's not anything new, but just reskined ImGui demo with FreeType rasterizer, I'll post it here. Here's how my current mockup looks like:

https://i.gyazo.com/23d49a4530571939106e884bf1f3e1d6.png

MOD Attachment fairy
23d49a4530571939106e884bf1f3e1d6

@ocornut
Copy link
Owner

ocornut commented May 4, 2016

Screenshot looking good! I have only skimmed through it but ideally it could be done without touching the imgui files, then the code will be easier to share and maintain. Since everything is public/struct you can have a totally extern Build function taking ImFontAtlas* parameter and work with it as a completely standalone piece of code. We can even provide this blurb of code in the repository as e.g. extra_fonts/imgui_freetype.h

Just call your function prior to calling GetTexDataXXX and normal Build() won't be called. If there is a need to add an extra flag/info in the structure to make that possible we can do that.

@ocornut
Copy link
Owner

ocornut commented May 29, 2016

@Vuhdo Any news about taking this code out of imgui_draw.cpp so it can be used as a separate extension?

@Vuhdo
Copy link
Author

Vuhdo commented May 30, 2016

@ocornut Yeah, it's here: https://github.com/Vuhdo/imgui_freetype.
Unfortunately, I had no time to fix all bugs (like, excessive font texture resolution).
It's good enough for me at the moment, but it definitely needs improvements.

@emoon
Copy link
Contributor

emoon commented Jun 6, 2016

Interested in having support for using FreeType as well even if I like the stb_x libs I like higher quality text more :)

@Flix01
Copy link

Flix01 commented Jun 6, 2016

@Vuhdo It works! And it doesn't need any modification to imgui (even if it's not possible to disable the stb_truetype stuff to reduce memory usage)!

Some thoughts:
-> ImGuiFreeType::BuildFontAtlas(...) needs ImGui::MemFree(nodes); at the end, according to Valgrind (sorry, but there was no issue section in your repository).
-> Instead of calling ImGuiFreeType::BuildFontAtlas(...) myself, I've wrapped these calls too:

namespace ImGuiFreeType {
IMGUI_API void GetTexDataAsAlpha8(ImFontAtlas* atlas,unsigned char** out_pixels, int* out_width, int* out_height, int* out_bytes_per_pixel,unsigned int flags=0)   {
    using namespace ImGui;
    // Build atlas on demand
    if (atlas->TexPixelsAlpha8 == NULL)
    {
        if (atlas->ConfigData.empty())
            atlas->AddFontDefault();
        BuildFontAtlas(atlas,flags);
    }

    *out_pixels = atlas->TexPixelsAlpha8;
    if (out_width) *out_width = atlas->TexWidth;
    if (out_height) *out_height = atlas->TexHeight;
    if (out_bytes_per_pixel) *out_bytes_per_pixel = 1;
}
IMGUI_API void GetTexDataAsRGBA32(ImFontAtlas* atlas,unsigned char** out_pixels, int* out_width, int* out_height, int* out_bytes_per_pixel,unsigned int flags=0)    {
    // Convert to RGBA32 format on demand
    // Although it is likely to be the most commonly used format, our font rendering is 1 channel / 8 bpp
    if (!atlas->TexPixelsRGBA32)
    {
        unsigned char* pixels;
        GetTexDataAsAlpha8(atlas,&pixels, NULL, NULL,NULL,flags);
        atlas->TexPixelsRGBA32 = (unsigned int*)ImGui::MemAlloc((size_t)(atlas->TexWidth * atlas->TexHeight * 4));
        const unsigned char* src = pixels;
        unsigned int* dst = atlas->TexPixelsRGBA32;
        for (int n = atlas->TexWidth * atlas->TexHeight; n > 0; n--)
            *dst++ = ((unsigned int)(*src++) << 24) | 0x00FFFFFF;
    }

    *out_pixels = (unsigned char*)atlas->TexPixelsRGBA32;
    if (out_width) *out_width = atlas->TexWidth;
    if (out_height) *out_height = atlas->TexHeight;
    if (out_bytes_per_pixel) *out_bytes_per_pixel = 4;
}
} // namespace ImGuiFreetype

Then, instead of calling something like:

io.Fonts->GetTexDataAsRGBA32(&pixels, &width, &height);

I call:

ImGuiFreeType::GetTexDataAsRGBA32(io.Fonts,&pixels, &width, &height,NULL,flags);
// where flags are the ImGuiFreeType::RasterizationFlags I want to use

Do you think it's correct, or there are cases where my approach doesn't work ?
-> It should be nice if the flags could be set on a per font basis (when using multiple fonts).
-> Maybe by using Freetype we can "resurrect" outline fonts (at least when GetTexDataAsRGBA32(...) is used). I'm not sure this is possible/easy to achieve, but I remember that ImGui had them long time ago!
-> In any case the Bold and Oblique additions are useful: we can use a single .ttf for Regular,Bold and Italic chars.

P.S. I think I'll merge your code into my branch sooner or later, as an optional feature :)

@ocornut
Copy link
Owner

ocornut commented Jun 6, 2016

Better just add a separate Build function that the user can call, you don't need to create new GetTexDataAsXXX functions. And that code needs not to be part of imgui.cpp file, it can be separate.

@Vuhdo
Copy link
Author

Vuhdo commented Jun 7, 2016

@Flix01 Thanks for the bug report!

I've fixed the memory leak, added missing FreeType's binaries and updated README:
https://github.com/Vuhdo/imgui_freetype

@ocornut
Copy link
Owner

ocornut commented Jun 7, 2016

Nice! Now it only needs to be moved outside of an imgui folder into its one folder so zero forking/merging is needed :)

I wonder if it would be a good occasion to create an official imgui_extras repo for that sort of stuff, and docking?

@Vuhdo
Copy link
Author

Vuhdo commented Jun 7, 2016

Yeah, official imgui_extras would be great, but will require a lot of attention from you anyway :)
On the other side, we desperately need that docking stuff!

@ChengduLittleA
Copy link

well I happened to find out that, if you use Freetype and your text looks a little blurry when small, try pow(bitmap_alpha, 1/2.2), accidentally found that this looks pretty good when i'm dealing with sRGB stuff, not a ultimate solution but it's worth trying for freetype users.
*the bitmap_alpha is the font texture alpha channel.

@cupe
Copy link

cupe commented Aug 28, 2016

@Vuhdo nice! Is there any specific reason your version doesn't use subpixel rendering/hinting? It's the one thing I'd like most from freetype.

@RobertoMalatesta
Copy link

@ocornut said:

I wonder if it would be a good occasion to create an official imgui_extras repo for that sort of stuff, and docking?

It's about time since say, two years at least.
I've seen so many gems extending IMGUI lost in the rain of code of github.
(wondering if to put a smiley here or not.)
--R

@bluebear94
Copy link

Any update on this issue?

@ocornut
Copy link
Owner

ocornut commented Jan 15, 2017

Any update on this issue?

No, but you can freely use @Vuhdo's code if you want to rasterize using freetype for now.

@ocornut
Copy link
Owner

ocornut commented Aug 17, 2017

Tada!
That code broke today as I made some needed changes to ImFontAtlas, but I did as planned and I'm starting a repo for officially maintained helpers of that sort..

https://github.com/ocornut/imgui_club/

The updated imgui_freetype.h code is here:
https://github.com/ocornut/imgui_club/tree/master/imgui_freetype

Hopefully with @Vuhdo benediction!

@Vuhdo
Copy link
Author

Vuhdo commented Aug 17, 2017

Awesome, Omar!

@ocornut
Copy link
Owner

ocornut commented Aug 25, 2017

Thanks @Vuhdo. Closing this topic now!.
I will maintain the version in imgui_club and will soon commit some changes to it.

@ocornut
Copy link
Owner

ocornut commented Feb 4, 2018

@Vuhdo , @mikesart, @pdoane

I have moved imgui_freetype into the main imgui repository in the misc/freetype folder.
This means you don't need to sync/merge two repos.

I noticed that in many instances the vertical positioning is not the same between the STB and freetype rasterizer. Will need to look into it, as a workaround you can alter the font->DisplayOffset.y field.

The README in misc/freetype/ also has a bit of code you can use to experiment with this rasterizer.
image

Note that the RasterizerMultiply field also works with the default rasterizer and for light-weight font can be useful.

@mikesart
Copy link

mikesart commented Feb 5, 2018

Oh cool, this is fantastic. I'll get this version into gpuvis. Thanks Omar!

ocornut added a commit that referenced this issue Jan 10, 2019
- Fixed abnormally high atlas height. (#618)
- Fixed support for any values of TexGlyphPadding (not just only 1). (#618)
- Atlas width is now properly based on total surface rather than glyph count (unless overridden with TexDesiredWidth). (#618)
- Fixed atlas builder so missing glyphs won't influence the atlas texture width. (#2233, #618)
- Fixed atlas builder so duplicate glyphs (when merging fonts) won't be included in the rasterized atlas. (#618)
@ocornut
Copy link
Owner

ocornut commented Jan 10, 2019

FYI @Vuhdo I have pushed a rework of the imgui_freetype code
#2270

Namely to standardize both builders and to fix the space waste that the FreeType builder had.

Will probably merge in master soon if I get enough feedback that nothing has been broken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants