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

Make mesh attr vertex count mismatch warn more readable #10259

Merged
merged 1 commit into from
Oct 25, 2023

Conversation

torsteingrindvik
Copy link
Contributor

Objective

When a mesh vertex attribute has a vertex count mismatch, a warning message is printed with the index of the attribute which did not match.

Change to name the attribute, or fall back to the old behaviour if it was not a known attribute.

Before:

MeshVertexAttributeId(2) has a different vertex count (32) than other attributes (64) in this mesh, all attributes will be truncated to match the smallest.

After:

Vertex_Uv has a different vertex count (32) than other attributes (64) in this mesh, all attributes will be truncated to match the smallest.

Solution

Name the mesh attribute which had a count mismatch.

Changelog

  • If a mesh vertex attribute has a different count than other vertex attributes, name the offending attribute using a human readable name

Copy link
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.

Seems to work.

Related question that doesn't need to be addressed here:

I was testing by adding an 12th vertex color in mesh2d_manual. The warning seems to imply that things should generally work with mismatched attribute counts. But it seems to crash (after emitting the warning) with errors like this:

thread 'Compute Task Pool (0)' panicked at crates/bevy_render/src/mesh/mesh/mod.rs:430:46:
range end index 192 out of range for slice of length 176

count_vertices is returning 11 in that case, which seems correct.

Is that expected or should it be followed up on in another issue?

@rparrett rparrett added A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Oct 25, 2023
@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Oct 25, 2023
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Oct 25, 2023
Merged via the queue into bevyengine:main with commit f992398 Oct 25, 2023
25 checks passed
@torsteingrindvik
Copy link
Contributor Author

Seems to work.

Related question that doesn't need to be addressed here:

I was testing by adding an 12th vertex color in mesh2d_manual. The warning seems to imply that things should generally work with mismatched attribute counts. But it seems to crash (after emitting the warning) with errors like this:

thread 'Compute Task Pool (0)' panicked at crates/bevy_render/src/mesh/mesh/mod.rs:430:46:
range end index 192 out of range for slice of length 176

count_vertices is returning 11 in that case, which seems correct.

Is that expected or should it be followed up on in another issue?

Good point, since it's a warning and not a panic that should imply it should work at some level, but I had a panic as a consequence as well. That could be followed up in another issue/PR.

ameknite pushed a commit to ameknite/bevy that referenced this pull request Nov 6, 2023
…0259)

# Objective

When a mesh vertex attribute has a vertex count mismatch, a warning
message is printed with the index of the attribute which did not match.

Change to name the attribute, or fall back to the old behaviour if it
was not a known attribute.

Before:

```
MeshVertexAttributeId(2) has a different vertex count (32) than other attributes (64) in this mesh, all attributes will be truncated to match the smallest.
```

After:

```
Vertex_Uv has a different vertex count (32) than other attributes (64) in this mesh, all attributes will be truncated to match the smallest.
```

## Solution

Name the mesh attribute which had a count mismatch.


## Changelog

- If a mesh vertex attribute has a different count than other vertex
attributes, name the offending attribute using a human readable name

Signed-off-by: Torstein Grindvik <[email protected]>
Co-authored-by: Torstein Grindvik <[email protected]>
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this pull request Jan 9, 2024
…0259)

# Objective

When a mesh vertex attribute has a vertex count mismatch, a warning
message is printed with the index of the attribute which did not match.

Change to name the attribute, or fall back to the old behaviour if it
was not a known attribute.

Before:

```
MeshVertexAttributeId(2) has a different vertex count (32) than other attributes (64) in this mesh, all attributes will be truncated to match the smallest.
```

After:

```
Vertex_Uv has a different vertex count (32) than other attributes (64) in this mesh, all attributes will be truncated to match the smallest.
```

## Solution

Name the mesh attribute which had a count mismatch.


## Changelog

- If a mesh vertex attribute has a different count than other vertex
attributes, name the offending attribute using a human readable name

Signed-off-by: Torstein Grindvik <[email protected]>
Co-authored-by: Torstein Grindvik <[email protected]>
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-Usability A targeted quality-of-life change that makes Bevy easier to use 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.

3 participants