ocornut / imgui_test_engine

Dear ImGui Automation Engine & Test Suite
386 stars 40 forks source link

Screen capture on macOS with HighDPI fails because of FrameBufferScale!=1 #32

Open pthom opened 9 months ago

pthom commented 9 months ago

Bonjour Omar,

Under macOS with HighDpi, ImGui::GetDrawData().FrameBufferScale is equal to (2., 2.)

The window coordinates are correctly shown by the capture tool, however when capturing we get a capture with bad coordinates

edges shown by capture tool

image

capture that I get

image

This is due to the fact that the FramebufferScale is not 1.

This is probably related to https://github.com/ocornut/imgui_test_engine/issues/8

See the current capture implementation in imgui_app.cpp: it cannot handle the scale

#if defined(IMGUI_APP_GL2) || defined(IMGUI_APP_GL3)
static bool ImGuiApp_ImplGL_CaptureFramebuffer(ImGuiApp* app, int x, int y, int w, int h, unsigned int* pixels, void* user_data)
{
    IM_UNUSED(app);
    IM_UNUSED(user_data);

#ifdef __linux__
    // FIXME: Odd timing issue is observed on linux (Plasma/X11 specifically), which causes outdated frames to be captured, unless we give compositor some time to update screen.
    // glFlush() didn't seem enough. Will probably need to revisit that.
    usleep(1000);   // 1ms
#endif

    int y2 = (int)ImGui::GetIO().DisplaySize.y - (y + h);
    glPixelStorei(GL_PACK_ALIGNMENT, 1);
    glReadPixels(x, y2, w, h, GL_RGBA, GL_UNSIGNED_BYTE, pixels);

    // Flip vertically
    size_t comp = 4;
    size_t stride = (size_t)w * comp;
    unsigned char* line_tmp = new unsigned char[stride];
    unsigned char* line_a = (unsigned char*)pixels;
    unsigned char* line_b = (unsigned char*)pixels + (stride * ((size_t)h - 1));
    while (line_a < line_b)
    {
        memcpy(line_tmp, line_a, stride);
        memcpy(line_a, line_b, stride);
        memcpy(line_b, line_tmp, stride);
        line_a += stride;
        line_b -= stride;
    }
    delete[] line_tmp;
    return true;
}
#endif

I previously implemented a screen capture utility in HelloImGui, and I had to take the FrameBufferScale into account: see opengl_screenshot.cpp in the hello imgui code.

Basically, it multiplies the capture size, like this:

        auto draw_data = ImGui::GetDrawData();
        int fb_width = (int)(draw_data->DisplaySize.x * draw_data->FramebufferScale.x);
        int fb_height = (int)(draw_data->DisplaySize.y * draw_data->FramebufferScale.y);

However this solution cannot apply here for two main reasons:

I tried a dirty hack that consists of creating an intermediary buffer for the capture, and then do a simple resize. It can work, but it is not very nice, and require to allocate an intermediary buffer.

pthom commented 9 months ago

For information, I could implement a temporary hack. If you are interested, I include it below.

The principle is to acquire in a temporary buffer with the correct size, and then do a quick and dirty down-sampling.

Beware:


    void _GlCaptureFramebuffer(
        int x, int y, int w, int h,
        float frameBufferScaleY,    // We now need to know the frameBufferScaleY to be able to flip the y coordinate into y2
        unsigned int* pixels)
    {
#ifdef __linux__
        // FIXME: Odd timing issue is observed on linux (Plasma/X11 specifically), which causes outdated frames to be captured, unless we give compositor some time to update screen.
    // glFlush() didn't seem enough. Will probably need to revisit that.
    usleep(1000);   // 1ms
#endif
        int y2 = (int)ImGui::GetIO().DisplaySize.y * frameBufferScaleY - (y + h);
        glPixelStorei(GL_PACK_ALIGNMENT, 1);
        glReadPixels(x, y2, w, h, GL_RGBA, GL_UNSIGNED_BYTE, pixels);

        // Flip vertically
        size_t comp = 4;
        size_t stride = (size_t)w * comp;
        unsigned char* line_tmp = new unsigned char[stride];
        unsigned char* line_a = (unsigned char*)pixels;
        unsigned char* line_b = (unsigned char*)pixels + (stride * ((size_t)h - 1));
        while (line_a < line_b)
        {
            memcpy(line_tmp, line_a, stride);
            memcpy(line_a, line_b, stride);
            memcpy(line_b, line_tmp, stride);
            line_a += stride;
            line_b -= stride;
        }
        delete[] line_tmp;
    }

    bool ImGuiApp_ImplGL_CaptureFramebuffer(ImGuiID viewport_id, int x, int y, int w, int h, unsigned int* pixels, void* user_data)
    {
        IM_UNUSED(viewport_id);
        IM_UNUSED(user_data);

        ImVec2 framebufferScale(1., 1.f);
        if (ImGui::GetDrawData() != nullptr)
            framebufferScale = ImGui::GetDrawData()->FramebufferScale; // WARNING WARNING: Sometimes GetDrawData() will return NULL
                                                                       //  when invoked from the CaptureTool window!
        // framebufferScale = ImVec2(2.f, 2.f);                        // Manual hack for when GetDrawData() returns null

        // Are we using a scaled frame buffer (for example on macOS with retina screen)
        bool hasFramebufferScale = (framebufferScale.x != 1.f) || (framebufferScale.y != 1.f);

        // if not using scaled frame buffer, perform simple capture
        if (!hasFramebufferScale)
        {
            _GlCaptureFramebuffer(x, y, w, h, framebufferScale.y, pixels);
            return true;
        }

        //
        // else
        //

        // 1. Capture to temporary buffer capturePixels
        auto x_to_scaled = [framebufferScale](int _x) -> int { return (int)((float)_x * framebufferScale.x); };
        auto y_to_scaled = [framebufferScale](int _y) -> int { return (int)((float)_y * framebufferScale.y); };

        int xs = x_to_scaled(x), ys = y_to_scaled(y), ws = x_to_scaled(w), hs = y_to_scaled(h);

        unsigned int* capturePixels = new unsigned int[ws * hs];
        _GlCaptureFramebuffer(xs, ys, ws, hs, framebufferScale.y, capturePixels);

        // 2. Fill pixel from capturePixels: an atrocious and slow loop
        auto get_capture_pixel = [&](int _x, int _y) -> unsigned int {
            int _xs = x_to_scaled(_x), _ys = y_to_scaled(_y);
            return capturePixels[_ys * ws + _xs];
        };
        for (int _y = 0; _y < h; ++_y)
            for (int _x = 0; _x < w; ++_x)
                pixels[_y * w + _x] = get_capture_pixel(_x, _y);

        delete[] capturePixels;
        return true;
    }
ocornut commented 9 months ago

While I am currently unhappy with the use of io.FramebufferScale in general (I believe we should make the coordinates match, which I will do once I have a large DPI pass incl on OSX), i believe in the meanwhile we should add support for it in the capture tool.

pthom commented 9 months ago

Would you accept a PR with a polished version of this hack, or do you prefer to change the capture size (in which case, it will be too hard for me)?

By polished I mean that I could remove the unnecessary calls to lambdas (x_to_scaled), and add some pointer logic in the conversion loop, in order to make it faster. I could not remove the allocation, although.

ocornut commented 9 months ago

I'd prefer to fix the capture size but I'll probably need your help to test it as I don't presently have a Mac.

pthom commented 9 months ago

Ok, ping me when you want me to run a test. My mac reports FrameBufferScale = (2, 2). I never encountered a situation where x_scale != y_scale and/or where they are not an integer.

For info, I wrote a FAQ where I grouped my findings for emscripten, macOS, linux and windows concerning the Dpi handling: https://pthom.github.io/imgui_bundle/faq.html