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

MeshletMesh material textures not rendering sometimes #13811

Closed
JMS55 opened this issue Jun 11, 2024 · 2 comments · Fixed by #14016
Closed

MeshletMesh material textures not rendering sometimes #13811

JMS55 opened this issue Jun 11, 2024 · 2 comments · Fixed by #14016
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior S-Needs-Investigation This issue requires detective work to figure out what's going wrong

Comments

@JMS55
Copy link
Contributor

JMS55 commented Jun 11, 2024

Bevy d659a1f7d506c5c6eba9dfe2a6e878a8c72ecef6.

I used a MeshletMeshMaterialBundle with a normal_map_texture loaded from disk via the asset server. Sometimes, it would render without the normal map. Sometimes it would render with the normal map. It was inconsistent across runs, but not between frames.

Regular MeshMaterialBundles do not seem to have this problem.

Presumably this applies to all kinds of textures, not just normal maps.
@JMS55 JMS55 added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen S-Needs-Investigation This issue requires detective work to figure out what's going wrong labels Jun 11, 2024
@JMS55 JMS55 added this to the 0.14 milestone Jun 11, 2024
@alice-i-cecile alice-i-cecile removed this from the 0.14 milestone Jun 25, 2024
@alice-i-cecile
Copy link
Member

This looks like a system order ambiguity to me based on the description. See #7386 for efforts to prevent this sort of bug in the future.

@alice-i-cecile
Copy link
Member

I've removed this from the milestone as it's a) not a regression b) has no fix ready to review and c) a bug with an explicitly experimental feature.

@JMS55 JMS55 mentioned this issue Jun 25, 2024
Merged
github-merge-queue bot pushed a commit that referenced this issue Jun 25, 2024
@JMS55
Fix MeshletMesh material system ordering (#14016)
d8b45ca 
# 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
mockersf pushed a commit that referenced this issue Jun 25, 2024
@JMS55 @mockersf
Fix MeshletMesh material system ordering (#14016)
65daab8 
# 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
Assignees
No one assigned
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior S-Needs-Investigation This issue requires detective work to figure out what's going wrong
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix MeshletMesh material system ordering JMS55/bevy
2 participants
@alice-i-cecile @JMS55