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

Web-view crashes in chrome on linux when shaders are too large #3931

Open
jleibs opened this issue Oct 19, 2023 · 4 comments
Open

Web-view crashes in chrome on linux when shaders are too large #3931

jleibs opened this issue Oct 19, 2023 · 4 comments
Labels
💣 crash crash, deadlock/freeze, do-no-start 🔺 re_renderer affects re_renderer itself 🦟 regression A thing that used to work in an earlier release
Milestone

Comments

@jleibs
Copy link
Member

jleibs commented Oct 19, 2023

Describe the bug
When I try to use app.rerun.io on nightly I get a crash:
image
image

To Reproduce
Go to https://app.rerun.io/version/nightly/
Try to load any example.

Desktop (please complete the following information):

  • Ubuntu 22.10 / Chrome crashes
  • Ubuntu 22.10 / Firefox does not crash but fails to render images.

Rerun version

  • nightly only
    • 0.9.1 works without issue
@jleibs jleibs added 🪳 bug Something isn't working 👀 needs triage This issue needs to be triaged by the Rerun team labels Oct 19, 2023
@jleibs jleibs added this to the 0.10 Polish (non-blocking) milestone Oct 19, 2023
@jleibs
Copy link
Member Author

jleibs commented Oct 19, 2023

@jleibs jleibs added 🦟 regression A thing that used to work in an earlier release 🔺 re_renderer affects re_renderer itself and removed 🪳 bug Something isn't working 👀 needs triage This issue needs to be triaged by the Rerun team labels Oct 19, 2023
@Wumpf
Copy link
Member

Wumpf commented Oct 19, 2023

As discussed on Slack, the NV12 PR had a different issue which made it fail on all platforms which was fixed in

After that we're left with the issue described in the PR description

@Wumpf Wumpf added the 💣 crash crash, deadlock/freeze, do-no-start label Oct 19, 2023
@jleibs jleibs changed the title Web-view crashes in chrome on linux Web-view crashes in chrome on linux when shaders are too large Oct 20, 2023
@jleibs
Copy link
Member Author

jleibs commented Oct 20, 2023

The best working theory we have at the moment is that this is a bad interaction between ANGLE and MESA that occurs in particular edge cases when the shader crosses a size threshold.

The workaround in #3948 lends some support to that theory.

Hopefully this observation can lead us to a more minimal repro and upstream bug-report(s).

jleibs added a commit that referenced this issue Oct 23, 2023
…le/mesa bug (#3948)

### What
:see_no_evil: 

This mitigates the crash reported in:
* #3931
but should be considered a workaround, rather than a proper fix.

After a fair bit of shooting in the dark, our best working theory is
that the there is a magic size threshold at which shaders fail to link
on angle+mesa.

On that hunch, I refactored this shader and am no longer seeing the
crash.

### 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/3948) (if
applicable)
* [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/3948)
- [Docs
preview](https://rerun.io/preview/aae3dae91c18d6697fc5be06d84e0f9ca16d0ba9/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/aae3dae91c18d6697fc5be06d84e0f9ca16d0ba9/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://ref.rerun.io/dev/bench/)
- [Wasm size tracking](https://ref.rerun.io/dev/sizes/)
@emilk
Copy link
Member

emilk commented Oct 24, 2023

#3948 worked around the problem, so it is mitigated, but not fixed

jleibs added a commit that referenced this issue Feb 7, 2024
… and avoid crash on chrome (#3931) (#5074)

### What
When we added support for YUY2 format in
(#4877) we we re-triggered the
issue originally originally reported in
#3931 and now
#5073
- This is a more extreme version of:
#3948 which was the workaround the
first time we hit this.

### 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/5074/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/5074/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/5074/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](tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/5074)
- [Docs
preview](https://rerun.io/preview/e35e3a58e1f2093140d4d63479098124f6556463/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/e35e3a58e1f2093140d4d63479098124f6556463/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
@emilk emilk mentioned this issue Jul 30, 2024
6 tasks
Wumpf added a commit that referenced this issue Oct 9, 2024
…nversions (#7640)

### What

* Major part of #7608
* Related to #3931
* since it makes the rectangle shader smaller which kept acting up in
the past


via `pixi run -e py python
./tests/python/chroma_downsample_image/main.py`:

https://rerun.io/viewer/pr/7640?url=https%3A%2F%2Fstatic.rerun.io%2Frrd%2F0.19%2Fchroma_formats_ad9697d05933f4104fbbe66af23073ad4e9e4a58.rrd


![image](https://github.com/user-attachments/assets/76756c5f-75db-44b7-8db0-f04256f72e3a)



The new functionality is tightly integrated into `TextureManager2D`
which now ingests all incoming data via a new internal method
`transfer_image_data_to_texture`.
This in turn takes care of data upload via `GpuReadCpuWriteBelt` (new!
previously, we would use `queue.write_texture` in this case) and may if
needed run gpu based conversion.

Gpu based conversion is for convenience implemented like a `Renderer` -
while it is not _used_ like a typical `Renderer`, the lifecycle (create
lazily once, store on context, feed with data bundles [...]) was so
similar that it made sense to me to use this existing pattern even
though it's not a perfect match.

**A curious sideeffect of this is that you can now put chroma
downsampled textures on custom meshes!**

Next steps:
* support formats required for AV1
   * while on it, check if things could get simpler by us  
* move BGR handling into the new pipeline
* revisit the re_renderer sided input data description. Not sure if I'm
happy about it drifting away from the user facing one!
* once we're there we're also very close to get rid of
`SourceImageDataFormat::WgpuCompatible`. But it's not strictly
necessary, this might yet prove a convenient loophole, although its
presence makes me a little bit nervous (details see code comments) 🤔
* consider exposing color primaries to public API
* the only thing keeping us from it is adding a new enum to the color
format!
    * should it be called "primaries" or "color space" 🤔   

### Checklist
* [x] test on web Mac Chrome/Firefox/Safari
* [x] test on web Window Chrome/Firefox
* [ ] test on web Firefox Chrome/Firefox (@jleibs plz 🥺 )
* [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/7640?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/7640?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)!
* [x] If have noted any breaking changes to the log API in
`CHANGELOG.md` and the migration guide

- [PR Build Summary](https://build.rerun.io/pr/7640)
- [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`.
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 🦟 regression A thing that used to work in an earlier release
Projects
None yet
Development

No branches or pull requests

3 participants