[mtl] present with transaction, optionally

[kvark]

Add a presentation path that forces CoreAnimation transaction.

[scoopr]

So I rewrote the present bit like this

            if image.present_with_transaction {
                command_buffer.commit();
                command_buffer.wait_until_scheduled();
                image.drawable.present();
            } else {
                command_buffer.present_drawable(&image.drawable);
                command_buffer.commit();
            }

Because the apple docs on presentWithTransaction says that you "commit, waitUntilScheduled, present" to ensure that the transaction is available.

This works, in that all frames I get are perfect, but after couple of back-to-back resize events, I get a second long stall which is subpar.

I wonder if I'm flooding it with frames somehow. Funny thing, with this setup, my fps counter goes to 120, disabling the feature and its back to 60.

[scoopr]

Though I get "not annoying anymore, even if technically not perfect" solution was to put this at the start of Surface::new (as mentioned in the Tristan Hume's blog post)

Perhaps winit could do that instead.

        let NSViewLayerContentsPlacementTopLeft = 11;
        if let Some(v) = view {
            unsafe { let () = msg_send![v.as_ptr(), setLayerContentsPlacement:NSViewLayerContentsPlacementTopLeft]; }
        }

[kvark]

@scoopr that seems like an interesting solution without any obvious costs. Want to make a PR for it?

[scoopr]

For whatever reason, setting the layerContentsPlacement in winit side didn't work for me, I wonder if it somehow gets reset when the layer is added to the view.

There is few downsides I've seen so far.. One is a smaller thing, the resizing from left edge is can result in black bars to the right side etc., but its not like something like vscode handles that all that much better.

But a real issue is that the dpi changes (like dragging a window from hires screen to lores) leaves the layer to be completely wrong size. I guess it needs to watch for those notifications and set the backing scale factor. The default scaling behavior handles that fine.

[scoopr]

Just to document my tests,

• The long stall with the present_with_transaction seemed to come from the fact I was presenting from the WindowEvent::Resized, I'm supposing it is sending out more resized events than it is willing to present for the compositor and then runs out of drawables?
• I then thought, maybe setting the CAMetalLayer's needsDisplayOnBoundsChange property to true would then give me Event::RedrawRequested from winit, whenever the compositor feels like its time to redraw, but those event didn't come through, didn't yet investigate why. window.request_redraw() from the Resized event works but is again too late and stretching would occur
• Couldn't really get matching behavior to happen in Tristan's test app

[kvark]

@scoopr I just realized I didn't push the changes to the right branch. The thing that works for me is close to what you describe in #3627 (comment), and it's pushed now.

Am I reading this right that the only concern with the change was having a long stall, and this turned out to be a problem with how you handle Resize, and not something we should be concerned about in gfx?

Also, do you understand why the original code here didn't work as expected? The one with the "scheduled handler", I'd expect it to do the same thing, just more asynchronously.

[scoopr]

Well, if I don't render within the Resized event, then there is no resized frame to display, so the result is perfectly synchronized wrong frame.

Not sure why the scheduled way didn't work, perhaps there is some work done after the final user callback when properly waiting for it 🤷 Does metal call those callbacks straight from the NSRunLoop queue? I guess you could post a dispatch_async on the mainloop so it would run the persent just after the callback as opposed in the callback, dunno.

I think the layer placement should be done in any case, because that makes the error-behaviour to be the least annoying and works in all cases, just need to take care of the small details so the size is correctly tracked.

It would be nice to have the present_with_transaction flow to work as well, maybe by making one of the vsyncing PresentModes use it, or as an additional option. But need to be careful with it so that it wouldn't be a non-obvious perfomance footgun, or document well on the restrictions it might impose.

[kvark]

@scoopr ok, would you want to make a PR for layer placement then?

[scoopr]

On closer inspection

• I thought a recent Mac: Redraw immediately to prevent shaking on window resize rust-windowing/winit#1901 commit would of fixed the resized events, but on closer inspection it is making the redraw request synchronous, not the resized event. I think same kind of change needs to be done for the resized events, and that seemed to work in a quick test. FWIW I'm not getting any redraw requests from the resizing events, I wonder if those don't come through when using CAMetalLayer or some other reason. (I tried to make that happen in a separate test app, and couldn't)
• I don't have my low-dpi monitor at hand right now, can't test the dpi change currently, so the layerPlacement thing isn't ready to go yet. Its a rare edge-case, but that is precisely why I want to test it, as I happened to come across it accidently while testing previously, and I wouldn't want that to regress. It seems you can test hidpi on lodpi screen with quartzdebug, but not vice versa 🤦
• I find it a bit odd that the layer:shouldInheritContentsScale:fromWindow: implementation didn't handle the dpi change when I tested the layerPlacement .. but then I wonder, you set a GfxManagedMetalLayerDelegate as the CAMetalLayer delegate, but that is excepting a CALayerDelegate, but the .. shouldInheritContentsScale.. is part of NSViewLayerContentScaleDelegate. I wonder if that method should actually be part of the NSView implementation? The docs are not super clear on this, but the protocol name perhaps suggests this.

The present_to_transaction implementation in this pull request is I think fine, and I think the option should be exposed for those who want to utilize it. Might warrant some extra testing, as:

• I'm still getting the stalls if I actually render in the Resized event, otherwise it is visually perfect. If I just skip the redraw in resized, it works otherwise correctly, but I'm lagging a frame.
• Why the heck do I get 120fps when I use that, despite whatever I have as the PresentMode?
• Edit: Oh and I don't think the present_to_transaction implementation itself will change all that much despite these questions, and fixes will likely not affect this pull

[scoopr]

Are you going to put this in for 0.8? Its a (small) api change

[kvark]

@scoopr I thought this is internal API for gfx-backend-metal, not gfx-hal exposed. So we could add this API in a non-breaking way at any point.
Anyway, I'm going to make this mergable now.

[kvark]

bors r=scoopr

[bors]

Build succeeded:

• Android Stable
• Dependency Check
• iOS Stable
• MacOS Nightly
• MacOS Stable
• Ubuntu Nightly
• Ubuntu Stable
• Web Assembly
• Windows Nightly
• Windows Stable