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: font atlas texture format convertion will happen more than once #3868

Closed
Demonese opened this issue Mar 3, 2021 · 2 comments
Labels

Comments

@Demonese
Copy link
Contributor

Demonese commented Mar 3, 2021

Version/Branch of Dear ImGui:

Version: 1.82 (WIP latest)
Branch: master

Back-end/Renderer/Compiler/OS

Back-ends: imgui_impl_dx9.cpp
Compiler: any
Operating System: any

My Issue/Question:

In imgui_draw.cpp, GetTexDataAsRGBA32 will return texture data if has been built.

void    ImFontAtlas::GetTexDataAsAlpha8(unsigned char** out_pixels, int* out_width, int* out_height, int* out_bytes_per_pixel)
{
    // Build atlas on demand
    if (TexPixelsAlpha8 == NULL)
    {
        if (ConfigData.empty())
            AddFontDefault();
        Build();
    }

    *out_pixels = TexPixelsAlpha8;
    if (out_width) *out_width = TexWidth;
    if (out_height) *out_height = TexHeight;
    if (out_bytes_per_pixel) *out_bytes_per_pixel = 1;
}

void    ImFontAtlas::GetTexDataAsRGBA32(unsigned char** out_pixels, int* out_width, int* out_height, int* out_bytes_per_pixel)
{
    // 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 (!TexPixelsRGBA32)
    {
        unsigned char* pixels = NULL;
        GetTexDataAsAlpha8(&pixels, NULL, NULL);
        if (pixels)
        {
            TexPixelsRGBA32 = (unsigned int*)IM_ALLOC((size_t)TexWidth * (size_t)TexHeight * 4);
            const unsigned char* src = pixels;
            unsigned int* dst = TexPixelsRGBA32;
            for (int n = TexWidth * TexHeight; n > 0; n--)
                *dst++ = IM_COL32(255, 255, 255, (unsigned int)(*src++));
        }
    }

    *out_pixels = (unsigned char*)TexPixelsRGBA32;
    if (out_width) *out_width = TexWidth;
    if (out_height) *out_height = TexHeight;
    if (out_bytes_per_pixel) *out_bytes_per_pixel = 4;
}

That means if we don't call Clear manually, the following code will causes the R and B channels to swap multiple times (for example, if we change window size, device will lose and we have to recreate all objects):

static bool ImGui_ImplDX9_CreateFontsTexture()
{
    // Build texture atlas
    ImGuiIO& io = ImGui::GetIO();
    unsigned char* pixels;
    int width, height, bytes_per_pixel;
    io.Fonts->GetTexDataAsRGBA32(&pixels, &width, &height, &bytes_per_pixel);
    
    // Convert RGBA32 to BGRA32 as the earlier is not well supported by DX9 devices
#ifndef IMGUI_USE_BGRA_PACKED_COLOR
    if (io.Fonts->TexPixelsUseColors)
        for (ImU32* p = (ImU32*)pixels, *p_end = p + width * height; p < p_end; p++)
            *p = IMGUI_COL_TO_DX9_ARGB(*p);
#endif

Screenshots/Video

first open dx9 example:
1
adjust window size:
2

Standalone, minimal, complete and verifiable example: (see #2261)

test code from #3844

#define IMGUI_USE_WCHAR32
#define IMGUI_ENABLE_FREETYPE
//#define IMGUI_USE_BGRA_PACKED_COLOR

main.cpp

...

#include "imgui_freetype.h"

...

    ImFontGlyphRangesBuilder builder;
    builder.AddRanges(io.Fonts->GetGlyphRangesDefault());
    builder.AddText(u8"😀");
    ImVector<ImWchar> ranges;
    builder.BuildRanges(&ranges);
    ImFontConfig cfg;
    cfg.FontBuilderFlags = ImGuiFreeTypeBuilderFlags_LoadColor;
    ImFont* font = io.Fonts->AddFontFromFileTTF("c:\\Windows\\Fonts\\SEGUIEMJ.ttf", 16.0f, &cfg, ranges.Data);

How to solve this problem?

How are you again, DX9 Backend, your BGRA32 color format cause so many problems : (
There are several possibilities:

  1. add a field to record the texture data format (once and for all solutions)
  2. call Clear() everytime before create texture object (This is the simplest solution, but the performance penalty is unacceptable)
  3. add a global bool flag in dx9 backend to record whether a conversion has occurred (might cause other problem and may not be compatible with dynamic font atlas in the future)

Thanks!

@ocornut
Copy link
Owner

ocornut commented Mar 3, 2021

You are right.
I think the simplest solution is to temporary malloc a new array in the backend, copy/convert and then upload that to video memory. Will implement that now.

@ocornut
Copy link
Owner

ocornut commented Mar 3, 2021

Pushed fix now, thank you very much!

@ocornut ocornut closed this as completed Mar 3, 2021
Demonese pushed a commit to Demonese/imgui that referenced this issue Mar 4, 2021
Demonese pushed a commit to Demonese/imgui that referenced this issue Mar 5, 2021
Demonese pushed a commit to Demonese/imgui that referenced this issue Mar 11, 2021
Demonese pushed a commit to Demonese/imgui that referenced this issue Mar 13, 2021
Demonese pushed a commit to Demonese/imgui that referenced this issue Mar 16, 2021
Demonese pushed a commit to Demonese/imgui that referenced this issue Mar 19, 2021
Demonese pushed a commit to Demonese/imgui that referenced this issue Apr 29, 2021
Demonese pushed a commit to Demonese/imgui that referenced this issue May 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants