Hide Outlines for 3D Models #9629
Conversation
|
Thanks for the pull request @srothst1!
Reviewers, don't forget to make sure that:
|
|
@ebogo1 It does not seem necessary to allow users to change this option after initialization. I have not seen anything in the community forum that suggests that this functionality is critical. |
Ideally it would be toggleable. However I don't think the current Model code is flexible enough for that right now, and it's not a dealbreaker for this PR. CC @ptrgags @sanjeetsuhag when you go to integrate |
Co-authored-by: Sean Lilley <lilleyse@gmail.com>
…js to include similar content to Batched3DModel3DTileContent.js
…ing the CESIUM_primative_outline extension
…stanced3DModel3DTileContent and ModelInstanceCollection�nd documented the showOutline option in createOsmBuildings
|
@lilleyse Thank you for your suggestions! I believe I have incorporated all of your feedback:
Let me know if you have any questions, comments, or concerns. |
|
@lilleyse I just
Hopefully I didn't miss anything - let me know! |
|
The code looks good! I just left one comment about testing. |
…wOutline: false and not call ModelOutlineLoader.outlinePrimitives
|
@lilleyse I just added a spec to |
Specs/Scene/ModelSpec.js
Outdated
| it("loads a glTF 2.0 model with showOutline set as false", function () { | ||
| return loadModel(boxGltf2Url).then(function (m) { | ||
| verifyRender(m); | ||
| m.show = true; | ||
| m.luminanceAtZenith = undefined; | ||
| m.showOutline = false; | ||
| spyOn(ModelOutlineLoader, "outlinePrimitives"); | ||
|
|
||
| expect({ | ||
| scene: scene, | ||
| time: JulianDate.fromDate(new Date("January 1, 2014 12:00:00 UTC")), | ||
| }).toRenderAndCall(function (rgba) { | ||
| expect(rgba).toEqualEpsilon([179, 9, 9, 255], 5); // Red | ||
| }); | ||
| expect(ModelOutlineLoader.outlinePrimitives).not.toHaveBeenCalled(); | ||
|
|
||
| primitives.remove(m); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The test can be simplified by using the options parameter for loadModel
it("loads a glTF 2.0 model with showOutline set as false", function () {
spyOn(ModelOutlineLoader, "outlinePrimitives");
return loadModel(boxGltf2Url, {
showOutline: false
}).then(function (m) {
expect(ModelOutlineLoader.outlinePrimitives).not.toHaveBeenCalled();
primitives.remove(m);
});
});
|
@lilleyse just committed your suggestions. |
|
Thanks @srothst1! |
Fixes #8959
Added a new property
ignoreOutlineto Model.js and Cesium3DTileset.js. Based on this property, the lineModelOutlineLoader.outlinePrimitives(this);in Model.js will be either included or omitted. This exposes the ability to turn off outlines on 3D models.The current issue that I am facing is that
this.ignoreOutlineis not being updated correctly inModel.js. I created a sandcastle demo that showcases this issue. Please let me know what your suggestions are for resolving this.Lastly, there was a discussion in #8959 about where this new property should be implemented. It is unclear if this feature should only be implemented for Cesium OSM Buildings. Feedback on this matter would also be apricated.
@ebogo1
@lilleyse