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

Improper batching of non-mesh phase items. #14295

Closed
james-j-obrien opened this issue Jul 13, 2024 · 0 comments · Fixed by #14296
Closed

Improper batching of non-mesh phase items. #14295

james-j-obrien opened this issue Jul 13, 2024 · 0 comments · Fixed by #14296
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior C-Regression Functionality that used to work but no longer does. Add a test for this!

Comments

@james-j-obrien
Copy link
Contributor

james-j-obrien commented Jul 13, 2024 •
edited

Bevy version

v0.14.0

What you did

Run bevy_vector_shapes examples with all features enabled.

What went wrong

The batch_range of custom phase items was being trampled causing failure to render, upon investigation I discovered that this was happening in the gpu_preprocessing variant of batch_and_prepare_sorted_render_phase. The result is non-deterministic due to system ordering.

When GFBD::get_index_and_compare_data() returns None it should skip batching for that item in it's entirety as it may already be being batched by some other pipeline. The batch_range of those items should not be modified.

I was able to mitigate the issue in the repro I have by adding this at line 426:

if current_batch_input_index.is_none() {
    continue;
}

There may be a more appropriate way to handle this in the context of mesh batching at large but the current behavior is definitely incorrect.

Additional information

Original bug: james-j-obrien/bevy_vector_shapes#42
@james-j-obrien james-j-obrien added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled A-Rendering Drawing game state to the screen C-Regression Functionality that used to work but no longer does. Add a test for this! labels Jul 13, 2024
@james-j-obrien james-j-obrien mentioned this issue Jul 13, 2024
Merged
@alice-i-cecile alice-i-cecile added C-Regression Functionality that used to work but no longer does. Add a test for this! and removed C-Regression Functionality that used to work but no longer does. Add a test for this! S-Needs-Triage This issue needs to be labelled labels Jul 14, 2024
@rparrett rparrett mentioned this issue Jul 31, 2024
Closed
github-merge-queue bot pushed a commit that referenced this issue Aug 2, 2024
@james-j-obrien
Skip batching for phase items from other pipelines (#14296)
b98d15f 
# Objective

- Fix #14295

## Solution

- Early out when `GFBD::get_index_and_compare_data` returns None.

## Testing

- Tested on a selection of examples including `many_foxes` and
`3d_shapes`.
- Resolved the original issue in `bevy_vector_shapes`.
mockersf pushed a commit that referenced this issue Aug 2, 2024
@james-j-obrien @mockersf
Skip batching for phase items from other pipelines (#14296)
833ee3f 
# Objective

- Fix #14295

## Solution

- Early out when `GFBD::get_index_and_compare_data` returns None.

## Testing

- Tested on a selection of examples including `many_foxes` and
`3d_shapes`.
- Resolved the original issue in `bevy_vector_shapes`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior C-Regression Functionality that used to work but no longer does. Add a test for this!
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Skip batching for phase items from other pipelines james-j-obrien/bevy
2 participants
@alice-i-cecile @james-j-obrien