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: Only skip invalid meshes, instead of aborting the pass completely. #543

Merged
merged 1 commit into from Jan 17, 2018

Conversation

Projects
None yet
4 participants
@Rhuagh
Member

Rhuagh commented Jan 15, 2018

This change is Reviewable

@Xaeroxe

LGTM! Thanks!

@omni-viral

Looking good. Only one nit.

@@ -188,7 +188,7 @@ impl Pass for DrawPbmSeparate {
{
match mesh.buffer(attrs) {
Some(vbuf) => effect.data.vertex_bufs.push(vbuf.clone()),
- None => return,
+ None => continue 'drawable,

This comment has been minimized.

@omni-viral

omni-viral Jan 15, 2018

Member

effect.data.vertex_bufs must be cleared.

@omni-viral

omni-viral Jan 15, 2018

Member

effect.data.vertex_bufs must be cleared.

@Rhuagh

This comment has been minimized.

Show comment
Hide comment
@Rhuagh

Rhuagh Jan 15, 2018

Member

Fixed.

Member

Rhuagh commented Jan 15, 2018

Fixed.

@omni-viral

LGTM. Thanks!

@torkleyy

We have some code duplication here, but that's obviously not your PR.

@omni-viral

This comment has been minimized.

Show comment
Hide comment
@omni-viral

omni-viral Jan 16, 2018

Member

@torkleyy I wouldn't spend much time to fix minor problems in old renderer.

Member

omni-viral commented Jan 16, 2018

@torkleyy I wouldn't spend much time to fix minor problems in old renderer.

@Rhuagh

This comment has been minimized.

Show comment
Hide comment
@Rhuagh

Rhuagh Jan 16, 2018

Member

Yeah, there's some code duplication, but no good way of removing that without introducing about the same amount of duplication just trying to send data to functions.

Member

Rhuagh commented Jan 16, 2018

Yeah, there's some code duplication, but no good way of removing that without introducing about the same amount of duplication just trying to send data to functions.

@Rhuagh

This comment has been minimized.

Show comment
Hide comment
@Rhuagh

Rhuagh Jan 16, 2018

Member

bors r+

Member

Rhuagh commented Jan 16, 2018

bors r+

bors bot added a commit that referenced this pull request Jan 16, 2018

Merge #543
543: fix: Only skip invalid meshes, instead of aborting the pass completely. r=Rhuagh a=Rhuagh



<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/amethyst/amethyst/543)
<!-- Reviewable:end -->
@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors bot Jan 17, 2018

Contributor

Timed out

Contributor

bors bot commented Jan 17, 2018

Timed out

@Xaeroxe

This comment has been minimized.

Show comment
Hide comment
@Xaeroxe

Xaeroxe Jan 17, 2018

Member

Overriding

Member

Xaeroxe commented Jan 17, 2018

Overriding

@Xaeroxe Xaeroxe merged commit d3b7d51 into amethyst:develop Jan 17, 2018

2 of 4 checks passed

bors Timed out
code-review/reviewable 3 files, 1 discussion left (omni-viral, Rhuagh)
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Rhuagh Rhuagh deleted the Rhuagh:fix/renderer-ignore-bad-meshes branch Jan 17, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment