https://bugs.winehq.org/show_bug.cgi?id=52354
--- Comment #13 from Tim Clem tclem@codeweavers.com --- (In reply to Charles Davis from comment #10)
(In reply to Tim Clem from comment #9)
Just to clarify, the original build failures were Metal related, but the rendering problems are not. I can reproduce these issues on my 2015 MacBook Pro, which supports Metal.
There are two distinct issues at play here, both of which do not happen in Mojave or later.
- As Brendan noted in the mailing list thread, calling setNeedsDisplay: on
WineContentViews doesn't trigger an updateLayer call. This is supposed to happen if the layer has a layerContentsRedrawPolicy of NSViewLayerContentsRedrawOnSetNeedsDisplay. We don't currently set that, so it must somehow work without the redraw policy on later versions of macOS.
The default for a layer-backed view is NSViewLayerContentsRedrawDuringViewResize, which is a subset of NSViewLayerContentsRedrawOnSetNeedsDisplay.
Ah, right you are. Didn't notice that in the documentation.
However, even with that policy set, updateLayer basically gets called once per window on High Sierra. I can work around this by making WineContentView's setNeedsDisplay: and setNeedsDisplayInRect: manually call the analogous methods on its layer. That makes the window contents draw, and reliably reveals issue #2.
- On retina-capable screens on High Sierra, the contentsScale of
WineContentView's layer is always effectively 2, regardless of Wine's settings. We set the initial contentsScale on the layer when we make WineContentViews, and we return false from -layer:shouldInheritContentsScale:fromWindow: unless retina mode is on, so this should not happen. But somehow, by the time updateLayer is called, the contentsScale is always 2. That's not a scenario that the code expects, so we wind up creating an incorrectly scaled image, and we get the misalignment shown in Dean's screenshot.
According to the docs, that method's associated protocol isn't supported until 10.14. I wonder if it were broken prior to 10.14. Or maybe we shouldn't even say YES in response to the new scale being 1.0--maybe it calls the method once prior to 10.14, and caches the result. Does that help?
I get the same behavior if I just have shouldInheritContentsScale return a constant NO, or remove it entirely. It's never called with a new scale of 1 in my testing.
If I set the contentsScale according to retinaMode in updateLayer, it at least sticks between calls, but the layer still seems to render as if it's 2. The contents are upscaled and blurry.
While it's relatively easy to work around issue #1, issue #2 is more persistent. The best mitigation I've come up with for that involves forcibly rendering the layer's contents at the scale implied by Wine's retina setting (rather than the layer's actual contentsScale). On retina displays that at least gets the view's contents to align correctly, but the upscaling looks horrible.
I think it makes sense to just revert the patches. I'll submit that to wine-devel.
In the long run I'm not sure that layer-backed views are the right direction, if only because updateLayer requires a complete redraw of the view vs. drawRect:'s more precise dirtyRect.
Apple's docs suggest that setting a bitmap into a layer is faster than drawing into it. Since we already draw into a bitmap, I thought this would be an obvious performance win.
Do we have any way to test the performance impact of these sorts of changes? There were some patches on the mailing list a few weeks ago that squeezes significant gains out of the old drawRect approach in one particular app (https://www.winehq.org/pipermail/wine-devel/2022-March/211906.html), but some sort of general benchmarking tool would be useful.
Part of the reason I wanted to move to layer-backed views is that I wanted to use this to support drawing to other processes' windows, using the CARemoteLayer infrastructure. Without layer-backed views, how do you propose we implement drawing to other processes' windows?
I didn't realize that was a motivating factor for these patches. Barring remote layers, I suppose the only other route is sharing an IOSurface. I don't really know the tradeoffs between those options. I assume remote layers are simpler and probably more performant, since they're intended for exactly that scenario.
I'm not at all against the idea of using layers, but we do need some sort of working solution for High Sierra.