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

Error when quickly resizing viewer with 3D view #4455

Closed
roym899 opened this issue Dec 7, 2023 · 2 comments · Fixed by #5189
Closed

Error when quickly resizing viewer with 3D view #4455

roym899 opened this issue Dec 7, 2023 · 2 comments · Fixed by #5189
Assignees
Labels
🪳 bug Something isn't working 🏎️ Quick Issue Can be fixed in a few hours or less 🔺 re_renderer affects re_renderer itself 📺 re_viewer affects re_viewer itself

Comments

@roym899
Copy link
Collaborator

roym899 commented Dec 7, 2023

Describe the bug
When resizing the viewer for a little while it eventually crashes with the following error

2023-12-07T10:44:52Z ERROR wgpu::backend::direct] Handling wgpu errors as fatal by default

thread 'ThreadId(1)' panicked at 'wgpu error: Validation Error

Caused by:
    In a RenderPass
      note: encoder = `encoder`
    In a set_viewport command
    Viewport has invalid rect Rect { x: 224.0, y: 72.0, w: 932.0, h: 496.0 }; origin and/or size is less than or equal to 0, and/or is not contained in the render target Extent3d { width: 1055, height: 631, depth_or_array_layers: 1 }
2023-12-07.12-00-15.mp4

To Reproduce
Open the viewer and continuously resize for a few seconds. I think what matters is that it is fast (I guess too fast for my computer to handle something, wouldn't be surprised if this is harder to reproduce on better hardware or different OS). This is the code I use here, but I don't think it matters:

import numpy as np
import rerun as rr

rr.init("collapsing", spawn=True)
rr.log("parent/p1", rr.Points3D(np.random.rand(10,3)))
rr.log("parent/p2", rr.Points3D(np.random.rand(10,3)))
rr.log("parent/p3", rr.Points3D(np.random.rand(10,3)))
rr.log("parent/p4", rr.Points3D(np.random.rand(10,3)))
rr.log("parent/p5", rr.Points3D(np.random.rand(10,3)))
rr.log("parent/p6", rr.Points3D(np.random.rand(10,3)))

Desktop (please complete the following information):

  • OS: Ubuntu 20.04

Rerun version

rerun_py 0.12.0-alpha.1+dev [rustc 1.74.0 (79e9716c9 2023-11-13), LLVM 17.0.4] x86_64-unknown-linux-gnu
@roym899 roym899 added 🪳 bug Something isn't working 👀 needs triage This issue needs to be triaged by the Rerun team 💣 crash crash, deadlock/freeze, do-no-start 📺 re_viewer affects re_viewer itself labels Dec 7, 2023
@Wumpf
Copy link
Member

Wumpf commented Dec 7, 2023

We fixed the same error in egui prior to release of latest version. Unclear if it's still the same or a similar thing on our side (we do have a set_viewport call that could make the same mistake)

@Wumpf Wumpf removed the 👀 needs triage This issue needs to be triaged by the Rerun team label Dec 7, 2023
@Wumpf Wumpf added this to the 0.12 milestone Dec 7, 2023
@emilk emilk added the 🔺 re_renderer affects re_renderer itself label Dec 13, 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)
@Wumpf
Copy link
Member

Wumpf commented Dec 14, 2023

confirmed that this no longer crashes, but only logs a (instant recoverable) error 🥳
still should figure out though what keeps setting the wrong viewport on resize!

@Wumpf Wumpf removed the 💣 crash crash, deadlock/freeze, do-no-start label Dec 14, 2023
@Wumpf Wumpf changed the title Crash when quickly resizing viewer with 3D view ~Crash~ Error when quickly resizing viewer with 3D view Dec 14, 2023
@emilk emilk changed the title ~Crash~ Error when quickly resizing viewer with 3D view Error when quickly resizing viewer with 3D view Dec 19, 2023
@emilk emilk removed this from the 0.12 milestone Jan 2, 2024
@Wumpf Wumpf added the 🏎️ Quick Issue Can be fixed in a few hours or less label Jan 16, 2024
@Wumpf Wumpf self-assigned this Feb 13, 2024
Wumpf added a commit that referenced this issue Feb 13, 2024
### What

* Fixes #4455

Got a good repro of this on my windows machine by using the screenshot
action on a low-dpi screen (screenshot action will do a quick resize on
native) while having the pose tracking example open.

### 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 newly built examples:
[app.rerun.io](https://app.rerun.io/pr/5189/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/5189/index.html?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[app.rerun.io](https://app.rerun.io/pr/5189/index.html?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/5189)
- [Docs
preview](https://rerun.io/preview/f54be52f2819874bcec35e91db189eb369a007af/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/f54be52f2819874bcec35e91db189eb369a007af/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
Wumpf added a commit that referenced this issue Feb 21, 2024
### What

* Fixes #4455

Got a good repro of this on my windows machine by using the screenshot
action on a low-dpi screen (screenshot action will do a quick resize on
native) while having the pose tracking example open.

### 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 newly built examples:
[app.rerun.io](https://app.rerun.io/pr/5189/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/5189/index.html?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[app.rerun.io](https://app.rerun.io/pr/5189/index.html?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/5189)
- [Docs
preview](https://rerun.io/preview/f54be52f2819874bcec35e91db189eb369a007af/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/f54be52f2819874bcec35e91db189eb369a007af/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🪳 bug Something isn't working 🏎️ Quick Issue Can be fixed in a few hours or less 🔺 re_renderer affects re_renderer itself 📺 re_viewer affects re_viewer itself
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants