LukasBanana / LLGL

Low Level Graphics Library (LLGL) is a thin abstraction layer for the modern graphics APIs OpenGL, Direct3D, Vulkan, and Metal
BSD 3-Clause "New" or "Revised" License
2.05k stars 139 forks source link

Correctly handling Retina displays on the Mac #60

Closed beldenfox closed 3 years ago

beldenfox commented 4 years ago

I’ve been working on getting Retina support into LLGL. The new code is not done and requires a lot of clean-up but the changes involve re-thinking what SetVideoMode does so I thought you should take a look sooner rather than later.

In Retina mode the size of the surface and the size of the drawables diverge. An NSWindow is sized in a space that roughly approximates 72 dpi and then the drawables are sized upward to a match the actual dpi of the screen. So I needed to alter SetVideoMode to resize the drawables independently of the surface size. I started by stubbing out Window::AdjustForVideoMode and all of the fullscreen handling (more on fullscreen later).

Then I added Surface::GetPreferredResolution() so a surface can advertise a preferred resolution that differs from GetContentSize(). Finally I updated OnSetVideoMode for both Metal and OpenGL to resize the drawables appropriately. And that was pretty much it beyond tweaking Info.plist and updating the sample code a bit.

Basically I’ve pared down SetVideoMode to only update the size of the drawables, roughly equivalent to ResizeBuffer in DirectX. This eliminates the possibility that calling SetVideoMode could generate a resize event while handling a resize event. It also eliminates the code that was trying to re-center the window on every resize event which was making live re-resizing impossible on the Mac.

(BTW, I tweaked the DirectX versions of OnSetVideoMode to allow arbitrary sizes to match the Mac. On Windows the main use would be to to reduce the size of the drawables for performance reasons.)

The next step is to get fullscreen working again. Previously SetVideoMode was called at both ends of the process, first to kick the system into fullscreen mode and then to handle the resulting resize event. I want to avoid that recursion by introducing new RenderContext routines for entering and exiting fullscreen mode. Then clean-up, including adding some capability flags.

LukasBanana commented 4 years ago

So far I was only manually adding the HighResolution attribute to the .plist files when testing something on Mac locally, but that was of course only a temporary solution. You seem to be more experienced with the Retina display support on the Mac. That being said, some of your changes seem to be quite temporary, too, like commenting out members from interface structures such as fullscreen. I can take a closer look over the weekend, but if I understand you correctly, you're not quite done yet anyway. Nevertheless, I'm happy to see someone working on those missing features in that project.

beldenfox commented 4 years ago

I should have made it clearer that this pull request was really an invitation for a design review on SetVideoMode. I set aside the fullscreen logic to focus on two specific parts of the design.

The first issue is the recursive nature of SetVideoMode. It is being used to both invoke changes to the surface AND to respond to the resize events that those changes generate. This has some odd side-effects. For example, every time the window is resized the code tries to re-center it (the path is WM_SIZE => PostResize => SetVideoMode => Window::AdjustForVideoMode => Window::SetDesc with the ‘centered’ flag sent.) This only works on Windows because there’s a bug in Win32Window::SetDesc; it ignores the ‘centered’ flag unless the descriptor also changes the window’s size or position.

There are comments in Win32Window::SetDesc that suggest that SetDesc was generating WM_SIZE events that were leading back to SetDesc. This may have been aggravated by DirectX 12’s fullscreen implementation; entering fullscreen not only calls SetDesc it also leads to Direct3D’s SetFullscreenState call, both of which could generate more WM_SIZE events. At some point you tweaked the code to terminate the recursion by ensuring that SetDesc never sends out a PostResize message. I ran into that this weekend; I added some code that called SetDesc and the window resized without ever calling PostResize.

There are similar problems on the Mac. The code is trying to work around the re-centering problem by postponing the PostResize call until the user has stopped dragging. But when the user releases the mouse PostResize and SetVideoMode are finally called and try to re-center the window. Currently the Mac ignores the ‘centered’ flag and there’s a TODO comment about cleaning this up. In theory the Mac should also have recursion problems; a call to SetDesc will generate a resize event that will end up calling SetDesc again. I haven’t tested that but it’s in the design.

The cleanest way to fix all of this is to handle a resize event using code that updates the back buffers without touching the surface. As an experiment I pared down SetVideoMode so that’s all it does. That’s why I stubbed out fullscreen handling and Window::AdjustForVideoMode.

I also stubbed out fullscreen because there are even more recursion issues in the Direct3D backends. As of right now entering fullscreen using OnSetVideoMode in the Direct3D backends can generate resize events and those resize events are then handed back to OnSetVideoMode. I think that’s causing some issues but am still researching.

I will generate a pull request that tries to leave SetVideoMode alone as much as possible as a front-end call for going into and out of fullscreen mode. I will introduce a new back-end routine (e.g. RenderContext::ResizeBuffers or some such) just for handling WM_RESIZE events. I will try to keep the existing fullscreen logic but it’s complicated.

The second issue is support for arbitrarily sized back buffers. On the Mac I altered SetVideoMode to allow the back buffers to be sized independently of the surface size (which is necessary for Retina). Now the question is whether to carry the changes over to Direct3D as well. The use case on Windows is when an application wants to reduce the back buffer size because it’s running into fill rate issues in windowed mode on a high-resolution display (e.g. 4K). This would also be useful for apps that want to support borderless fullscreen windowed mode but run at a lower resolution than the display. In general this is a feature of DirectX with known use cases and I think LLGL should find a way to support it.

LukasBanana commented 4 years ago

Thanks for your detailed description, it makes a lot of things more clearer to me now. It sounds like you already did quite some investigation on that feature, so I'm happy to leave some of the new design choices up to you. You are right, the recursion in SetVideoMode is a weak design and it probably makes sense to split that into two functions.

LukasBanana commented 3 years ago

My apologies that I never took the time to do the design review you asked for, but I don't actively work on this project anymore. If there is no update from your side, I'm going to close this PR.