imgui icon indicating copy to clipboard operation
imgui copied to clipboard

Add an ability to update font atlas texture

Open thedmd opened this issue 4 years ago • 13 comments
trafficstars

PR add an ability to update font atlas texture. Based on feature/shadows.

There is an information about overall progress:

  • SDL, GLUT, GLFW3 and Allegro 5 backends were tested on Windows.
  • Metal backends was tested on macOS Big Sur.
  • Emscripten was tested on emcc 2.0.12.
  • Marmalade backend and example is not supported.

Backend

Progress: 15/15

Backend Status Comment
imgui_impl_win32.h No change required.
imgui_impl_vulkan.h There is ImGui_ImplVulkan_UpdateFontsTexture user can need to use in own code.
imgui_impl_sdl.h No change required.
imgui_impl_osx.h No change required.
imgui_impl_opengl3.h Done.
imgui_impl_opengl2.h Done.
imgui_impl_metal.h Done.
imgui_impl_marmalade.h Not supported.
imgui_impl_glut.h No change required.
imgui_impl_glfw.h No change required.
imgui_impl_dx12.h There is ImGui_ImplDX12_UpdateFontsTexture user need to call in own code.
imgui_impl_dx11.h Done.
imgui_impl_dx10.h Done.
imgui_impl_dx9.h Done.
imgui_impl_allegro5.h Done.

Examples

Progress: 20/20

Example Status Comment
example_allegro5 Works.
example_apple_metal Works.
example_apple_opengl2 Works.
example_emscripten_opengl3 Works.
example_glfw_metal Works.
example_glfw_opengl2 Works.
example_glfw_opengl3 Works.
example_glfw_vulkan Works. Use ImGui_ImplVulkan_UpdateFontsTexture and explicitly set ImGuiBackendFlags_RendererHasTexReload flag.
example_glut_opengl2 Works.
example_marmalade Not supported.
example_null No change required.
example_sdl_directx11 Works.
example_sdl_metal Works.
example_sdl_opengl2 Works.
example_sdl_opengl3 Works.
example_sdl_vulkan Works. Use ImGui_ImplVulkan_UpdateFontsTexture and explicitly set ImGuiBackendFlags_RendererHasTexReload flag.
example_win32_directx9 Works.
example_win32_directx10 Works.
example_win32_directx11 Works.
example_win32_directx12 Works. Use ImGui_ImplDX12_UpdateFontsTexture and explicitly set ImGuiBackendFlags_RendererHasTexReload flag.

thedmd avatar Jan 26 '21 20:01 thedmd

Thank you for the amazing work reviving this @thedmd. Some quick thoughts (and we can discuss details in our chat later):

Given the simplified specs for "full reload" this looks good, but I would like us to make sure we have a sensible path for when we'll want to transition this to the fuller version (https://github.com/ocornut/imgui_private/issues/3) They are two main difference:

1. Textures may have to be uploaded in _Render (to support dynamic glyphes).

I think it may be simpler/favorable to only try to upload at the end of the frame rather than also support the optimal timing for the initialization scenario or rare best-case scenario where we can do a full upload around NewFrame().

So presumably, the first ImplXXX_RenderDrawData() function call of each standard backend may be the one doing the upload if required.

(Advanced: to reduce latency on high-performing application, we want to permit custom backend to start uploading the texture after EndFrame() - without waiting for Render() or even ImplXXX_RenderDrawData(). This shouldn't be a problem at all from ImGui:: point of view. To test this scenario, I imagine some default backend may want expose a ~ImplXXX_UpdateTexture function which ImplXXX_RenderDrawData would call is texture is dirty, but we allow user to call that function. It's not 100% required that all default backend support it, but I still it would help convey the possibility and and test the scenario)

Timeline for Frame N:

  • Old/existing backends will still attempt to build the atlas in ImplXXX_NewFrame() - before ImGui::NewFrame() - and then upload texture. Should not be a problem.
  • ImGui::NewFrame() will need to build the atlas data if dirty because we need glyphs UV ready. (with new backends, ImGui::NewFrame will be the first piece of code updating this, with old backends, this is likely to be already built). Technically we don't even need the pixels to be rasterized at this point (but it's simpler to rasterize them).
  • As a result of forcing the atlas build in NewFrame(), the very useful assert IM_ASSERT(g.IO.Fonts->Fonts[0]->IsLoaded() in NewFrame() doesn't make sense anymore. Instead we can try to replace the assert with one that checks that if the texture was dirty on NewFrame..EndFrame N, someone should have called XXXGetTexData by the time NewFrame N+1 is called (because with new backends the right time for texture upload will be after ImGui::EndFrame or ImGui::Render)
  • Anytime after EndFrame(), the backend is allowed to get texture data and upload it. In default backends this will generally be done in ImplXXX_RenderDrawData().

2. We'll want (in future) to notify the back-end of partial update.

This was partially (and bit awkwardly) covered in the original version. I don't think we need to have it 100% ready now but we should explore the proof of concept API at leaast, to ensure a smooth transition from this PR version. Maybe we can even come to the realization that we can implement the API completely today, and have the back-end not fully honor the partial update, or something (not sure that's a good idea). Maybe we need to separate into two flags ImGuiBackendFlags_RendererHasTexReload and ImGuiBackendFlags_RendererHasTexPartialUpdate ?

Unrelated (minor): why does ClearInputData() remove CustomRects.clear() and PackIdXXX clearing?

ocornut avatar Feb 03 '21 16:02 ocornut

  1. Textures may have to be uploaded in _Render (to support dynamic glyphs).

Incremental atlas baking branch i am working on updates font atlas texture outside of imgui frame. Which means that for very first time an unknown glyph is encountered, a fallback character is rendered. It is invisible to naked eye at 60fps so probably is good enough.

rokups avatar Feb 08 '21 12:02 rokups

Which means that for very first time an unknown glyph is encountered, a fallback character is rendered. It is invisible to naked eye at 60fps so probably is good enough.

Sorry but this is not good enough.

We're working out a scheme with @thedmd to push this forward, it's going to work out :)

ocornut avatar Feb 08 '21 12:02 ocornut

Turns out rasterizing glyphs immediately wasnt that big of a deal. I am not quite sure if locking font atlas during frame is warranted. It rebuilds just fine, granted it happens on main thread. The only issue i noticed is that build step intends to clear texture id, which is undesirable as ImDrawList::AddText() does IM_ASSERT(font->ContainerAtlas->TexID == _CmdHeader.TextureId). I am not sure whether this detail was overlooked or not, but apparently if we request font atlas update during this frame and we would like to use new atlas by the time this frame is rendering on GPU - we will have to update ImDrawCmd::TextureId of all draw commands with a new texture id.

rokups avatar Mar 01 '21 10:03 rokups

I am not quite sure if locking font atlas during frame is warranted.

Probably not anymore.

We will have to update ImDrawCmd::TextureId of all draw commands with a new texture id.

Yes this is why we changed all the render loop to use draw_cmd->GetTexID().

ocornut avatar Mar 02 '21 11:03 ocornut

Well, status table went out of date after introducing ability to update texture mid-frame.

thedmd avatar Jul 30 '21 07:07 thedmd

Technically we also moved this to a private branch for now (but thedmd: you could push that branch back into your public copy)

ocornut avatar Jul 30 '21 08:07 ocornut

I pushed private branch here. I expect most backends will not pass build test. DX9 however should build fine. This is still work in progress.

thedmd avatar Jul 31 '21 12:07 thedmd

Rebased on v1.89 WIP (18808)

thedmd avatar Aug 19 '22 14:08 thedmd

Hello, we are interested in this PR, we are a bit stuck on a feature until this is merged https://github.com/maximegmd/CyberEngineTweaks/pull/840

We cannot rebuild textures currently, we also believe we are experiencing a GPU crash because of fonts when there is a d3d12 reset, this PR should also address this issue.

Is there something we can do to help move this forward?

maximegmd avatar Jun 28 '23 19:06 maximegmd

@maximegmd Based on the commit history of https://github.com/ocornut/imgui/commits/features/range_select, I guess when range select is ready, v1.90 will be shipped and this PR would also be included.

vkedwardli avatar Jul 18 '23 11:07 vkedwardli

Unfortunately the 1.90 milestone goal was probably too optimistic for this so it’ll probably be a bit later. Right now indeed focusing on range-select and after 1.90 is shipped we can look at this.

ocornut avatar Jul 18 '23 12:07 ocornut

Rebased this on v1.89.8 WIP (18974), done only basic testing. (was a bit tedious because of the NULL->nullptr change in backend but should be all done now). Briefly noticed that both DX10/DX11 have issue with "Rebuild every frame" in demo but this already happened before I rebased, we may have just left it as is. Won't look at it until post 1.90.

ocornut avatar Jul 25 '23 11:07 ocornut