Skip to content

Alternative fix for mesh2d_manual example#15883

Merged
alice-i-cecile merged 1 commit intobevyengine:mainfrom
rparrett:fix-mesh2d-manual-alt
Oct 14, 2024
Merged

Alternative fix for mesh2d_manual example#15883
alice-i-cecile merged 1 commit intobevyengine:mainfrom
rparrett:fix-mesh2d-manual-alt

Conversation

@rparrett
Copy link
Copy Markdown
Contributor

@rparrett rparrett commented Oct 13, 2024

Objective

Fixes #15847

Alternative to #15862. Would appreciate a rendering person signaling preference for one or the other.

Solution

Partially revert the changes made to this example in #15524.

Add comment explaining that the non-usage of the built-in color vertex attribute is intentional.

Testing

cargo run --example mesh2d_manual

@rparrett rparrett changed the title Fix mesh2d_manual example Alternative fix for mesh2d_manual example Oct 13, 2024
@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior C-Examples An addition or correction to our examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Oct 13, 2024
@alice-i-cecile alice-i-cecile added this to the 0.15 milestone Oct 13, 2024
@alice-i-cecile alice-i-cecile added the A-Rendering Drawing game state to the screen label Oct 13, 2024
Copy link
Copy Markdown
Contributor

@MiniaczQ MiniaczQ left a comment

Choose a reason for hiding this comment

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

I like this more than the alternative,
can confirm the example works

Copy link
Copy Markdown
Member

@tychedelia tychedelia left a comment

Choose a reason for hiding this comment

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

I think it makes sense to demonstrate something actually custom. Thanks!

@IceSentry
Copy link
Copy Markdown
Contributor

Why does the current code not work? AFAIK attributes haven't changed at all for a long time?

@rparrett
Copy link
Copy Markdown
Contributor Author

Why does the current code not work? AFAIK attributes haven't changed at all for a long time?

@IceSentry It was mistake during the migration in #15862. During the process of figuring out why mesh2d_manual wasn't working, it was tidied up to not use the built-in color vertex attribute instead of the custom one. But the shader code wasn't updated. The underlying problem with the example was fixed in another way.

So the question right now is just "do we want to use a custom vertex attribute in mesh2d_manual or the built-in one?".

Copy link
Copy Markdown
Contributor

@IceSentry IceSentry left a comment

Choose a reason for hiding this comment

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

Okay, thank you for the context, in that case, yeah I'm happy to use a custom attribute here.

@IceSentry IceSentry 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 Oct 13, 2024
@BenjaminBrienen BenjaminBrienen added the D-Straightforward Simple bug fixes and API improvements, docs, test and examples label Oct 13, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Oct 14, 2024
Merged via the queue into bevyengine:main with commit 9dd6f42 Oct 14, 2024
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 C-Examples An addition or correction to our examples 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

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

mesh2d_manual example doesn't render correctly

6 participants