-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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 geometry offset attribute for 2D and CV #6753
Conversation
@hpinkos, thanks for the pull request! Maintainers, we have a signed CLA from @hpinkos, so you can review this at any time.
I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome. 🌍 🌎 🌏 |
Fixed the eslint error |
Source/Scene/Primitive.js
Outdated
var offsetScratchCartesian = new Cartesian3(); | ||
function updateBatchTableOffsets(primitive, frameState) { | ||
var hasOffset = defined(primitive._batchTableAttributeIndices.offset); | ||
if (!hasOffset|| primitive._batchTableOffsetsUpdated || frameState.scene3DOnly) { |
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.
whitespace
Source/Scene/Primitive.js
Outdated
} | ||
|
||
var center = boundingSphere.center; | ||
center = ellipsoid.scaleToGeodeticSurface(center, center); |
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.
You don't want to modify the bounding sphere right? Should this use a scratch variable?
Thanks @bagnell ! Updated |
👍 |
The offset vector needs to be projected for 2D and columbus view
Also fixed the doc for
OffsetGeometryInstanceAttribute
Also, partial fix for #6365. I wrapped the distance display condition 2D bounding sphere calculations in
if (!frameState.scene3DOnly)
so the example to longer crashes whenscene3DOnly: true
.