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.

Sign up for GitHub

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

Jump to bottom

Fix crash when trying to render too many line segments #3093

Merged
merged 8 commits into from
Aug 25, 2023
Merged

Fix crash when trying to render too many line segments #3093

merged 8 commits into from
Aug 25, 2023

Conversation

Wumpf
Copy link
Member

@Wumpf Wumpf commented Aug 24, 2023 •
edited
Loading

What

The second commit is the actual fix. But to avoid this family of crashes, the cpuwritegpuread belt now always does error checks and returns errors instead of paniking

Checklist

@Wumpf Wumpf added 🔺 re_renderer affects re_renderer itself 💣 crash crash, deadlock/freeze, do-no-start labels Aug 24, 2023
emilk
emilk approved these changes Aug 24, 2023
View reviewed changes
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.

LGTM, except for the silent swallowing of errors

crates/re_renderer/src/allocator/cpu_write_gpu_read_belt.rs Outdated Show resolved Hide resolved
crates/re_renderer/src/allocator/uniform_buffer_fill.rs Outdated Show resolved Hide resolved
crates/re_renderer/src/line_strip_builder.rs Outdated Show resolved Hide resolved
crates/re_renderer/src/line_strip_builder.rs Outdated Show resolved Hide resolved
crates/re_renderer/src/point_cloud_builder.rs Outdated Show resolved Hide resolved
crates/re_renderer/src/point_cloud_builder.rs Outdated Show resolved Hide resolved
@emilk emilk added this to the 0.8.2 milestone Aug 24, 2023
@Wumpf Wumpf force-pushed the andreas/fix-line-overflow-crash branch from e5400f1 to 6cc27d8 Compare August 25, 2023 08:03
@Wumpf Wumpf force-pushed the andreas/fix-line-overflow-crash branch from 6cc27d8 to b6b7379 Compare August 25, 2023 08:06
@Wumpf Wumpf merged commit c2c982a into main Aug 25, 2023
26 of 27 checks passed
@Wumpf Wumpf deleted the andreas/fix-line-overflow-crash branch August 25, 2023 08:49
jleibs pushed a commit that referenced this pull request Aug 31, 2023
@Wumpf @jleibs
Fix crash on reaching line number limit (#3093)
6f535e8 
* Fixes: #3075
* Related to: #3076

The second commit is the actual fix. But to avoid this family of
crashes, the cpuwritegpuread belt now always does error checks and
returns errors instead of paniking

* [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/3093) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/3093)
- [Docs
preview](https://rerun.io/preview/b6b737939d3219b0ca32e763d7276b5ca0d504ee/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/b6b737939d3219b0ca32e763d7276b5ca0d504ee/examples)
<!--EXAMPLES-PREVIEW--><!--EXAMPLES-PREVIEW--><!--EXAMPLES-PREVIEW--><!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://ref.rerun.io/dev/bench/)
- [Wasm size tracking](https://ref.rerun.io/dev/sizes/)
jleibs pushed a commit that referenced this pull request Aug 31, 2023
@Wumpf @jleibs
Fix crash on reaching line number limit (#3093)
98bd1f5 
* Fixes: #3075
* Related to: #3076

The second commit is the actual fix. But to avoid this family of
crashes, the cpuwritegpuread belt now always does error checks and
returns errors instead of paniking

* [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/3093) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/3093)
- [Docs
preview](https://rerun.io/preview/b6b737939d3219b0ca32e763d7276b5ca0d504ee/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/b6b737939d3219b0ca32e763d7276b5ca0d504ee/examples)
<!--EXAMPLES-PREVIEW--><!--EXAMPLES-PREVIEW--><!--EXAMPLES-PREVIEW--><!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://ref.rerun.io/dev/bench/)
- [Wasm size tracking](https://ref.rerun.io/dev/sizes/)
jleibs pushed a commit that referenced this pull request Aug 31, 2023
@Wumpf @jleibs
Fix crash on reaching line number limit (#3093)
c11fbf1 
* Fixes: #3075
* Related to: #3076

The second commit is the actual fix. But to avoid this family of
crashes, the cpuwritegpuread belt now always does error checks and
returns errors instead of paniking

* [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/3093) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/3093)
- [Docs
preview](https://rerun.io/preview/b6b737939d3219b0ca32e763d7276b5ca0d504ee/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/b6b737939d3219b0ca32e763d7276b5ca0d504ee/examples)
<!--EXAMPLES-PREVIEW--><!--EXAMPLES-PREVIEW--><!--EXAMPLES-PREVIEW--><!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://ref.rerun.io/dev/bench/)
- [Wasm size tracking](https://ref.rerun.io/dev/sizes/)
@jleibs jleibs mentioned this pull request Aug 31, 2023
Closed
3 tasks
@emilk emilk changed the title Fix crash on reaching line number limit Fix crash on reaching line segment limit Aug 31, 2023
@jleibs jleibs changed the title Fix crash on reaching line segment limit Fix crash when trying to render too many line segments Aug 31, 2023
@emilk emilk mentioned this pull request Sep 4, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emilk emilk emilk approved these changes

Assignees
No one assigned
Labels
💣 crash crash, deadlock/freeze, do-no-start 🔺 re_renderer affects re_renderer itself
Projects
None yet
Milestone
0.8.2
Development

Successfully merging this pull request may close these issues.

Crash when reaching maximum number of linestrips
2 participants
@Wumpf @emilk