-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Add silhouettes to ModelExperimental
#10457
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
Conversation
Thanks for the pull request @j9liu!
Reviewers, don't forget to make sure that:
|
Figured out what the bug was: in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@j9liu I had a few comments.
Also I tried the sandcastle and it seems to be rendering only the silhouette:
Specs/Scene/ModelExperimental/ModelExperimentalDrawCommandSpec.js
Outdated
Show resolved
Hide resolved
Specs/Scene/ModelExperimental/ModelExperimentalDrawCommandSpec.js
Outdated
Show resolved
Hide resolved
@ptrgags - updated, let me know if the uniform change from |
@ptrgags ready! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@j9liu this is getting close, it works even with custom shaders! though a point cloud without normals failed:
@ptrgags updated with a fix for the point cloud crash! |
Looks good, thanks @j9liu! |
This PR adds support for silhouettes in

ModelExperimental
, like so:I also found that
ModelExperimentalSpec
had a lot of invalid unit tests and was missing some forModelExperimental.color
, so I fixed those as well.Here's the Sandcastle for testing silhouettes.
Testing Notes
Model
, so they won't work here.One bug I still have to figure out: the "Instanced Box" model doesn't work with translucent silhouettes for some reason, even though all the other models work. I can't compare the results toFIXEDModel
either becauseModel
doesn't support GPU instancing, but the silhouette works for this model if the draw command passes are the same (opaque model color + silhouette, translucent model color + silhouette).