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

Enabling vertical outer borders on tables produces an off-by-one pixel error for inner vertical borders #3752

Closed
rationalcoder opened this issue Jan 23, 2021 · 3 comments

Comments

@rationalcoder
Copy link

Version/Branch of Dear ImGui:

Version: 1.80
Branch: docking

Back-ends: imgui_impl_win32.cpp + custom OpenGL renderer
Operating System: Windows

Config dump:

Dear ImGui 1.80 WIP (17907)
--------------------------------
sizeof(size_t): 8, sizeof(ImDrawIdx): 2, sizeof(ImDrawVert): 20
define: __cplusplus=199711
define: _WIN32
define: _WIN64
define: _MSC_VER=1925
define: IMGUI_HAS_VIEWPORT
define: IMGUI_HAS_DOCK
--------------------------------
io.BackendPlatformName: imgui_impl_win32
io.BackendRendererName: NULL
io.ConfigFlags: 0x00000040
 DockingEnable
io.ConfigViewportsNoDecoration
io.ConfigInputTextCursorBlink
io.ConfigWindowsResizeFromEdges
io.ConfigMemoryCompactTimer = 60.0
io.BackendFlags: 0x00000C06
 HasMouseCursors
 HasSetMousePos
 PlatformHasViewports
 HasMouseHoveredViewport
--------------------------------
io.Fonts: 1 fonts, Flags: 0x00000000, TexSize: 512,64
io.DisplaySize: 2560.00,1387.00
io.DisplayFramebufferScale: 1.00,1.00
--------------------------------
style.WindowPadding: 8.00,8.00
style.WindowBorderSize: 1.00
style.FramePadding: 4.00,3.00
style.FrameRounding: 0.00
style.FrameBorderSize: 0.00
style.ItemSpacing: 8.00,4.00
style.ItemInnerSpacing: 4.00,4.00

My Issue/Question:

When using the Tables API, I have noticed that the BordersOuterV flag causes on off-by-one pixel rendering error on the inner vertical borders. I can reproduce this in my own app by stripping it down to just the demo window with no custom fonts or anything, but I can't reproduce it with the examples, so I am assuming it's an issue with my renderer? I figure the scissor logic could be wrong, and I did fix something after re-reading it, but the issue remains. If it's related to clipping, I don't see similar issues anywhere else.

Note that the vertical line seems to be staying the same size when toggling the BordersOuterV flag. The bottom horizontal border actually comes up by one pixel.

With BordersOuterV
BordersOuterV_one_pixel_off

Without BordersOuterV
NoBordersOuterV_just_fine

Off By One
off_by_one_pixel

The IMGUI OpenGL Rendering Code


extern void
renderer_render_imgui(Renderer* r, ImDrawData* drawData)
{
    OpenGL_Renderer* renderer = r->renderer;
    ImGui_Resources& imgui    = renderer->imgui;

    // Avoid rendering when minimized, scale coordinates for retina displays (screen coordinates != framebuffer coordinates)
    ImGuiIO& io  = ImGui::GetIO();
    int fbWidth  = (int)(drawData->DisplaySize.x * io.DisplayFramebufferScale.x);
    int fbHeight = (int)(drawData->DisplaySize.y * io.DisplayFramebufferScale.y);
    if (fbWidth <= 0 || fbHeight <= 0)
        return;

    drawData->ScaleClipRects(io.DisplayFramebufferScale);

    glEnable(GL_SCISSOR_TEST);
    glDisable(GL_CULL_FACE);
    glDisable(GL_DEPTH_TEST);
    //glDisable(GL_FRAMEBUFFER_SRGB);
    glPolygonMode(GL_FRONT_AND_BACK, GL_FILL);

    glUseProgram(imgui.program.id);
    glUniform1i(imgui.program.texture, 0);

    v2 topLeft      = drawData->DisplayPos;
    v2 bottomRight  = (v2)drawData->DisplayPos + (v2)drawData->DisplaySize;
    mat4 projection = glm::ortho(topLeft.x, bottomRight.x, bottomRight.y, topLeft.y);
    glUniformMatrix4fv(imgui.program.projectionMatrix, 1, GL_FALSE, glm::value_ptr(projection));

    glBindVertexArray(imgui.vao);

    glBindBuffer(GL_ARRAY_BUFFER, imgui.vertexBuffer);
    glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, imgui.indexBuffer);

    glVertexAttribPointer(0, 2, GL_FLOAT,         GL_FALSE, sizeof(ImDrawVert), (void*)offsetof(ImDrawVert, pos));
    glVertexAttribPointer(1, 2, GL_FLOAT,         GL_FALSE, sizeof(ImDrawVert), (void*)offsetof(ImDrawVert, uv));
    glVertexAttribPointer(2, 4, GL_UNSIGNED_BYTE, GL_TRUE,  sizeof(ImDrawVert), (void*)offsetof(ImDrawVert, col));

    for (int listI = 0; listI < drawData->CmdListsCount; listI++) {
        const ImDrawList* cmdList      = drawData->CmdLists[listI];
        const ImDrawVert* vertexBuffer = cmdList->VtxBuffer.Data;
        const ImDrawIdx*  indexBuffer  = cmdList->IdxBuffer.Data;

        glBufferData(GL_ARRAY_BUFFER, cmdList->VtxBuffer.Size * sizeof(ImDrawVert), vertexBuffer, GL_STREAM_DRAW);
        glBufferData(GL_ELEMENT_ARRAY_BUFFER, cmdList->IdxBuffer.Size * sizeof(ImDrawIdx), indexBuffer, GL_STREAM_DRAW);

        const ImDrawIdx* indexBufferOffset = 0;

        for (int cmdI = 0; cmdI < cmdList->CmdBuffer.Size; cmdI++) {
            const ImDrawCmd* cmd = &cmdList->CmdBuffer[cmdI];
            if (cmd->UserCallback) {
                cmd->UserCallback(cmdList, cmd);
            }
            else {
                v4 clipRect = v4(cmd->ClipRect.x - topLeft.x, cmd->ClipRect.y - topLeft.y, cmd->ClipRect.z - topLeft.x, cmd->ClipRect.w - topLeft.y);

                if (clipRect.x < fbWidth && clipRect.y < fbHeight && clipRect.z >= 0.0f && clipRect.w >= 0.0f) {
                    // NOTE(blake): from IMGUI docs:
                    // In the interest of supporting multi-viewport applications in the future (see 'viewport' branch on github),
                    // always subtract draw_data->DisplayPos from clipping bounds to convert them to your viewport space.
                    // Note that pcmd->ClipRect contains Min+Max bounds. Some graphics API may use Min+Max, other may use Min+Size (size being Max-Min)
                    glScissor((int)clipRect.x, (int)(fbHeight - clipRect.w),
                              (int)(clipRect.z - clipRect.x), (int)(clipRect.w - clipRect.y));

                    GLuint texture = (GLuint)(umm)cmd->TextureId;
                    glActiveTexture(GL_TEXTURE0);
                    glBindTexture(GL_TEXTURE_2D, texture);

                    // NOTE(blake): from IMGUI docs:
                    // Render 'pcmd->ElemCount/3' indexed triangles.
                    // By default the indices ImDrawIdx are 16-bits, you can change them to 32-bits in imconfig.h if your engine doesn't support 16-bits indices.
                    constexpr GLenum kIndexType = sizeof(ImDrawIdx) == 2 ? GL_UNSIGNED_SHORT : GL_UNSIGNED_INT;
                    glDrawElements(GL_TRIANGLES, cmd->ElemCount, kIndexType, indexBufferOffset);
                }
            }

            indexBufferOffset += cmd->ElemCount;
        }
    }

    glBindTexture(GL_TEXTURE_2D, 0);
    glBindVertexArray(0);
    glBindBuffer(GL_ARRAY_BUFFER, 0);
    glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);
    glUseProgram(0);

    //glEnable(GL_FRAMEBUFFER_SRGB);
    //glEnable(GL_DEPTH_TEST);
    //glEnable(GL_CULL_FACE);
    glDisable(GL_SCISSOR_TEST);
}

@rationalcoder
Copy link
Author

I have looked at lines produced in the draw lists for the vertical table borders, and they end on half-pixels. Is this intended? At any rate, it looks like something this low-priority and not reproducible with examples is unlikely to get much attention. That being said, I can't build the OpenGL example on win32 right now, so maybe that should be checked first?

@ocornut ocornut reopened this Apr 6, 2021
@v-ein
Copy link

v-ein commented Sep 8, 2023

This might be caused by the same bug as #6765. Notice how the behavior differs between "full" outer border (a rectangle is used) vs. horizontal-only (rendered as two lines). More details can be found in my comments to #6765.

@ocornut
Copy link
Owner

ocornut commented Sep 11, 2023

Hello,

I investigated this and your fix @v-ein seems right, pushed it as a340718. Thank you!

Apologies I didn't investigate this and #3752 earlier: some of the border code in tables is subtly tricky because it closely relates to cliprect matching and ImDrawCmd merging constraints. However making this specific changes passes my regression tests in ImGuiTestSuite and didn't alter ImDrawCmd count.

While investigating this I also spotted another long-time bug there in some situations the top-most outer border would be drawn twice, once with TableBorderLight (incorrectly), once with TableBorderStrong (correctly). The earlier would show underneath when TableBorderStrong has alpha under 1.0f. This is fixed with 5a483c2.

@ocornut ocornut closed this as completed Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants