Skip to content

Actually add objects to the scene buffers in sorted render phases.#17849

Merged
mockersf merged 2 commits intobevyengine:mainfrom
pcwalton:add-sorted-objects-again
Feb 14, 2025
Merged

Actually add objects to the scene buffers in sorted render phases.#17849
mockersf merged 2 commits intobevyengine:mainfrom
pcwalton:add-sorted-objects-again

Conversation

@pcwalton
Copy link
Copy Markdown
Contributor

There was nonsense code in batch_and_prepare_sorted_render_phase that created temporary buffers to add objects to instead of using the correct ones. I think this was debug code. This commit removes that code in favor of writing to the actual buffers.

Closes #17846.

There was nonsense code in `batch_and_prepare_sorted_render_phase` that
created temporary buffers to add objects to instead of using the correct
ones. I think this was debug code. This commit removes that code in
favor of writing to the actual buffers.

Closes bevyengine#17846.
@pcwalton pcwalton added A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior P-Regression Functionality that used to work but no longer does. Add a test for this! P-High This is particularly urgent, and deserves immediate attention S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Feb 13, 2025
Copy link
Copy Markdown
Contributor

@rparrett rparrett left a comment

Choose a reason for hiding this comment

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

Changeset seems to make logical sense. I spot checked a few examples.

Copy link
Copy Markdown
Contributor

@ickshonpe ickshonpe left a comment

Choose a reason for hiding this comment

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

Examples working correctly now.

@pcwalton pcwalton added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Feb 13, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Feb 13, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 13, 2025
@mockersf mockersf added this pull request to the merge queue Feb 14, 2025
Merged via the queue into bevyengine:main with commit 3c9e696 Feb 14, 2025
github-merge-queue bot pushed a commit that referenced this pull request Feb 16, 2025
# Objective

Fix panic in `custom_render_phase`.

This example was broken by #17764, but that breakage evolved into a
panic after #17849. This new panic seems to illustrate the problem in a
pretty straightforward way.

```
2025-02-15T00:44:11.833622Z  INFO bevy_diagnostic::system_information_diagnostics_plugin::internal: SystemInfo { os: "macOS 15.3 Sequoia", kernel: "24.3.0", cpu: "Apple M4 Max", core_count: "16", memory: "64.0 GiB" }    
2025-02-15T00:44:11.908328Z  INFO bevy_render::renderer: AdapterInfo { name: "Apple M4 Max", vendor: 0, device: 0, device_type: IntegratedGpu, driver: "", driver_info: "", backend: Metal }
2025-02-15T00:44:12.314930Z  INFO bevy_winit::system: Creating new window App (0v1)
thread 'Compute Task Pool (1)' panicked at /Users/me/src/bevy/crates/bevy_ecs/src/system/function_system.rs:216:28:
bevy_render::batching::gpu_preprocessing::batch_and_prepare_sorted_render_phase<custom_render_phase::Stencil3d, custom_render_phase::StencilPipeline> could not access system parameter ResMut<PhaseBatchedInstanceBuffers<Stencil3d, MeshUniform>>
```

## Solution

Add a `SortedRenderPhasePlugin` for the custom phase.

## Testing

`cargo run --example custom_render_phase`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior P-High This is particularly urgent, and deserves immediate attention P-Regression Functionality that used to work but no longer does. Add a test for this! S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

Transparent things don't render and cause warning spam

6 participants