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.

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

Fix non-meshlet shaders for non-bindless mode #16966

Merged
merged 2 commits into from
Dec 26, 2024

Conversation

mockersf
Copy link
Member

@mockersf mockersf commented Dec 25, 2024

Objective

  • Running example load_gltf when not using bindless gives this error
ERROR bevy_render::render_resource::pipeline_cache: failed to process shader:
error: no definition in scope for identifier: 'slot'
    ┌─ crates/bevy_pbr/src/render/pbr_fragment.wgsl:153:13
    │
153 │             slot,
    │             ^^^^ unknown identifier
    │
    = no definition in scope for identifier: 'slot'

Solution

  • Set slot to the expected value when not mindless
  • Also use it for uv_b

Testing

  • Run example load_gltf on a Mac or in wasm

@mockersf mockersf added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen P-Regression Functionality that used to work but no longer does. Add a test for this! labels Dec 25, 2024
@BenjaminBrienen BenjaminBrienen 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 Dec 25, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Dec 26, 2024
Merged via the queue into bevyengine:main with commit e8fc279 Dec 26, 2024
34 checks passed
github-merge-queue bot pushed a commit that referenced this pull request Dec 26, 2024
# Objective

Fixes #16978

While testing, discovered that the morph weight interface in
`scene_viewer` has been broken for a while (panics when loaded model has
morph weights), probably since #15591. Fixed that too.

While testing, saw example text in morph interface with [wrong
padding](https://bevyengine.org/learn/contribute/helping-out/creating-examples/#visual-guidelines).
Fixed that too. Left the small font size because there may be a lot of
morphs to display, so that seems intentional.

## Solution

Use normal queries and bail early

## Testing

Morph interface can be tested with
```
cargo run --example scene_viewer assets/models/animated/MorphStressTest.gltf
```

## Discussion

I noticed that this fix is different than what is happening in #16976.
Feel free to discard this for an alternative fix. I opened this anyway
to document the issue with morph weight display.

This is on top of #16966 which is required to test.

---------

Co-authored-by: François Mockers <[email protected]>
Co-authored-by: François Mockers <[email protected]>
mockersf added a commit that referenced this pull request Jan 3, 2025
Fixes #16978

While testing, discovered that the morph weight interface in
`scene_viewer` has been broken for a while (panics when loaded model has
morph weights), probably since #15591. Fixed that too.

While testing, saw example text in morph interface with [wrong
padding](https://bevyengine.org/learn/contribute/helping-out/creating-examples/#visual-guidelines).
Fixed that too. Left the small font size because there may be a lot of
morphs to display, so that seems intentional.

Use normal queries and bail early

Morph interface can be tested with
```
cargo run --example scene_viewer assets/models/animated/MorphStressTest.gltf
```

I noticed that this fix is different than what is happening in #16976.
Feel free to discard this for an alternative fix. I opened this anyway
to document the issue with morph weight display.

This is on top of #16966 which is required to test.

---------

Co-authored-by: François Mockers <[email protected]>
Co-authored-by: François Mockers <[email protected]>
ecoskey pushed a commit to ecoskey/bevy that referenced this pull request Jan 6, 2025
# Objective

- Running example `load_gltf` when not using bindless gives this error
```
ERROR bevy_render::render_resource::pipeline_cache: failed to process shader:
error: no definition in scope for identifier: 'slot'
    ┌─ crates/bevy_pbr/src/render/pbr_fragment.wgsl:153:13
    │
153 │             slot,
    │             ^^^^ unknown identifier
    │
    = no definition in scope for identifier: 'slot'
```
- since bevyengine#16825

## Solution

- Set `slot` to the expected value when not mindless
- Also use it for `uv_b`

## Testing

- Run example `load_gltf` on a Mac or in wasm
ecoskey pushed a commit to ecoskey/bevy that referenced this pull request Jan 6, 2025
# Objective

Fixes bevyengine#16978

While testing, discovered that the morph weight interface in
`scene_viewer` has been broken for a while (panics when loaded model has
morph weights), probably since bevyengine#15591. Fixed that too.

While testing, saw example text in morph interface with [wrong
padding](https://bevyengine.org/learn/contribute/helping-out/creating-examples/#visual-guidelines).
Fixed that too. Left the small font size because there may be a lot of
morphs to display, so that seems intentional.

## Solution

Use normal queries and bail early

## Testing

Morph interface can be tested with
```
cargo run --example scene_viewer assets/models/animated/MorphStressTest.gltf
```

## Discussion

I noticed that this fix is different than what is happening in bevyengine#16976.
Feel free to discard this for an alternative fix. I opened this anyway
to document the issue with morph weight display.

This is on top of bevyengine#16966 which is required to test.

---------

Co-authored-by: François Mockers <[email protected]>
Co-authored-by: François Mockers <[email protected]>
mrchantey pushed a commit to mrchantey/bevy that referenced this pull request Feb 4, 2025
# Objective

- Running example `load_gltf` when not using bindless gives this error
```
ERROR bevy_render::render_resource::pipeline_cache: failed to process shader:
error: no definition in scope for identifier: 'slot'
    ┌─ crates/bevy_pbr/src/render/pbr_fragment.wgsl:153:13
    │
153 │             slot,
    │             ^^^^ unknown identifier
    │
    = no definition in scope for identifier: 'slot'
```
- since bevyengine#16825

## Solution

- Set `slot` to the expected value when not mindless
- Also use it for `uv_b`

## Testing

- Run example `load_gltf` on a Mac or in wasm
mrchantey pushed a commit to mrchantey/bevy that referenced this pull request Feb 4, 2025
# Objective

Fixes bevyengine#16978

While testing, discovered that the morph weight interface in
`scene_viewer` has been broken for a while (panics when loaded model has
morph weights), probably since bevyengine#15591. Fixed that too.

While testing, saw example text in morph interface with [wrong
padding](https://bevyengine.org/learn/contribute/helping-out/creating-examples/#visual-guidelines).
Fixed that too. Left the small font size because there may be a lot of
morphs to display, so that seems intentional.

## Solution

Use normal queries and bail early

## Testing

Morph interface can be tested with
```
cargo run --example scene_viewer assets/models/animated/MorphStressTest.gltf
```

## Discussion

I noticed that this fix is different than what is happening in bevyengine#16976.
Feel free to discard this for an alternative fix. I opened this anyway
to document the issue with morph weight display.

This is on top of bevyengine#16966 which is required to test.

---------

Co-authored-by: François Mockers <[email protected]>
Co-authored-by: François Mockers <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Feb 10, 2025
# Objective

#16966 tried to fix a bug where
`slot` wasn't passed to `parallaxed_uv` when not running under bindless,
but failed to account for meshlets. This surfaces on macOS where
bindless is disabled.

## Solution

Lift the slot variable out of the bindless condition so it's always
available.
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 P-Regression Functionality that used to work but no longer does. Add a test for this! S-Needs-Review Needs reviewer attention (from anyone!) to move forward
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants