Skip to content

Fix MeshletMesh material system ordering #14016

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

Merged

Conversation

JMS55
Copy link
Contributor

@JMS55 JMS55 commented Jun 25, 2024

Objective

Solution

  • Turns out that Queue and PrepareAssets are not ordered. We should probably either rethink our system sets (again), or improve the documentation here. For reference, I've included the current ordering below.
  • The prepare_meshlet_meshes_X systems need to run after prepare_assets::<PreparedMaterial<M>>, and have also been moved to QueueMeshes.
schedule.configure_sets(
    (
        ExtractCommands,
        ManageViews,
        Queue,
        PhaseSort,
        Prepare,
        Render,
        Cleanup,
    )
        .chain(),
);

schedule.configure_sets((ExtractCommands, PrepareAssets, Prepare).chain());
schedule.configure_sets(QueueMeshes.in_set(Queue).after(prepare_assets::<GpuMesh>));
schedule.configure_sets(
    (PrepareResources, PrepareResourcesFlush, PrepareBindGroups)
        .chain()
        .in_set(Prepare),
);

Testing

  • Ambiguity checker to make sure I don't have ambiguous system ordering

@JMS55 JMS55 added this to the 0.14 milestone Jun 25, 2024
@JMS55 JMS55 added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen labels Jun 25, 2024
@alice-i-cecile alice-i-cecile added D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jun 25, 2024
@alice-i-cecile alice-i-cecile 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 Jun 25, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jun 25, 2024
Merged via the queue into bevyengine:main with commit d8b45ca Jun 25, 2024
33 checks passed
mockersf pushed a commit that referenced this pull request Jun 25, 2024
# Objective
- Fixes #13811 (probably, I lost my test code...)

## Solution
- Turns out that Queue and PrepareAssets are _not_ ordered. We should
probably either rethink our system sets (again), or improve the
documentation here. For reference, I've included the current ordering
below.
- The `prepare_meshlet_meshes_X` systems need to run after
`prepare_assets::<PreparedMaterial<M>>`, and have also been moved to
QueueMeshes.

```rust
schedule.configure_sets(
    (
        ExtractCommands,
        ManageViews,
        Queue,
        PhaseSort,
        Prepare,
        Render,
        Cleanup,
    )
        .chain(),
);

schedule.configure_sets((ExtractCommands, PrepareAssets, Prepare).chain());
schedule.configure_sets(QueueMeshes.in_set(Queue).after(prepare_assets::<GpuMesh>));
schedule.configure_sets(
    (PrepareResources, PrepareResourcesFlush, PrepareBindGroups)
        .chain()
        .in_set(Prepare),
);
```

## Testing
- Ambiguity checker to make sure I don't have ambiguous system ordering
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 D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MeshletMesh material textures not rendering sometimes
3 participants