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

[sokol_imgui] add the optional backend interface for font management #994

Merged
merged 2 commits into from
Mar 2, 2024

Conversation

Dvad
Copy link

@Dvad Dvad commented Feb 22, 2024

As a happy sokol_imgui user, I am humbling proposing this PR. It splits the font texture creation in a API function and add a function for the font texture destruction. Those functions are considered optional function of the imgui backends.

For instance they are implemented for opengl3 on the main imgui repo:

https://github.com/ocornut/imgui/blob/659fb41d0a23efbb9ea6cf74f51ecae0a51575b5/backends/imgui_impl_opengl3.h#L39C1-L42C3

I find those functions are useful in two cases:

  • when testing different fonts raterization config, it is convenient to recreate the texture dynamically. The imgui's author provides an example of such application: https://gist.github.com/ocornut/b3a9ecf13502fd818799a452969649ad
  • when creating non default font, this allows to remove the boilerplate of uploading the font on GPU for the user of sokol_imgui.

Does this proposal make sense?

…ction

This split the font texture  creation in a API function and add a function for the font texture destruction. Those functions are  optional function of the backends.

For instance implemented for opengl3 on the main imgui repo:

https://github.com/ocornut/imgui/blob/659fb41d0a23efbb9ea6cf74f51ecae0a51575b5/backends/imgui_impl_opengl3.h#L39C1-L42C3

Those functions are useful in two cases:
- when testing different fonts raterization config,  it is convenient to recreate the texture dynamically. The imgui's author provides an example of such application: https://gist.github.com/ocornut/b3a9ecf13502fd818799a452969649ad
- when creating non default font, this allows to remove the burdern of the boilerplate of uploading the font on GPU to the user of sokol_imgui.
@floooh
Copy link
Owner

floooh commented Feb 22, 2024

Good idea! But I wonder if we should make the create-fonts function a bit more flexible by passing some data in.

For instance, it would be nice if I could replace this code block in the imgui-highdpi-sapp.c sample which sets up a TTF font texture with the new function:

https://github.com/floooh/sokol-samples/blob/6a03725117ce0175a45fa2d0c51ad3272c955cb5/sapp/imgui-highdpi-sapp.cc#L45-L67

...but as you can see here I'm also using a different sampler with LINEAR filtering:

https://github.com/floooh/sokol-samples/blob/6a03725117ce0175a45fa2d0c51ad3272c955cb5/sapp/imgui-highdpi-sapp.cc#L58-L63

Maybe simgui_create_fonts_texture() should allow to pass in optional parameters (like either a sampler object, or maybe better just a minimal set of sampler params, like min+mag filter?

Or do you solve this same problem currently in some other way? (since you are writing: "...when testing different fonts raterization config...").

@floooh floooh self-assigned this Feb 22, 2024
@Dvad
Copy link
Author

Dvad commented Feb 22, 2024

This is good suggestion!

I have not played with the sampler in my testing so far, the tool I linked allow to switch from freetype to stb_truetype with different parameters but the texture sampler was not one.

@Dvad
Copy link
Author

Dvad commented Feb 22, 2024

I gave it a shot and submitted the sister PR:
floooh/sokol-samples#135

@floooh
Copy link
Owner

floooh commented Mar 2, 2024

Ok, looking into this PR now.

@floooh floooh merged commit 764375f into floooh:master Mar 2, 2024
23 checks passed
@floooh
Copy link
Owner

floooh commented Mar 2, 2024

And merged, many thanks! (also updated the changelog)

@Dvad Dvad deleted the font_texture branch March 3, 2024 01:52
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