Mismatch of screen coordinates between rendering backends

[wolfpld]

Consider the following code and its results:

ImGui::Begin( "Pixel test" );
auto draw = ImGui::GetWindowDrawList();
const auto wpos = ImGui::GetCursorScreenPos();
draw->AddRect( ImVec2( wpos.x + 0, wpos.y + 0 ), ImVec2( wpos.x + 10, wpos.y + 10 ), 0xFFFFFFFF );
draw->AddLine( ImVec2( wpos.x + 2, wpos.y + 2 ), ImVec2( wpos.x + 3,  wpos.y + 2 ), 0xFF0000FF );
draw->AddLine( ImVec2( wpos.x + 4, wpos.y + 2 ), ImVec2( wpos.x + 8,  wpos.y + 2 ), 0xFF00FF00 );
draw->AddLine( ImVec2( wpos.x + 2, wpos.y + 3 ), ImVec2( wpos.x + 2,  wpos.y + 7 ), 0xFF00FFFF );
ImGui::End();

The image above shows image reference on the left, with blue markers to help with counting pixels, the result of using OpenGL 2/3 backends in the center, and the result of using DirectX 9/10/11 and Vulkan backends on the right.

There are some interesting observations here:

1. The rectangle is specified as ranging from pixels 0 to 10, but is only drawn in the 0-9 range. I'd say this is most likely a pain point for many people, especially newcomers, but is probably quickly handwaved as an "off-by-one" mistake, fixed, and never thought about anymore.
2. All three lines are one pixel short of their specified length. This is expected, to prevent overdraw of start and end points of lines in a strip.
3. Selection of which end point should be skipped in vertical lines is different between the OpenGL backends and other backends. Looking at the specified positions and symmetries between horizontal and vertical lines it is clear that the DirectX/Vulkan rendering is correct and OpenGL is off.

The issues specified in point 3 make it impossible to freely switch the rendering backends, as the rendering output of the client application will differ.

I think the most obvious place to start looking for a solution would be near the handling of the mirrored Y axis, required to support the OpenGL coordinate system.

Special care must be taken to not break already existing applications. Unifying the rendering will change the results of programs relying on the way things are working at the moment. The current behavior should be still available through an ImGui configuration option.

[ocornut]

Also linking to #2441 which discuss a similar issue. I am sorry I haven't got to look into this in depth.

From your description it seems like 1-2 are the same issue, started off from my (perhaps non-standard) definition of the coordinate system, where I would expect expect that passing a WIDTH (max.x - min.x) or HEIGHT (max.y - min.x) of 10 would lead to covering 10 pixels on that axis. This is maybe not in line with other or more standard graphics api coordinate systems? I am genuinely not sure what this is what I'd assume from your comment.

Note the extra awkward definition of AddLine()

void ImDrawList::AddLine(const ImVec2& p1, const ImVec2& p2, ImU32 col, float thickness)
{
    if ((col & IM_COL32_A_MASK) == 0)
        return;
    PathLineTo(p1 + ImVec2(0.50f, 0.50f));
    PathLineTo(p2 + ImVec2(0.50f, 0.50f));
    PathStroke(col, false, thickness);
}

And not how for those specific rounded values, dumbly replacing all 0.50f by 0.49f seemingly makes behavior matching between OpenGL and other renderers. Since ImDrawList has been mostly used to render neat pixel-aligned shapes I suspect there has been a lot of handwaving and sweeping under the rug involved. Maybe under the present circumstance tweaking those offsets would make a difference.

[wolfpld]

Also linking to #2441 which discuss a similar issue.

I think this is (at the low level) the same issue (concerning points 1 and 2).

From your description it seems like 1-2 are the same issue, started off from my (perhaps non-standard) definition of the coordinate system, where I would expect expect that passing a WIDTH (max.x - min.x) or HEIGHT (max.y - min.x) of 10 would lead to covering 10 pixels on that axis.

There are two conflicting ways of thinking about it. Let's assume we're drawing a line from pixel 0 to pixel 1. Mathematically the distance is 1-0=1, so only one pixel should be lit, as you wrote. But you could also treat is as an implicit "put pixel" operation, in which case you're touching two pixels.

This is maybe not in line with other or more standard graphics api coordinate systems?

I don't know. However, the diamond exit rule mentioned in #2441 is important. I've been bitten by an similar issue, as described here: https://github.com/HuuugeGames/Tools/blob/master/docs/surfsplit.txt

And not how for those specific rounded values, dumbly replacing all 0.50f by 0.49f seemingly makes behavior matching between OpenGL and other renderers.

Ok, I will try that. Thanks!

[wolfpld]

This doesn't work as expected. Adding a small y offset to the line (so that the -0.01 change is nullified) makes things mismatched again. This naturally occurs when drawing graphs, with ranges scaled to pixel values.

I can workaround this by rounding the y coordinate in vertical lines, but it's not a general case solution.

Also notice that when a 0.25 offset is added to all y values, the horizontal lines are properly blended between pixels, but the vertical line is not.

Adding more geometry at start and end of a path might be a way to fix this issue.

[eliasdaler]

I've encountered the issue similar to this, I believe. When I render 10px thick lines at integer coordinates, I get this:

Looks like anti-aliasing to me, which is most likely caused by rounding errors.

[wolfpld]

Thinking about this some more, it may be worthwhile to check if ANGLE (https://chromium.googlesource.com/angle/angle) has some solution for this.

[ShironekoBen]

I've been looking into this a bit, building some tests, and it's quite a deep rabbit hole... I don't have answers for everything yet, but a few thoughts:

Firstly, it's not exactly a "clever" solution, but as a band-aid for line rasterisation woes right now, you can do worse than changing ImDrawList::AddLine() to this:

void ImDrawList::AddLine(const ImVec2& p1, const ImVec2& p2, ImU32 col, float thickness)
{
    if ((col & IM_COL32_A_MASK) == 0)
        return;

    ImVec2 line_draw_offset(0.5f, 0.5f);
#ifdef IMGUI_USE_OPENGL_RASTERISATION_WORKAROUND
    line_draw_offset.y -= 0.002f; // Very small additional offset to make line rasterisation under OpenGL match other APIs
#endif

    PathLineTo(p1 + line_draw_offset);
    PathLineTo(p2 + line_draw_offset);
    PathStroke(col, false, thickness);
}

...and then setting IMGUI_USE_OPENGL_RASTERISATION_WORKAROUND from imconfig.h when using an OpenGL backend. This seems to resolve all of the inter-API inconsistencies I've found thus far by forcing the rasterisation behavior to "tie-break" in the right direction on OpenGL.

(you can also achieve the same effect by adding the offset to the OpenGL projection matrix parameters, which has the bonus that it affects all rendering and thus might potentially fix some other cases that don't involve AddLine(), and the downside that it affects all rendering and thus might potentially break some other cases that don't involve AddLine(). Also, it's in the backend and thus harder to change!)

The diamond exit rule is great, and would solve a lot of our line-related problems... except we can't use it because ultimately we're emitting a triangle list and thus all our primitives use triangle rasterisation rules :-( Sadly I don't think we're going to be able to fix that any time soon as the logistics of emitting "real" line primitives are problematic on a lot of levels.

Angle is a good thought! I had a bit of a look through the code, but couldn't spot anything that looked like it was trying to address this problem... I'm not actually sure they're in a position to fix it in the same manner as we've been discussing here though, as it looks like Angle is operating at too low a level to actually have sufficient data to understand the matrices/coordinates involved. I couldn't see any magic vendor extensions or hint flags that might have provided a behind-the-curtain route to a fix, either. If anyone does know if/how they're tackling it I'd be very happy to be proven wrong, though.

Non-integer-pixel rendering looks like an even bigger can of worms that I haven't properly investigated yet... I'll post some more information if/when I get some.

[ocornut]

Thank you Ben for looking into this!

(you can also achieve the same effect by adding the offset to the OpenGL projection matrix parameters, which has the bonus that it affects all rendering and thus might potentially fix some other cases that don't involve AddLine(), and the downside that it affects all rendering and thus might potentially break some other cases that don't involve AddLine(). Also, it's in the backend and thus harder to change!)

FYI I am absolutely fine with making change in the back-end, especially if we promote it nicely in release notes and with the interactive checker below. But that's assuming it doesn't break other things as well and it looks like modifying AddLine()/AddRect() may be safer.

On the minutia of implementing this, considering any drawing path will pull from _Data->TexUvWhitePixel we can store that offset in that same memory region so it's resolved dynamically, and back-end can instruct dear imgui to set that value. I don't imagine it is a difference we'd be able to measure?

Pasting capture from a test Ben wrote (will aim to make them avail)