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

Flooring of clip rectangle on negative monitor coordinates cuts off content #6861

Closed
alektron opened this issue Sep 25, 2023 · 7 comments
Closed

Comments

@alektron
Copy link

Version/Branch of Dear ImGui:

Version: 1.90 WIP (18992)
Branch: docking

Back-end/Renderer/Compiler/OS

Back-ends: (tested with) example_win32_directx11, example_win32_opengl3, custom backend
Operating System: Windows 10

Issue:
On a setup with multiple monitors, content inside an imgui child window gets clipped on the left side of the clipping rectangle if the viewport/platform window is positioned on a monitor with negative X coordinates.

On Windows, when using multiple monitors, one monitor is always set as the 'main monitor'. Other monitors can then be positioned left, right, above or below. All monitors together use one big coordinate system with the origin being in the top left corner of the main monitor (at least on all my systems/setups). On a setup where there is a monitor left to the main monitor, most coordinates used by Windows will become negative on the x axis. This includes e.g. mouse position or window position.

If we move our window to such a monitor with negative x coordinates, imgui items that are very close to the clipping rectangle border can be cut off as seen here:

image

The effect is most notable with frame borders enabled (and a high contrast) but to a sharp eye it is also visible on widgets with rounded frames. The text is also affected.

  ImGui::PushStyleVar(ImGuiStyleVar_FrameBorderSize, 1);
  ImGui::Begin("Hello, world!"); 
  ImGui::BeginChild("Child", ImVec2(0, 0), false);
  int v;
  ImGui::InputInt("TestInput", &v);
  ImGui::Text("Window pos X: %f", ImGui::GetWindowPos().x);
  ImGui::EndChild();
  ImGui::End();
  ImGui::PopStyleVar();

A possible monitor setup is the one below as seen in the Windows 10 desktop settings.
The platform window of the imgui application would have to be on screen one with this setup.
image

My best guess is that internally the clipping rectangle coordinates get floored which in the worst case usually shifts the left clipping rectangle border <1px to the left. However for negative floats this will shift the left border <1px to the right.

The culprit seems to be the code below (imgui.cpp ImGui::Begin(), Line: 7157) where the clipping rectangle gets floored.
There is probably a smarter way to solve this than adding a check for negative coordinates but I'm not sure.

float top_border_size = (((flags & ImGuiWindowFlags_MenuBar) || !(flags & ImGuiWindowFlags_NoTitleBar)) ? style.FrameBorderSize : window->WindowBorderSize);
window->InnerClipRect.Min.x = ImFloor(0.5f + window->InnerRect.Min.x + ImMax(ImFloor(window->WindowPadding.x * 0.5f), window->WindowBorderSize));
window->InnerClipRect.Min.y = ImFloor(0.5f + window->InnerRect.Min.y + top_border_size);
window->InnerClipRect.Max.x = ImFloor(0.5f + window->InnerRect.Max.x - ImMax(ImFloor(window->WindowPadding.x * 0.5f), window->WindowBorderSize));
window->InnerClipRect.Max.y = ImFloor(0.5f + window->InnerRect.Max.y - window->WindowBorderSize);
window->InnerClipRect.ClipWithFull(host_rect);
@ocornut
Copy link
Owner

ocornut commented Sep 25, 2023

Looks like this is the the same #2884

I've been a little stubbornly looking into implementing the full solution of remapping viewport coordinates, but I think we should be able to at least implement the quick-and-incomplete fix for InnerClipRect suggested in both issues. It would at least reduce one visible manifestation of the problem, but please note this is a wider problem than just this.

@ocornut
Copy link
Owner

ocornut commented Sep 25, 2023

Could you try replacing those 4 outer ImFloor() calls with ImFloorSigned() and see if it seems to be fixing the most noticeable issue at least?

@alektron
Copy link
Author

Could you try replacing those 4 outer ImFloor() calls with ImFloorSigned() and see if it seems to be fixing the most noticeable issue at least?

It does indeed seem to solve the problem in the example and our application where it appeared in quite a few places.
I can't see any new artifacts introduces by this change but if there are any, they are probably equally subtle and situational.

ocornut added a commit that referenced this issue Sep 25, 2023
@ocornut
Copy link
Owner

ocornut commented Sep 25, 2023

I have pushed c418685 for this.

I currently don't have a multi-monitor setup anymore but I believe it'll be a good test case to add to the ImGui Test Suite using mock viewport emulation, I'll work on a test for this later (simulate negative coordinate viewport, verify dimensions of some rectangles).

Thanks for pushing this again and making a nice bug report.

@GamingMinds-DanielC
Copy link
Contributor

So basically ImFloorSigned is an implementation of floorf while ImFloor is truncf. Same for the macro definition IM_FLOOR, that's also a truncf. Since they are all in imgui_internal.h and not part of the normal interface, would it make sense to rename them to match the standard functions (so ImFloor, ImTrunc and IM_TRUNC)?

@ocornut
Copy link
Owner

ocornut commented Sep 26, 2023

This is a good point. You'd be surprised to find out I forgot about the truncf() function all-together and this altered the naming.

In principle as it could be a frequently called function in user code, I'd be cautious to rename this even though it is in imgui_internal.h, but it's highly unlikely that a change in behavior of ImFloor() (from trunc to actual floor) would actually matters, so I'm going to try doing this.

@ocornut
Copy link
Owner

ocornut commented Sep 26, 2023

ImGui:

  • pushed 94da584
  • merged docking, rebased string-view, range_select. all passing tests even with IMGUI_DISABLE_OBSOLETE_FUNCTIONS).
  • reviewed uses of ImFloor and ImFloorSigned in those branches and applied same renames as 94da584.

ImPlot:

imgui-node-editor:

  • Seems ok. Some uses of ImFloor() which don't seem detrimental to switch to new behavior.

@ocornut ocornut closed this as completed Sep 26, 2023
ocornut added a commit that referenced this issue May 3, 2024
…6861, #2884, followed by rename 94da584)

Since negative windows can never be visibile in master it didn't show as a difference.
ocornut pushed a commit that referenced this issue May 3, 2024
…y one when window is located on a monitor with negative coordinates. (#6861, #2884)
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