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

feat: added more information on obj_model setup #433

Merged
merged 2 commits into from
Jan 28, 2023

Conversation

erwanvivien
Copy link
Contributor

@erwanvivien erwanvivien commented Jan 1, 2023

When following the rendering a mesh part, I was kind of lost, so I added a little more info

Edit: I actually had to look at the code to be sure of what I was doing

Copy link
Owner

@sotrh sotrh left a comment

Choose a reason for hiding this comment

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

There's a typo that I mentioned below, other than that everything else looks fine

@@ -62,6 +62,8 @@ impl Vertex for ModelVertex {

This is basically the same as the original `VertexBufferLayout`, but we added a `VertexAttribute` for the `normal`. Remove the `Vertex` struct in `lib.rs` as we won't need it anymore, and use our new `Vertex` from `model` for the `RenderPipeline`.

We will also remove our homemade `vertex_buffer`, `index_buffer` and `num_indicdes`.
Copy link
Owner

Choose a reason for hiding this comment

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

There's a typo here: it should be num_indices not num_indicdes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fixed it! Thanks for review :)

@sotrh sotrh merged commit c17e90d into sotrh:master Jan 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants