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

Backends: DX9: use RGBA texture to avoid conversion if supported #6575

Closed

Conversation

Demonese
Copy link
Contributor

@Demonese Demonese commented Jul 5, 2023

My application users are Chinese and Japanese, so there are many glyphs and the font atlas texture is large (equal to or more than 2048x2048). Since dear-imgui does not yet support dynamic glyph caching, I also have to dynamically add glyphs and then recreate texture.

Converting 2048x2048 texture takes 16ms on Intel i7-12700h.

In order to shorten the time of application initialization, font size setup, response to DPI scaling, font atlas texture recreate, I think it is necessary to add support for RGBA texture formats (if supported).

Although theoretically DX9 does not support the RGBA format very well, I'm happy to find that there are still many devices support RGBA format, include d3d9on12 device, and d3d9on12 device is everywhere on the Intel 12th Gen or later CPU.

@ocornut
Copy link
Owner

ocornut commented Jul 5, 2023

Hello,
Thanks for the PR.
Could you instead store the bool inside ImGui_ImplDX9_Data so it’s only queried once?

Converting 2048x2048 texture takes 16ms on Intel i7-12700h.

Are you certain this is for the conversion loop we wrote, rather than just whole atlas building?

Note that if you are not using colored icons you could technically use a single channel texture as well.

@Demonese
Copy link
Contributor Author

Demonese commented Jul 5, 2023

Could you instead store the bool inside ImGui_ImplDX9_Data so it’s only queried once?

Cache is not reliable, because it also relies on the format used by the current swap-chain (output merge stage). Also, the cost of query is very cheap, usually 2 to 4 microseconds, this is perfectly acceptable.

Are you certain this is for the conversion loop we wrote, rather than just whole atlas building?

Yes, just converting, texture size is 2048x2048, x64 debug profile cost about 16ms, x64 release profile cost about 10ms.

    // Convert RGBA32 to BGRA32 (because RGBA32 is not well supported by DX9 devices)
#ifndef IMGUI_USE_BGRA_PACKED_COLOR
    LARGE_INTEGER freq{}, last{}, next{};
    QueryPerformanceFrequency(&freq);
    const bool rgba_support = false; ImGui_ImplDX9_CheckFormatSupport(bd->pd3dDevice, D3DFMT_A8B8G8R8);
    QueryPerformanceCounter(&last);
     if (!rgba_support && io.Fonts->TexPixelsUseColors)
    {
        ImU32* dst_start = (ImU32*)ImGui::MemAlloc((size_t)width * height * bytes_per_pixel);
        for (ImU32* src = (ImU32*)pixels, *dst = dst_start, *dst_end = dst_start + (size_t)width * height; dst < dst_end; src++, dst++)
            *dst = IMGUI_COL_TO_DX9_ARGB(*src);
        pixels = (unsigned char*)dst_start;
    }
    QueryPerformanceCounter(&next);
    double t = double(next.QuadPart - last.QuadPart) / double(freq.QuadPart);
    char buffer[64]{};
    snprintf(buffer, 64, "%.6fs\n", t);
    OutputDebugStringA(buffer);
#endif
  • Debug image
  • Release image

Note that if you are not using colored icons you could technically use a single channel texture as well.

I did think about it, but unfortunately we need to support colored emoji, colored icons. The better solution might be IMGUI_USE_BGRA_PACKED_COLOR.

@Demonese Demonese force-pushed the features/imgui_impl_dx9_rgba branch from 155fc57 to c5e6663 Compare July 5, 2023 17:49
@ocornut
Copy link
Owner

ocornut commented Feb 12, 2024

Thanks! I am merging this (w/ minor simplifications: 1d6f0ce) out of courtesy but if when we end up supporting dynamic atlas I may want to remove the additional complexity exhibited here.

@ocornut ocornut closed this Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants