Fix jittering during window resize on MacOS for WGPU/Metal

[noelgbl]

This update fixes a widely known jitter issue on Metal/MacOS #1521 that occurs during window resizing. It introduces a new resized_window field in MasonryState to track the state of window resizing. The on_window_resize_state_change method in RenderContext is added to handle the resizing behavior for surfaces.

[PoignardAzur]

This seems fine? I don't have a Mac on hand to test this on, so I'd appreciate if someone could test this.

[noelgbl]

I would need HAL features from Vello's wgpu namespace, so this might become a bigger change. For now, it works like expected on MacOS.

[noelgbl]

Any idea on doing this without touching Vello or completely disabling Vello's default features and using WGPU directly?

[PoignardAzur]

@DJMcNab ?

[jaredoconnell]

I won't approve since CI isn't passing and there is unsafe code, but I can confirm that this fixes the jitter.

[PoignardAzur]

I looked more into that unsafe block and looked up the wgpu side to see how safe this was, and even if it works in practice, I'm not convinced this isn't undefined behavior, though I could be wrong.

Aside from that, I don't think this change is forward-compatible. In the current head commit of wgpu, wgpu::hal::metal::Surface does not publicly expose present_with_transaction. Ordinarily, I wouldn't block a PR on a future dependency, but combined with the unclear safety, it's giving me pause.

[noelgbl]

#8716 simply exposes a different API for setting this flag, which is also part of v28. Unfortunately, this issue cannot be solved without setting this flag, and therefore, unsafe code. Leaving the required change here for WGPU 28 and for future reference.

unsafe {
    if let Some(hal_surface) = surface.surface.as_hal::<wgpu::hal::api::Metal>() {
        let guard = hal_surface.render_layer().lock();
        guard.set_presents_with_transaction(resizing);
    }
}

[PoignardAzur]

(Unrelatedly, wow, googling "cargo metal crate rust" does not give me the results I want.)

[PoignardAzur]

Unfortunately, this issue cannot be solved without setting this flag, and therefore, unsafe code.

The problem isn't unsafe code, it's undefined behavior. Taking a shared reference to a struct, transmuting it to a pointer then a mutable reference, then using it to mutating a field, is (I believe) undefined behavior unless the reference is to an internally mutable type. The compiler might optimize the mutation away in some cases, etc.

I'm more comfortable with the guard.set_presents_with_transaction(true); solution, which AFAICT uses objc's message-sending API.

However, it seems we can't use that API until we switch to wgpu 28, so...

I don't know. Ideologically, there are strong arguments for saying "Undefined behavior: not even once". In this particular case, the unsoundness would be temporary, only affect a single platform, there's no other way to get the feature/bugfix we want, it works on multiple people's computers, and while I know we're not supposed to speculate about the possible blast radius of undefined behavior, realistically I think the worst case behavior is that the bool doesn't get updated and we're back to square one.

I'd appreciate a second opinion on this.

[xStrom]

Can't we just enable the metal feature directly?

Suggested change

	[target.'cfg(target_os = "macos")'.dependencies]
	# Enables masonry_core/default -> vello -> wgpu -> wgpu_hal -> metal,
	# which we need to interact with CoreAnimation during window resizing.
	# This is not a very elegant fix and also enables all other HAL's.
	# Remove this once a feature gating solution is found.
	# See: https://github.com/linebender/xilem/pull/1551
	masonry_core = { workspace = true, features = ["default"] }
	[target.'cfg(target_os = "macos")'.dependencies]
	# We need Metal to interact with CoreAnimation during window resizing.
	wgpu = { version = "27.0.1", default-features = false, features = ["metal"] }

[noelgbl]

Also thought of this, but wanted to avoid having to sync the version with Vello. But you probably know better than me if that's the way to go.

[xStrom]

Avoiding undefined behavior seems like a great goal to me. Frankly I'm surprised that Vello isn't on wgpu 28 already, given that 29 is around the corner. We should see if we could get a Vello release with 28 out.

[noelgbl]

Then let's just wait for v28 if you feel more comfortable with it.