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

Work around crash on chrome (#3931) by working around around a suspected angle/mesa bug #3948

Merged
merged 3 commits into from
Oct 23, 2023

Conversation

jleibs
Copy link
Member

@jleibs jleibs commented Oct 20, 2023

What

Refactor the rectangle shader to reduce its size.

This mitigates the crash reported in:

Context

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

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I have tested demo.rerun.io (if applicable)
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG

@jleibs jleibs marked this pull request as ready for review October 20, 2023 17:14
@jleibs jleibs added 🔺 re_renderer affects re_renderer itself exclude from changelog PRs with this won't show up in CHANGELOG.md labels Oct 20, 2023
@jleibs jleibs force-pushed the jleibs/workaround_mesa_crash branch from 13c842d to aae3dae Compare October 20, 2023 17:57
Copy link
Member

@emilk emilk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicer looking code too, though now perhaps doing unnecessary work in the FILTER_NEAREST path?

Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra work by hitting clamp_to_edge_nearest_neighbor path more consequent seems fine.

The fact that this fixes our issue is extremely scary. I'm trying to follow up with a minimal repo for a bugreport

@emilk
Copy link
Member

emilk commented Oct 23, 2023

Does this fix the "crash on chrome on linux" bug? If so, please update the PR description to reflect that

@jleibs jleibs merged commit a8b581c into main Oct 23, 2023
34 checks passed
@jleibs jleibs deleted the jleibs/workaround_mesa_crash branch October 23, 2023 14:34
@jleibs jleibs changed the title Refactor the rectangle fragment shader to work around a suspected angle/mesa bug Work around crash on chrome (#3931) by working around around a suspected angle/mesa bug Oct 24, 2023
jleibs added a commit that referenced this pull request 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exclude from changelog PRs with this won't show up in CHANGELOG.md 🔺 re_renderer affects re_renderer itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants