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

Rendering validation errors should result in black screen, not a crash #4457

Closed
Wumpf opened this issue Dec 7, 2023 · 0 comments · Fixed by #4509
Closed

Rendering validation errors should result in black screen, not a crash #4457

Wumpf opened this issue Dec 7, 2023 · 0 comments · Fixed by #4509
Assignees
Labels
💣 crash crash, deadlock/freeze, do-no-start 🔺 re_renderer affects re_renderer itself
Milestone

Comments

@Wumpf
Copy link
Member

Wumpf commented Dec 7, 2023

We already install a wgpu validation error callback for shader reloading. But it seems we still forward to the default panic based handler in many cases.

Generally, ANY validation error during the rendering of a spaceview or setting up graphics data should result in an error message and/or black screen, but not a crash ever.

-> Make sure this is the case in all circumstances, no matter if shader reloading is on or off (this has a considerable effect on this)

Extra: Figure out if we're able to trace errors back to space views so we can draw with egui to the space view in following frames that it is in an invalid state. Essentially allowing space views to enter a "crashed" state

@Wumpf Wumpf added 🔺 re_renderer affects re_renderer itself 💣 crash crash, deadlock/freeze, do-no-start labels Dec 7, 2023
@Wumpf Wumpf added this to the 0.12 milestone Dec 7, 2023
@Wumpf Wumpf self-assigned this Dec 12, 2023
Wumpf added a commit that referenced this issue Dec 13, 2023
… errors (#4509)

### What

* Fixes #4457
* solves the crash part of #4455
* but keeping it open because I still want to check on the error message


There's a bunch of things that probably should have been separate PRs
but by the time I ventured there it got too hard to entangle, so please
bear with me and read this detailed list of changes carefully (if you
understand this list you've done half the review ;-)):

* re_renderer now uses `cfg_aliases` and `cfg_if` in various places to
make cfg decisions more readable
* `ErrorTracker` is now used in all build configurations. Previously, it
was only active on debug native builds
* The first thing `RendererContext` does now is to install global error
handlers that feed into the "top level" `ErrorTracker`. Previously this
was done delayed and only for debug+native
* `ErrorTracker` never panics now
* `ErrorTracker` no longer uses a frame counting heuristic (!!) to
discard _all_ errors, but *instead* relies on precise device timeline
frame counters to discard individual errors
* this fixes issues where previously an error would disappear and
reappear without being logged
* as commented several times in the code: Keep in mind the three
staggered timelines of WebGPU
       * `Content` (your code)
       * `Device` (everything happening on wgpu::Device)
* `Queue` (everything happening on the GPU, represented by wgpu::Queue)
* `Content` and `Device` are mostly in sync on wgpu-core, but NOT on
WebGPU in general!
* split out the wgpu-core specific parts of `ErrorTracker` into its own
module and fall back to simple string hashing for WebGPU
* on WebGPU we're dealing with a JS interface that doesn't provide these
types. Additionally, in a pure WebGPU build
(the only webgpu enabled build we have today), we don't even depend on
wgpu-core!
* Manual test of re_renderer samples shows that we now no longer crash
on WebGPU either!
* the wgpu-core error type handling is unchanged except for a change in
label hashing
* previously, in the absence of debug labels, error de-duplication would
fail. In practice leading to error spam on release builds
* Introduce `WgpuErrorScope` utility for safe & fully covering wgpu
error scopes
* The resulting futures are handled with `now_or_never` on wgpu-core
backend since we know this is safe. On WebGPU we hand off execution to
the browser.
* Each frame now has a top level `WgpuErrorScope` in order to catch all
errors
* this is not a _strict_ requirement on wgpu-core backends since
wgpu-core will always feed the fallback error handler. **But Chrome/Dawn
WebGPU does not**. The spec allows for this behavior



<img width="1623" alt="image"
src="https://github.com/rerun-io/rerun/assets/1220815/be42c6cc-a780-4f48-b24c-8f9dc36b872a">
Screenshot of a viewer in release mode with an intentionally broken mesh
shader. Also tried other breakages like render pipeline format
mismatches.



Things related, but NOT covered by this PR:
* crashes on startup / adapter selection
* this requires further improvements to egui: We need to improve the
hooks that go into creating the egui render state. Need to consider
allowing to create your own renderstate. This would also allow us to
move the actual device/queue/adapter ownership to re_renderer
* #4507
* punting on this one. It's a nice-to-have extension of the work
presented here
* close look into #4455
* the crash is almost certainly fixed now and replaced by a recoverable
error 🥳, but I want to understand what sets the wrong viewport - either
the one in egui we already fixed or the ViewBuilder (which surely could
use some safety check)

### 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 the web demo (if applicable):
  * Full build: [app.rerun.io](https://app.rerun.io/pr/4509/index.html)
* Partial build:
[app.rerun.io](https://app.rerun.io/pr/4509/index.html?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
- Useful for quick testing when changes do not affect examples in any
way
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4509)
- [Docs
preview](https://rerun.io/preview/754b7c4109ba687a6192689ef9c8974dd014d770/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/754b7c4109ba687a6192689ef9c8974dd014d770/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
emilk added a commit that referenced this issue May 29, 2024
* Uses: rerun-io/egui_tiles#67
* Uses: lampsitter/egui_commonmark#51
* Split off from #6171
* Closes #5280

### Test
* [x] image loading
* [x] gltf
 
### 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 the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6448?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6448?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/6448)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.

# egui changelog so far

Full diff at https://github.com/emilk/egui/compare/0.27.0..HEAD

#### ecolor
* Fix `hex_color!` macro by re-exporting `color_hex` crate from `ecolor`
[#4372](emilk/egui#4372) (thanks
[@dataphract](https://github.com/dataphract)!)
* Remove `extra_asserts` and `extra_debug_asserts` feature flags
[#4478](emilk/egui#4478)

#### eframe
* Add `register_native_texture` in `eframe::Frame`
[#4246](emilk/egui#4246) (thanks
[@Chaojimengnan](https://github.com/Chaojimengnan)!)
* Early-out from context switching the `glow` backend
[#4284](emilk/egui#4284)
* Fix `ViewportCommand::InnerSize` not resizing viewport on Wayland
(#4211) [#4211](emilk/egui#4211) (thanks
[@rustbasic](https://github.com/rustbasic)!)
* Only avoid glow context switching on Windows
[#4296](emilk/egui#4296)
* Improve IME support with new `Event::Ime`
[#4358](emilk/egui#4358) (thanks
[@rustbasic](https://github.com/rustbasic)!)
* Allow users to create viewports larger than monitor on Windows & macOS
[#4337](emilk/egui#4337) (thanks
[@lopo12123](https://github.com/lopo12123)!)
* Use `objc2` and its framework crates
[#4395](emilk/egui#4395) (thanks
[@madsmtm](https://github.com/madsmtm)!)
* Update to Rust 1.76 [#4411](emilk/egui#4411)
* Egui-winit: emit physical key presses when a non-Latin layout is
active [#4461](emilk/egui#4461) (thanks
[@TicClick](https://github.com/TicClick)!)
* IME for chinese [#4436](emilk/egui#4436)
(thanks [@rustbasic](https://github.com/rustbasic)!)
* Fix : In Windows, the 'egui_demo_app' screen does not appear.
[#4410](emilk/egui#4410) (thanks
[@rustbasic](https://github.com/rustbasic)!)
* Fix: Window position creeps between executions on scaled monitors
[#4443](emilk/egui#4443) (thanks
[@avery-radmacher](https://github.com/avery-radmacher)!)
* Update `image` crate to 0.25
[#4160](emilk/egui#4160)
* Ignore synthetic key presses
[#4514](emilk/egui#4514) (thanks
[@hut](https://github.com/hut)!)
* Fix: still track mouse when dragging outside web canvas
[#4522](emilk/egui#4522)
* Add `NativeOptions::persistence_path`
[#4423](emilk/egui#4423) (thanks
[@lucasmerlin](https://github.com/lucasmerlin)!)
* Use ResizeObserver instead of `resize` event
[#4536](emilk/egui#4536) (thanks
[@jprochazk](https://github.com/jprochazk)!)
* Fix: Don't `.forget()` RAF closure
[#4551](emilk/egui#4551) (thanks
[@jprochazk](https://github.com/jprochazk)!)

#### egui_extras
* Update `image` crate to 0.25
[#4160](emilk/egui#4160)

#### egui_plot
* `Plot::Items:allow_hover` give possibility to masked the interaction
on hovered item [#2558](emilk/egui#2558) (thanks
[@haricot](https://github.com/haricot)!)
* Expose `ClosestElem` and `PlotConfig`
[#4380](emilk/egui#4380) (thanks
[@Narcha](https://github.com/Narcha)!)
* Disable interaction for `ScrollArea` and `Plot` when UI is disabled
[#4457](emilk/egui#4457) (thanks
[@varphone](https://github.com/varphone)!)
* Make sure plot size is positive
[#4429](emilk/egui#4429) (thanks
[@rustbasic](https://github.com/rustbasic)!)
* Introduce lifetime to `egui_plot::Plot` to replace `'static` fields
[#4435](emilk/egui#4435) (thanks
[@Fabus1184](https://github.com/Fabus1184)!)
* Hide all other series when alt-clicking in the legend
[#4549](emilk/egui#4549) (thanks
[@abey79](https://github.com/abey79)!)
* Plot now respects the `interact_radius` set in the UI's style
[#4520](emilk/egui#4520) (thanks
[@YgorSouza](https://github.com/YgorSouza)!)

#### egui_glow
* Enable egui_glow's winit feature on wasm (#4420)
[#4421](emilk/egui#4421) (thanks
[@simon-frankau](https://github.com/simon-frankau)!)

#### egui-wgpu
* Update to wgpu 0.20 [#4433](emilk/egui#4433)
(thanks [@KeKsBoTer](https://github.com/KeKsBoTer)!)
* Revert update to wgpu 0.20 => downgrade to wgpu 0.19.1
[#4559](emilk/egui#4559)

#### egui-winit
* Update `webbrowser` to `v1.0.0`
[#4394](emilk/egui#4394) (thanks
[@torokati44](https://github.com/torokati44)!)
* Emit physical key presses when a non-Latin layout is active
[#4461](emilk/egui#4461) (thanks
[@TicClick](https://github.com/TicClick)!)
* IME for chinese [#4436](emilk/egui#4436)
(thanks [@rustbasic](https://github.com/rustbasic)!)
* Fix: Window position creeps between executions on scaled monitors
[#4443](emilk/egui#4443) (thanks
[@avery-radmacher](https://github.com/avery-radmacher)!)
* Ignore synthetic key presses
[#4514](emilk/egui#4514) (thanks
[@hut](https://github.com/hut)!)

#### egui
* Improve the UI for changing the egui theme
[#4257](emilk/egui#4257)
* Change the resize cursor when you reach the resize limit
[#4275](emilk/egui#4275)
* Make `TextEdit` an atomic widget
[#4276](emilk/egui#4276)
* Overload operators for `Rect + Margin`, `Rect - Margin` etc
[#4277](emilk/egui#4277)
* Implement blinking text cursor in `TextEdit`
[#4279](emilk/egui#4279)
* Rename `fn scroll2` to `fn scroll`
[#4282](emilk/egui#4282)
* Change `Frame::multiply_with_opacity` to multiply in gamma space
[#4283](emilk/egui#4283)
* Support order on windows
[#4301](emilk/egui#4301) (thanks
[@alexparlett](https://github.com/alexparlett)!)
* Fix wrong replacement function in deprecation notice of
`drag_released*` [#4314](emilk/egui#4314)
(thanks [@sornas](https://github.com/sornas)!)
* Consider layer transform when positioning text agent
[#4319](emilk/egui#4319) (thanks
[@juancampa](https://github.com/juancampa)!)
* Fix incorrect line breaks
[#4377](emilk/egui#4377) (thanks
[@juancampa](https://github.com/juancampa)!)
* Fix `hex_color!` macro by re-exporting `color_hex` crate from `ecolor`
[#4372](emilk/egui#4372) (thanks
[@dataphract](https://github.com/dataphract)!)
* Change `Ui::allocate_painter` to inherit properties from `Ui`
[#4343](emilk/egui#4343) (thanks
[@varphone](https://github.com/varphone)!)
* Use parent `Ui`s style for popups
[#4325](emilk/egui#4325) (thanks
[@alexparlett](https://github.com/alexparlett)!)
* Fix : take `rounding` into account when using `Slider::trailing_fill`
[#4308](emilk/egui#4308) (thanks
[@rustbasic](https://github.com/rustbasic)!)
* Add a way to specify Undoer settings and construct Undoers more easily
[#4357](emilk/egui#4357) (thanks
[@valadaptive](https://github.com/valadaptive)!)
* Add xtask crate [#4293](emilk/egui#4293)
(thanks [@YgorSouza](https://github.com/YgorSouza)!)
* Add `ViewportCommand::RequestCut`, `RequestCopy` and `RequestPaste` to
trigger Clipboard actions
[#4035](emilk/egui#4035) (thanks
[@bu5hm4nn](https://github.com/bu5hm4nn)!)
* Fix `Panel` incorrect size
[#4351](emilk/egui#4351) (thanks
[@zhatuokun](https://github.com/zhatuokun)!)
* Improve IME support with new `Event::Ime`
[#4358](emilk/egui#4358) (thanks
[@rustbasic](https://github.com/rustbasic)!)
* Allow users to create viewports larger than monitor on Windows & macOS
[#4337](emilk/egui#4337) (thanks
[@lopo12123](https://github.com/lopo12123)!)
* Added ability to define colors at UV coordinates along a path
[#4353](emilk/egui#4353) (thanks
[@murl-digital](https://github.com/murl-digital)!)
* Eframe: update ViewportBuilder.with_icon() documentation
[#4408](emilk/egui#4408) (thanks
[@roccoblues](https://github.com/roccoblues)!)
* Update to Rust 1.76 [#4411](emilk/egui#4411)
* Add a `Display` impl for `Vec2`, `Pos2`, and `Rect`
[#4428](emilk/egui#4428) (thanks
[@tgross35](https://github.com/tgross35)!)
* Remove `extra_asserts` and `extra_debug_asserts` feature flags
[#4478](emilk/egui#4478)
* Egui-winit: emit physical key presses when a non-Latin layout is
active [#4461](emilk/egui#4461) (thanks
[@TicClick](https://github.com/TicClick)!)
* Disable interaction for `ScrollArea` and `Plot` when UI is disabled
[#4457](emilk/egui#4457) (thanks
[@varphone](https://github.com/varphone)!)
* Update ahash 0.8.6 -> 0.8.11
[#4507](emilk/egui#4507) (thanks
[@hellodword](https://github.com/hellodword)!)
* `include_image!` now accepts expressions
[#4521](emilk/egui#4521) (thanks
[@YgorSouza](https://github.com/YgorSouza)!)
* Remove `Event::Scroll` and handle it in egui
[#4524](emilk/egui#4524)
* Remove scroll latency for smooth trackpads
[#4526](emilk/egui#4526)
* Smooth out zooming with discreet scroll wheel
[#4530](emilk/egui#4530)
* Add `Options::line_scroll_speed` and `scroll_zoom_speed`
[#4532](emilk/egui#4532)
* Don't panic when replacement glyph is not found
[#4542](emilk/egui#4542) (thanks
[@RyanBluth](https://github.com/RyanBluth)!)
* Make `TextEdit::return_key` optional
[#4543](emilk/egui#4543) (thanks
[@doonv](https://github.com/doonv)!)
* Add `TextEdit::hint_text_font`
[#4517](emilk/egui#4517) (thanks
[@zaaarf](https://github.com/zaaarf)!)
* Add `Options::reduce_texture_memory` to free up RAM
[#4431](emilk/egui#4431) (thanks
[@varphone](https://github.com/varphone)!)
* Fix `Ui::scroll_with_delta` only scrolling if the `ScrollArea` is
focused [#4303](emilk/egui#4303) (thanks
[@lucasmerlin](https://github.com/lucasmerlin)!)
* Add support for text truncation to `egui::Style`
[#4556](emilk/egui#4556) (thanks
[@abey79](https://github.com/abey79)!)
* Hide toolip when opening `ComboBox` drop-down
[#4546](emilk/egui#4546) (thanks
[@abey79](https://github.com/abey79)!)
* Better spacing and sizes for (menu) buttons
[#4558](emilk/egui#4558)

#### epaint
* Add `RectShape::blur_width` to implement shadows
[#4267](emilk/egui#4267)
* Overload operators for `Rect + Margin`, `Rect - Margin` etc
[#4277](emilk/egui#4277)
* Fix incorrect line breaks
[#4377](emilk/egui#4377) (thanks
[@juancampa](https://github.com/juancampa)!)
* Fix `hex_color!` macro by re-exporting `color_hex` crate from `ecolor`
[#4372](emilk/egui#4372) (thanks
[@dataphract](https://github.com/dataphract)!)
* Add `emath::OrderedFloat` (moved from `epaint::util::OrderedFloat`)
[#4389](emilk/egui#4389)
* Added ability to define colors at UV coordinates along a path
[#4353](emilk/egui#4353) (thanks
[@murl-digital](https://github.com/murl-digital)!)
* Add a `Display` impl for `Vec2`, `Pos2`, and `Rect`
[#4428](emilk/egui#4428) (thanks
[@tgross35](https://github.com/tgross35)!)
* Remove `extra_asserts` and `extra_debug_asserts` feature flags
[#4478](emilk/egui#4478)
* Make `epaint::mutex::RwLock` allow `?Sized` types
[#4485](emilk/egui#4485) (thanks
[@crumblingstatue](https://github.com/crumblingstatue)!)
* Update ahash 0.8.6 -> 0.8.11
[#4507](emilk/egui#4507) (thanks
[@hellodword](https://github.com/hellodword)!)
* Don't panic when replacement glyph is not found
[#4542](emilk/egui#4542) (thanks
[@RyanBluth](https://github.com/RyanBluth)!)

---------

Co-authored-by: Antoine Beyeler <[email protected]>
Co-authored-by: Andreas Reich <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💣 crash crash, deadlock/freeze, do-no-start 🔺 re_renderer affects re_renderer itself
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant