Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Metal validation errors when running piet-gpu in the Xcode debugger #166

Closed
vincentisambart opened this issue Apr 29, 2022 · 2 comments
Closed

Comments

@vincentisambart
Copy link
Contributor

When trying to run piet-gpu in the Xcode debugger, I got the following error:

validateComputeFunctionArguments:745: failed assertion `Compute Function(main0): missing buffer binding at index 25 for spvBufferSizeConstants[0].'

And indeed if you look at for example binning.msl, you have a parameter constant uint* spvBufferSizeConstants [[buffer(25)]] even though there doesn't seem to be any buffer assigned to 25 in metal.rs's fn dispatch().
Having this fixed would make investigating problems on macOS easier.

As I asked on zulip, it seems to be related to the use of the --msl-decoration-binding flag passed to SPIRV-Cross.

By the way here's a step-by-step way to run the winit app in the Xcode debugger:

  • Build the app with cargo build.
  • Open Xcode.
  • In Xcode's menu, select Debug > Debug Executable..., and select the winit executable in the file selection dialog. It should be in the target/debug subdirectory of your copy of the repo.
  • The scheme editor shows up. In the Options tab in "Working directory" turn on "Use custom working directory" and enter the directory you want to run the app from, for example the piet-gpu subdirectory of your copy of the repo. Then in the "Arguments" tab, in "Arguments Passed On Launch" click on the "+" and enter the argument you want to pass, for example "Ghostscript_Tiger.svg".
  • Press "Close". To edit the settings afterwards, in Xcode's menu select Product > Scheme > Edit Scheme... (Cmd+<)
  • To start the app in the debugger, click on the play button on the top left, or Cmd+R.
@vincentisambart
Copy link
Contributor Author

Based on the zulip conversation, and looking at the SPIRV-Cross and MoltenVK source code, I made up a quick and dirty change to fn dispatch() and rerun the app in the Xcode debugger. I was then told by Xcode that a framebuffer-only texture should not be used as the destination of a texture copy so I also fixed that, and with these 2 changes no more validation error in Xcode. Here's the diff:

diff --git a/piet-gpu-hal/src/metal.rs b/piet-gpu-hal/src/metal.rs
index e3157d4..0c3b1a1 100644
--- a/piet-gpu-hal/src/metal.rs
+++ b/piet-gpu-hal/src/metal.rs
@@ -136,6 +136,7 @@ impl MtlInstance {
             let () = msg_send![ns_view, setWantsLayer: YES];
             let bounds: CGRect = msg_send![ns_view, bounds];
             let () = msg_send![metal_layer, setFrame: bounds];
+            metal_layer.set_framebuffer_only(false);
 
             if !ns_window.is_null() {
                 let scale_factor: CGFloat = msg_send![ns_window, backingScaleFactor];
@@ -464,6 +465,13 @@ impl crate::backend::CmdBuf<MtlDevice> for CmdBuf {
             encoder.set_texture(img_ix, Some(&image.texture));
             img_ix += 1;
         }
+        const BUFFER_SIZE_BUFFER_INDEX: usize = 25;
+        let buffer_size_constants: [u32; 1] = [0];
+        encoder.set_bytes(
+            BUFFER_SIZE_BUFFER_INDEX as NSUInteger,
+            mem::size_of_val(&buffer_size_constants) as NSUInteger,
+            &buffer_size_constants as *const u32 as _,
+        );
         let workgroup_count = metal::MTLSize {
             width: workgroup_count.0 as u64,
             height: workgroup_count.1 as u64,

Note that even without validation error, on a M1 Mac you still get incorrect colors, so #154 still needs to be fixed.

In regards to the diff above, I'm pretty sure set_framebuffer_only(false) is needed (as I also needed it when investigating #154), at least with the current way the texture is handled.
Binding a buffer to 25 is also needed, but you might want to only do it in the pipelines that require it. And looking at MoltenVK it seems -[MTLComputeCommandEncoder setBytes:length:atIndex:] might not work on some old GPUs (though these GPUs might already not be supported by piet-gpu). Also I don't fully understand what that value is needed for, so is really 0 the correct value?

@vincentisambart
Copy link
Contributor Author

With vello now using WGPU as a backend this is not relevant anymore.
And just in case I tried running the current winit example in the Xcode debugger and it did not trigger any assertions, so closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant