-
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 clipping planes for tiles with RTC and/or no transforms #7034
Changes from 21 commits
ad7abcd
9f55539
9ed3695
bdb8799
6cb0426
aa71e96
0ec49a5
0a07e56
5aee22a
1f1cd16
8996da6
7d5bb57
b605a61
0d2dd5f
814c5ae
dd3a7c6
9db4500
2dac9f4
2fadcf9
b251c6b
e879cca
5245643
65a8496
3cc7aa0
888e2b2
f56c71c
911a520
a43ab57
79a49e0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,6 +55,13 @@ define([ | |
/** | ||
* Specifies a set of clipping planes. Clipping planes selectively disable rendering in a region on the | ||
* outside of the specified list of {@link ClippingPlane} objects for a single gltf model, 3D Tileset, or the globe. | ||
* <p> | ||
* In general the clipping planes' coordinates are relative to the object they're attached to, so a plane with distance set to 0 will clip | ||
* through the center of the object. | ||
* </p> | ||
* <p> | ||
* For 3D Tiles, the root tile's transform is used to position the clipping planes. If a transform is not defined, the root tile's {@link Cesium3DTile#boundingSphere} is used instead. | ||
* </p> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also add a similar note explaining what the clipping plane's coordinate system is when using it on a model and on the globe. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you think this is necessary? I was thinking the comment above that "clipping planes' coordinates are relative to the object they're attached to" covers the globe and individual models, since it would just be relative to the center of the globe and the model respectively. Since 3D Tiles are a bit more ambiguous I think it needs the extra explanation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alright makes sense. |
||
* | ||
* @alias ClippingPlaneCollection | ||
* @constructor | ||
|
@@ -66,6 +73,30 @@ define([ | |
* @param {Boolean} [options.unionClippingRegions=false] If true, a region will be clipped if included in any plane in the collection. Otherwise, the region to be clipped must intersect the regions defined by all planes in this collection. | ||
* @param {Color} [options.edgeColor=Color.WHITE] The color applied to highlight the edge along which an object is clipped. | ||
* @param {Number} [options.edgeWidth=0.0] The width, in pixels, of the highlight applied to the edge along which an object is clipped. | ||
* | ||
* @demo {@link https://cesiumjs.org/Cesium/Build/Apps/Sandcastle/?src=3D%20Tiles%20Clipping%20Planes.html|Clipping 3D Tiles and glTF models.} | ||
* @demo {@link https://cesiumjs.org/Cesium/Build/Apps/Sandcastle/?src=Terrain%20Clipping%20Planes.html|Clipping the Globe.} | ||
* | ||
* @example | ||
* // This clipping plane's distance is positive, which means its normal | ||
* // is facing the origin. This will clip everything that is behind | ||
* // the plane, which is anything with y coordinate > 5. | ||
* var clippingPlanes = new Cesium.ClippingPlaneCollection({ | ||
* planes : [ | ||
* new Cesium.ClippingPlane(new Cesium.Cartesian3(0.0, 1.0, 0.0), 5.0) | ||
* ], | ||
* }); | ||
* // Create an entity and attach the ClippingPlaneCollection to the model. | ||
* var entity = viewer.entities.add({ | ||
* position : Cesium.Cartesian3.fromDegrees(-123.0744619, 44.0503706, 10000), | ||
* model : { | ||
* uri : 'model.gltf', | ||
* minimumPixelSize : 128, | ||
* maximumScale : 20000, | ||
* clippingPlanes : clippingPlanes | ||
* } | ||
* }); | ||
* viewer.zoomTo(entity); | ||
*/ | ||
function ClippingPlaneCollection(options) { | ||
options = defaultValue(options, defaultValue.EMPTY_OBJECT); | ||
|
@@ -587,7 +618,7 @@ define([ | |
|
||
var modelMatrix = this.modelMatrix; | ||
if (defined(transform)) { | ||
modelMatrix = Matrix4.multiply(modelMatrix, transform, scratchMatrix); | ||
modelMatrix = Matrix4.multiply(transform, modelMatrix, scratchMatrix); | ||
} | ||
|
||
// If the collection is not set to union the clipping regions, the volume must be outside of all planes to be | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -539,6 +539,10 @@ define([ | |
this.clippingPlanes = options.clippingPlanes; | ||
// Used for checking if shaders need to be regenerated due to clipping plane changes. | ||
this._clippingPlanesState = 0; | ||
// If defined, use this matrix to position the clipping planes instead of the modelMatrix. | ||
// This is so that when models are part of a tileset they all get clipped relative | ||
// to the root tile. | ||
this._clippingPlaneOffsetMatrix = undefined; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Slight wording tweak |
||
|
||
/** | ||
* This property is for debugging only; it is not for production use nor is it optimized. | ||
|
@@ -594,7 +598,7 @@ define([ | |
this.opaquePass = defaultValue(options.opaquePass, Pass.OPAQUE); | ||
|
||
this._computedModelMatrix = new Matrix4(); // Derived from modelMatrix and scale | ||
this._modelViewMatrix = Matrix4.clone(Matrix4.IDENTITY); // Derived from modelMatrix, scale, and the current view matrix | ||
this._clippingPlaneModelViewMatrix = Matrix4.clone(Matrix4.IDENTITY); // Derived from modelMatrix, scale, and the current view matrix | ||
this._initialRadius = undefined; // Radius without model's scale property, model-matrix scale, animations, or skins | ||
this._boundingSphere = undefined; | ||
this._scaledBoundingSphere = new BoundingSphere(); | ||
|
@@ -3009,7 +3013,7 @@ define([ | |
if (!defined(clippingPlanes)) { | ||
return Matrix4.IDENTITY; | ||
} | ||
return Matrix4.multiply(model._modelViewMatrix, clippingPlanes.modelMatrix, scratchClippingPlaneMatrix); | ||
return Matrix4.multiply(model._clippingPlaneModelViewMatrix, clippingPlanes.modelMatrix, scratchClippingPlaneMatrix); | ||
}; | ||
} | ||
|
||
|
@@ -4426,7 +4430,8 @@ define([ | |
var clippingPlanes = this._clippingPlanes; | ||
var currentClippingPlanesState = 0; | ||
if (defined(clippingPlanes) && clippingPlanes.enabled) { | ||
Matrix4.multiply(context.uniformState.view3D, modelMatrix, this._modelViewMatrix); | ||
var clippingPlaneOffsetMatrix = defaultValue(this._clippingPlaneOffsetMatrix, modelMatrix); | ||
Matrix4.multiply(context.uniformState.view3D, clippingPlaneOffsetMatrix, this._clippingPlaneModelViewMatrix); | ||
currentClippingPlanesState = clippingPlanes.clippingPlanesState; | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -185,6 +185,10 @@ define([ | |
this.clippingPlanes = undefined; | ||
this.isClipped = false; | ||
this.clippingPlanesDirty = false; | ||
// If defined, use this matrix to position the clipping planes instead of the modelMatrix. | ||
// This is so that when point clouds are part of a tileset they all get clipped relative | ||
// to the root tile. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment here. |
||
this._clippingPlaneOffsetMatrix = undefined; | ||
|
||
this.attenuation = false; | ||
this._attenuation = false; | ||
|
@@ -819,8 +823,13 @@ define([ | |
if (!defined(clippingPlanes)) { | ||
return Matrix4.IDENTITY; | ||
} | ||
var modelViewMatrix = Matrix4.multiply(context.uniformState.view3D, pointCloud._modelMatrix, scratchClippingPlaneMatrix); | ||
return Matrix4.multiply(modelViewMatrix, clippingPlanes.modelMatrix, scratchClippingPlaneMatrix); | ||
|
||
if (!defined(pointCloud._clippingPlaneOffsetMatrix)) { | ||
Matrix4.multiply(context.uniformState.view3D, pointCloud._modelMatrix, scratchClippingPlaneMatrix); | ||
} else { | ||
Matrix4.multiply(context.uniformState.view3D, pointCloud._clippingPlaneOffsetMatrix, scratchClippingPlaneMatrix); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use |
||
return Matrix4.multiply(scratchClippingPlaneMatrix, clippingPlanes.modelMatrix, scratchClippingPlaneMatrix); | ||
} | ||
}; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -297,6 +297,8 @@ define([ | |
var styleDirty = this._styleDirty; | ||
this._styleDirty = false; | ||
|
||
pointCloud._clippingPlaneOffsetMatrix = tileset.clippingPlaneOffsetMatrix; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably best to rename In general any variable that can be set from another file doesn't use underscores. |
||
|
||
pointCloud.style = defined(batchTable) ? undefined : tileset.style; | ||
pointCloud.styleDirty = styleDirty; | ||
pointCloud.modelMatrix = tile.computedTransform; | ||
|
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.
The point cloud example might be a good use case showing when to use the clipping plane's model matrix since the clipping plane is visually off-center from the church due to the offset bounding sphere.
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.
I tried doing this but one issue is that, you only really need to offset the entity, so it wouldn't really need to use the clipping planes' model matrix.
The other issue is that it requires converting back and forth from Cartographic to Cartesian and it kind of makes creating a clipping plane look complicated.
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.
Makes sense too. And I remember now that the terrain example uses a model matrix, so one example is enough to illustrate the point.