Improved wgpu callbacks #3253
Improved wgpu callbacks #3253
Conversation
|
@MatrixDev, want to have a look if this fixes the issue for you? Doesn't address glow backend, but we can take that separately |
| @@ -817,7 +817,7 @@ pub struct PaintCallback { | |||
| /// | |||
| /// The rendering backend is also responsible for restoring any state, such as the bound shader | |||
| /// program, vertex array, etc. | |||
| pub callback: Arc<dyn Any + Sync + Send>, | |||
| pub callback: Arc<dyn Any + Send + Sync>, | |||
There was a problem hiding this comment.
accidental reordering of traits due to intra-pr-historical reasons ;)
Wow, this is some great work and it will fully cover my usecase. The only thing I'm not sure about is |
|
Thanks for you feedback! :) |
### What * Depends on emilk/egui#3253 * allows for a bit nicer callback handling on our side. Still not amazing but quite a bit better I reckon (no more locks and additional ref counting on our side!). I was hoping to use the new, more flexible `shared_paint_callback_resources` to store `ViewBuilder`, but it can't be accessed without a handle to the eframe renderer. The end goal would be to get rid of re_renderer's `per_frame_data_helper`. This will likely be enabled with wgpu's "Arcanization" effort as this will eliminate `wgpu::RenderPass`'s lifetime dependency. In that case we could store a `Mutex<ViewBuilder>` on `ReRendererCallback` 🤔 * For the forseeable future we'll have to enable `fragile-send-sync-non-atomic-wasm` on wgpu (luckily egui itself no longer has this restriction). The problem is that a lot of our infrastructure is built around assuming `Send`/`Sync` whereas in actuality webgpu rendering resources can't be shared accross threads easily. * Had to (discover and) work around https://github.com/gfx-rs/naga/issues/2436 ### Checklist * [x] I have read and agree to [Contributor Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and the [Code of Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md) * [x] I've included a screenshot or gif (if applicable) * [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/2980) (if applicable) * [x] Test WebGPU build - [PR Build Summary](https://build.rerun.io/pr/2980) - [Docs preview](https://rerun.io/preview/pr%3Aandreas%2Fupdate-wgpu/docs) - [Examples preview](https://rerun.io/preview/pr%3Aandreas%2Fupdate-wgpu/examples)
egui_wgpu::CallbackTraitCallbackTrait::finish_preparecalled after all prepare callbacks have been calledSend/Syncon wasmCallbackFnredesign #3163I followed @MatrixDev's proposal fairly closely. Key differences:
finish_prepare_callbackthat is not part of the callback trait. For convenience it still gets references to all callback objects. This technically makes the regularpreparecallbacks redundant, but I figured it's convenient to have both around. I think @MatrixDev correctly describes the typical scenario as each individualpreparepushing to some form of system/manager and then processing the overall results in a `finish_prepareTypeMapand replaced with integer hash map since:I ventured a little bit into trying to change egui's callback system on a more deeper level as hinted in the ticket, but I concluded the only way forward there would be to define separate callback data on the
Shape(always sync+send, immutable, comparable etc.) and separately callback "systems". I think it should be easier to transition there with this PR in the future but this would be a much more far-reaching change.