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
Quad example doesn't work on OS X #2563
Comments
Interesting, so it looks like we keep losing the surface (or at least the Metal backend thinks we do) and re-create the swapchain. I've been only running in 10.13 and 10.14 during development, and I wonder if there is a big difference with regards to |
I think that doing an OS upgrade is just papering over a race condition in the code by reordering operations. The issue is down in window.rs in this bit of code:
frame.drawable gets coughed up as None, and that sets off a whole chain of events. I'm not convinced that None is always an error. (ie. can it be None because the drawable is currently queued for rendering but hasn't been unlinked yet?). I'm also a little concerned by the fact that the code presents three frames in quick succession for no obvious reason (no resize, no mouse event, etc) -- it presents frame 1, frame 0, frame 1, and frame 0 at which point it hiccups and requests a new swap chain. Unfortunately, this is getting far to deep for my beginner level knowledge of Rust. |
That's pretty good analysis! Let me read up the code once more and describe what the logic is supposed to be.
… On Jan 10, 2019, at 19:36, buzmeg ***@***.***> wrote:
I think that doing an OS upgrade is just papering over a race condition in the code by reordering operations.
The issue is down in window.rs in this bit of code:
println!("Frame: {:?}", frame.drawable);
match frame.drawable.take() {
Some(drawable) => {
frame.available = true;
Ok(drawable)
}
None => {
frame.linked = false;
Err(())
}
}
frame.drawable gets coughed up as None, and that sets off a whole chain of events.
I'm not convinced that None is always an error. (ie. can it be None because the drawable is currently queued for rendering but hasn't been unlinked yet?). I'm also a little concerned by the fact that the code presents three frames in quick succession for no obvious reason (no resize, no mouse event, etc) -- it presents frame 1, frame 0, frame 1, and frame 0 at which point it hiccups and requests a new swap chain.
Unfortunately, this is getting far to deep for my beginner level knowledge of Rust.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.
|
and to clarify, I'm not suggesting an upgrade. I'm just excusing myself for not seeing this because I was running 10.13+ for a long time.
The reason for it being implemented this way is that we need to return So what Metal wants us to do is: get new frame, render it, return it, etc. But what we are actually doing - pretending that the frames are known, and when the next frame is returned by CoreAnimation, we compare it to all the ones we have to figure out what index it is. This is hacky, but it's the only solution I could see, and it seems to work in most cases (Dota2, VkQuake, Filament, etc). If we don't invalidate the drawable on |
2619: [mtl] fixed surface size, better presentation debug messages r=grovesNL a=kvark Fixes Dota in portability Also can help with #2563, but some testing is needed to confirm PR checklist: - [ ] `make` succeeds (on *nix) - [ ] `make reftests` succeeds - [ ] tested examples with the following backends: - [ ] `rustfmt` run on changed code Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
Is this related to the swapchain issue in wgpu? |
@seivan could be. We'll know for sure when this issue is resolved (pending confirmation). |
Just tested today. This problem is still extant. |
@buzmeg sorry about not addressing this. Rolling back an OS version on Mac is rather trouble-some. I hope to get my hands on a device with 10.12 installed to investigate. If anyone has that and can look into it, that would be great! |
Mstange has 10.12
…On Thu, Mar 21, 2019, 10:46 PM Dzmitry Malyshau ***@***.***> wrote:
@buzmeg <https://github.com/buzmeg> sorry about not addressing this.
Rolling back an OS version on Mac is rather trouble-some. I hope to get my
hands on a device with 10.12 installed to investigate. If anyone has that
and can look into it, that would be great!
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#2563 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAUTbQWmHRCUnhRx1bO56zzljs8eaZ2Fks5vZEQIgaJpZM4Z5Fi->
.
|
Just wanted to report that this example doesn't even compile on macOS 10.11:
I tried commenting out those functions in
|
@datadog23 thank you! Signposts should be easy to disable on OSX prior to 10.12. They are not required to function. gfx/src/backend/metal/src/native.rs Line 897 in 1ede669
|
@datadog23 Also to work around specialization constants (at least temporarily), I think you should be able to change:
to
If this doesn't work you might need to adjust the quad vertex shader too. |
@kvark I don't know what the purpose of the signposts is, they seem to be used for swapchain debugging in @grovesNL Changing line 546 gets me the same symptoms as the original issue above, a blank white window with this in the terminal:
..the last line repeating forever. I don't even understand why a 'specialization constant' was used (assuming it's the same as this?), there's only a single float in the vertex shader and it's a 2D scaling factor. If it needs to change why not pass a uniform, instead of having to recompile the shader? I'll have another look when I understand more about Vulkan. |
I think we'd ideally be able to put the externs for signposts and anywhere they're used behind
Thanks for the update, that's useful to know.
Yeah it's just used to demonstrate how to use specialization constants in gfx-hal. In this case there's no real reason the value couldn't be hardcoded into the shader or provided as a uniform instead. |
For signposts, the easiest way to proceed would be covering all of them in a new feature ( |
I don't know how to add optional features like that: although Line 25 in ETA: I've just found "The Cargo Book", that might help! |
@datadog23 you can check auto-capture feature for an example. |
I don't know how to submit edits. How would one enable the signpost (or auto-capture) feature, btw? |
@datadog23 you can file a pill request here: click "Fork" on Github, check out the fork, make you change in a branch, then push into your fork branch. A button will appear here to create a PR from it.
I think this should work: cargo run --bin quad --features metal,gfx-backend-metal/signpost |
Just pulled gfx again. Still not working with same SwapchainConfig message. As a side note, the vulkan version fails on OS X:
Thanks. |
Maybe because OSX doesn't have Vulkan? |
I was puzzled by this too -- the MoltenVK examples all run, so why doesn't gfx-hal just fall back to that? I was expecting invoking |
Um, then, perchance you want to ask the LunarG guys why they have a Vulkan SDK available for download for OS X here: I will open a separate issue rather than continuing to clutter this one as gfx still won't run the demos on my OS X machine either with Metal or Vulkan. Separate issue opened: |
Vulkan is not natively available on Apple systems. LunarG SDK is just a piece of software that's not even tightly controlled by the Vulkan working group.
That is to say, if users want to run their applications via that SDK, they are free to do so. I don't see agood reason why we'd need to support it in the examples though.
… On Jun 3, 2019, at 05:21, buzmeg ***@***.***> wrote:
Um, then, perchance you want to ask the LunarG guys why they have a Vulkan SDK available for download for OS X here:
https://vulkan.lunarg.com/sdk/home
I will open a separate issue rather than continuing to clutter this one as gfx still won't run the demos on my OS X machine either with Metal or Vulkan.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Ran into this issue as well on MacOS: 10.14.5
I originally found this on my own renderer and thought I'd gotten something wrong. Tried it on the quad example as a sanity check. Resulted in the same issue and then I found this GitHub issue. |
Thanks for the info @bigmstone ! We don't have a fix in the process, but we are looking at the issue. |
@kvark Yeah, I can check -- My renderer is a side project for me so I won't be able to dedicate a normal amount of time to it. I will look into the root cause and see if I can drum something up when I'm working on this. (Mostly nights/weekends) Re: wgpu-rs examples -- they appear to be working as expected. Frame-rate of |
Okay, I had a bit of a chance to look at this over the last couple of nights. Here's where I'm at/what I'm seeing. Disclaimer: I'm a bit ignorant to pretty much all things graphics...so...I might be getting all this wrong. 😅 So I traced back through a typical failure from the logs. When we submit a queue we don't return a @buzmeg observed I opened with the initial error happening during queue submission. During queue submission the trace is:
During
Note: I've removed everything from the frame except the frame's texture to make it easier to parse. As you can tell the texture isn't anywhere in the swapchain's frames, and since they don't match we (silently) fail and move on to presentation. Since we never had a chance to set the drawable of the frame Interestingly when I inspect this with Xcode's profiler (just found this, and wowah it's pretty cool) I see this strange So the question I'm asking myself now is, "Why are these textures different?" So I'll continue from there unless someone has any insights on if I'm mistaken or know the issue given my observations. |
This is some great investigation @bigmstone ! The logic of the metal backend is by far not simple, and you were able to get most of the details correctly.
We do a dummy present on the first grabbed image of the new swapchain here. Is that the one you are seeing?
The main problem with our approach today is that the way the internal I wish we had a more robust approach here. The only known alternative is what MoltenVK is doing: dererred-recording of all the command buffers and resolving frame textures at the time a command buffer is conceptually submitted - this is where it starts being translated into the native metal command buffer... I don't want to go this path, since I consider immediate recording to be one of our strongest advantage and a differentiating factor. Some more info on immediate vs deferred is on our blog. If you have any ideas on how we can make this better, or sketch out a completely different scheme, that would help tremendously. |
I think we'd be best off sketching out a completely different scheme. My first instinct is to modify A problem with this is that we need to know the swapchain images in advance for us to create the framebuffers in Vulkan (as for each swapchain image we require a framebuffer where it's used as an attachment). I'm not sure what the best approach here would be. Could gfx-hal manage framebuffers for you automatically? This would involve adding new parameters to swapchain creation for information about the render pass and attachments for each framebuffer, and perhaps a variant could be used for attachments that refer to the current swapchain image. Does any of this make sense? |
@aleksijuvani it does make sense, and this would indeed be (rather trivially) the least common denominator API between the backends. Main problem is Vulkan Portability, which is a big important use case for us. Metal model doesn't map to it very well (as we can see here...). One way forward could be for gfx-hal to provide both was of managing the swapchain. Just like Vulkan swapchain API is just an extension, we could have multiple variants supported (Vulkan and Metal styles). There are still some details missing about how the Metal model would work, given that the rest of the API is Vulkan-like. Another possibility that I just thought of is to have gfx-hal only provide the Metal swapchain model, while the awkwardness of Vulkan -> Metal translation in this area is simply moved from our Metal backend to gfx-portability. Having gfx-portability run on Vulkan would mean a lot of extra work then, but nobody needs that path anyway. Again, there are still unknowns about how gfx-portability DX backends would work with this, but at lest we'll have full control of what happens implicitly in the backends, so that we can stitch it smoothly with gfx-portability. |
2764: Start turning empty backend into basic mock r=kvark a=omni-viral Few types at a time. PR checklist: - [ ] `make` succeeds (on *nix) - [ ] `make reftests` succeeds - [ ] tested examples with the following backends: - [ ] `rustfmt` run on changed code 2788: kdebug_signpost* fns don't exist on macOS 10.11 r=kvark a=datadog23 > Fixes #issue Doesn't fix, but found during #2563 > PR checklist: > - [ ] `make` succeeds (on *nix) Yes. > - [ ] `make reftests` succeeds No, as it seems to be trying to compile with the feature this patch adds ('signpost') turned on? > - [ ] tested examples with the following backends: metal > - [ ] `rustfmt` run on changed code I don't know how, but I tried to copy the existing style. 2834: [gl] Enable debug output for native GL r=kvark a=cormac-obrien Will fix #2826 when grovesNL/glow#20 lands. PR checklist: - [ ] `make` succeeds (on *nix) - [ ] `make reftests` succeeds - [x] tested examples with the following backends: gl - [x] `rustfmt` run on changed code 2870: [mtl] override argument buffer sets r=grovesNL a=kvark Makes argument buffers compatible with push constants again. Also reduce the binding of argument buffers to only the affected stages. PR checklist: - [ ] `make` succeeds (on *nix) - [ ] `make reftests` succeeds - [ ] tested examples with the following backends: - [ ] `rustfmt` run on changed code I think this brings us to the end of the second push for argument buffers. They seem pretty solid and operational as of today, vkQuake is a good test case. There are serious limitations though that ask for big changes: 1. Storage images and dynamic-offset buffers can't be a part of the argument buffer - #2866 . This requires shader translation changes. 2. We need to know the dimensionality and msaa-ness of textures at the descriptor set layout creation time. This would mean another derivation from Vulkan proper. Once we have these implemented, we'll be able to benchmark Dota and Dolphin again. But until then, argument buffer support will be kept as an experimental feature disabled by default. Co-authored-by: Zakarum <scareaangel@gmail.com> Co-authored-by: datadog23 <datadog23@subluminal.info> Co-authored-by: Mac O'Brien <cormac@c-obrien.org> Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
2788: kdebug_signpost* fns don't exist on macOS 10.11 r=kvark a=datadog23 > Fixes #issue Doesn't fix, but found during #2563 > PR checklist: > - [ ] `make` succeeds (on *nix) Yes. > - [ ] `make reftests` succeeds No, as it seems to be trying to compile with the feature this patch adds ('signpost') turned on? > - [ ] tested examples with the following backends: metal > - [ ] `rustfmt` run on changed code I don't know how, but I tried to copy the existing style. Co-authored-by: datadog23 <datadog23@subluminal.info>
2788: kdebug_signpost* fns don't exist on macOS 10.11 r=compiler a=datadog23 > Fixes #issue Doesn't fix, but found during #2563 > PR checklist: > - [ ] `make` succeeds (on *nix) Yes. > - [ ] `make reftests` succeeds No, as it seems to be trying to compile with the feature this patch adds ('signpost') turned on? > - [ ] tested examples with the following backends: metal > - [ ] `rustfmt` run on changed code I don't know how, but I tried to copy the existing style. Co-authored-by: datadog23 <datadog23@subluminal.info> Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
$ cargo run --bin quad --features=metal
Produces a grey window and nothing else
Short info header:
$git log
commit 6c3c1d3
Merge: f8b74e2 86c5d35
Author: bors[bot] <bors[bot]@users.noreply.github.com>
Date: Wed Jan 9 17:45:47 2019 +0000
examples$ cargo run --bin quad --features=metal
Finished dev [unoptimized + debuginfo] target(s) in 0.20s
Running
/Users/andrewl/rust/gfx/target/debug/quad
AdapterInfo { name: "Intel(R) Iris(TM) Graphics 6100", vendor: 0, device: 0, device_type: DiscreteGpu }
Memory types: [MemoryType { properties: DEVICE_LOCAL, heap_index: 0 }, MemoryType { properties: COHERENT | CPU_VISIBLE, heap_index: 1 }, MemoryType { properties: DEVICE_LOCAL | CPU_VISIBLE, heap_index: 1 }, MemoryType { properties: DEVICE_LOCAL | CPU_VISIBLE | CPU_CACHED, heap_index: 1 }]
formats: Some([Bgra8Unorm, Bgra8Srgb, Rgba16Float])
SwapchainConfig { present_mode: Fifo, composite_alpha: Inherit, format: Bgra8Srgb, extent: Extent2D { width: 1024, height: 768 }, image_count: 2, image_layers: 1, image_usage: COLOR_ATTACHMENT }
SwapchainConfig { present_mode: Fifo, composite_alpha: Inherit, format: Bgra8Srgb, extent: Extent2D { width: 1024, height: 768 }, image_count: 2, image_layers: 1, image_usage: COLOR_ATTACHMENT }
SwapchainConfig { present_mode: Fifo, composite_alpha: Inherit, format: Bgra8Srgb, extent: Extent2D { width: 1024, height: 768 }, image_count: 2, image_layers: 1, image_usage: COLOR_ATTACHMENT }
SwapchainConfig { present_mode: Fifo, composite_alpha: Inherit, format: Bgra8Srgb, extent: Extent2D { width: 1024, height: 768 }, image_count: 2, image_layers: 1, image_usage: COLOR_ATTACHMENT }
SwapchainConfig { present_mode: Fifo, composite_alpha: Inherit, format: Bgra8Srgb, extent: Extent2D { width: 1024, height: 768 }, image_count: 2, image_layers: 1, image_usage: COLOR_ATTACHMENT }
[lines keep repeating infinitely]
The text was updated successfully, but these errors were encountered: