imgui icon indicating copy to clipboard operation
imgui copied to clipboard

fix ImGui_ImplOSX_GetWindowSize and ConvertNSRect

Open sivu opened this issue 3 years ago • 3 comments

small fixes that i think are just copy-paste errors.

sivu avatar Dec 20 '22 09:12 sivu

Hello Mikko,

Thanks for the PR. I have now merged the first commit as 16aaf60.

As for the second commit (9ffb69b23): It is possible for mousePoint to not be in the rectangle of any screen? If e.g. mouse behavior are configured to extend outside of bounds while dragging (not sure how but assuming it is possible)

I admit am not sure I understand the intent/logic, the whole thing with flipping Y coordinates being a little confusing in this backend. In Dear ImGui view we have a single "large" coordinate space, where typically the primary monitor starts at (0,0) on Windows, and negative values are in theory (*) OK, e.g. screen left-of or above primary monitor on Windows.

Say you have two vertically stacked screens on Windows, top screen goes e.g from 0 to 1080 on Y axis, bottom screen goes from 1080 to 2160. I imagine (but I have not verified) that on Mac the top screen would go from 2160 to 1080, and bottom screen from 1080 to 0. Therefore perhaps the height we should up to do this flipping is the MAX value of all monitor. I am right? (I am not so familiar with OSX even less-so with multi-monitor OSX).

(*) in practice there's a small issues with negative coordinates but it is unrelated to the main matter here. At some point we will offset virtual space to stay in positive coordinates.

Happy new year!

ocornut avatar Jan 02 '23 14:01 ocornut

As for the second commit (https://github.com/ocornut/imgui/commit/9ffb69b236f682098935bf7fc4e16cdfbbd959af): It is possible for mousePoint to not be in the rectangle of any screen? If e.g. mouse behavior are configured to extend outside of bounds while dragging (not sure how but assuming it is possible)

To my knowledge it is not possible for the mouse coordinates be outside of any screen, they are always clamped to screen boundaries.

I admit am not sure I understand the intent/logic, the whole thing with flipping Y coordinates being a little confusing in this backend. In Dear ImGui view we have a single "large" coordinate space, where typically the primary monitor starts at (0,0) on Windows, and negative values are in theory (*) OK, e.g. screen left-of or above primary monitor on Windows.

The coordinate system is very annoying in OSX, I think everybody agrees on that :) On OSX the main screen (internal display on laptop for example) is at 0,0 and all other displays are relative to that. I think this is how it also is on Windows. And even if you have two screens that have exact same resolution side-by-side, you can position them so that other is higher than the other. I have a setup where my external display is on the left side of my laptop, so the external display is in negative coordinates. Screenshot 2023-01-03 at 15 50 17

And you are right, my fix only worked for me in my use-case. We need to go through all monitors and find the screen that is most top-left and use that.

sivu avatar Jan 03 '23 13:01 sivu

Thank you for confirming.

And you are right, my fix only worked for me in my use-case. We need to go through all monitors and find the screen that is most top-left and use that.

In addition, it would be worth confirming in Metrics/Debugger->Viewports that the monitors are correctly positioned.

static void ImGui_ImplOSX_UpdateMonitors()
{
    ImGuiPlatformIO& platform_io = ImGui::GetPlatformIO();
    platform_io.Monitors.resize(0);

    for (NSScreen* screen in NSScreen.screens)
    {
        NSRect frame = screen.frame;
        NSRect visibleFrame = screen.visibleFrame;

        ImGuiPlatformMonitor imgui_monitor;
        imgui_monitor.MainPos = ImVec2(frame.origin.x, frame.origin.y);
        imgui_monitor.MainSize = ImVec2(frame.size.width, frame.size.height);
        imgui_monitor.WorkPos = ImVec2(visibleFrame.origin.x, visibleFrame.origin.y);
        imgui_monitor.WorkSize = ImVec2(visibleFrame.size.width, visibleFrame.size.height);
        imgui_monitor.DpiScale = screen.backingScaleFactor;

        platform_io.Monitors.push_back(imgui_monitor);
    }
}

Multi-viewports for this backend was added by 6868d116 + d666a1d4 (#4821, #2778) but I'm unsure it was testing with multiple-monitors.

According to https://developer.apple.com/documentation/corefoundation/cgrect "In the default Core Graphics coordinate space, the origin is located in the lower-left corner of the rectangle and the rectangle extends towards the upper-right corner. If the context has a flipped-coordinate space—often the case on iOS—the origin is in the upper-left corner and the rectangle extends towards the lower-right corner." May be better to use NSMinX(), NSMinY(). NSWidth(), NSHeight().

ocornut avatar Jan 03 '23 14:01 ocornut

I am currently on a Mac and trying to catch up with a bunch of related issues with the OSX native backend. (e.g. #6432, #7028, #7101), at the sluggish pace of a Windows user on a Mac.

It seems that everyone has a different workaround for some isolated problem but at core maybe out of sanity we should perhaps mimic SDL and GLFW in transforming coordinates so they match between all backends, starting from monitor coordinates. At the moment with the OSX backend I see different monitor coordinates as the SDL and GLFW ones. I don't think it is reasonable to apply isolated fixes from others # because we standardize that a little better.

ocornut avatar Jan 09 '24 19:01 ocornut

Agreed, I am closing this

sivu avatar Jan 09 '24 19:01 sivu

Keeping open as all OSX related references are useful until this is solved.

ocornut avatar Jan 09 '24 19:01 ocornut

I think things are now generally fixed with a683033e4 but it's a little difficult to reason about this, since the patch fixes others things leading to this AFAIK not being needed. At least I tested OSX backend with multi-viewports and multi-monitors in various positions and it worked (checked monitors positions, mouse position relative to what's displayed, etc.)

(few remaining issue see https://github.com/ocornut/imgui/issues/7028#issuecomment-1883845945 but generally usable)

ocornut avatar Jan 09 '24 21:01 ocornut