Increase max clipping planes using textures #6201
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,7 +50,7 @@ define([ | |
|
|
||
| /** | ||
| * Specifies a set of clipping planes. Clipping planes selectively disable rendering in a region on the | ||
| * outside of the specified list of {@link Plane} objects for a single gltf model, 3D Tileset, or the globe. | ||
| * | ||
| * @alias ClippingPlaneCollection | ||
| * @constructor | ||
|
|
@@ -107,20 +107,21 @@ define([ | |
| this.edgeWidth = defaultValue(options.edgeWidth, 0.0); | ||
|
|
||
| /** | ||
| * If this ClippingPlaneCollection has an owner, only its owner should update or destroy it. | ||
| * This is because in a Cesium3DTileset multiple models may reference the tileset's ClippingPlaneCollection. | ||
| * @private | ||
| * | ||
| * @type {Object} | ||
| * @default undefined | ||
| */ | ||
| this._owner = undefined; | ||
|
There was a problem hiding this comment. Since it's an underscored variable you can remove the doc but leave the comment. There was a problem hiding this comment. The indentation of the comment is slightly off. |
||
|
|
||
| this._testIntersection = undefined; | ||
| this._unionClippingRegions = undefined; | ||
| this.unionClippingRegions = defaultValue(options.unionClippingRegions, false); | ||
|
|
||
| // Avoid wastefully re-uploading textures when the clipping planes don't change between frames | ||
| var previousTextureBytes = new ArrayBuffer(ClippingPlaneCollection.TEXTURE_WIDTH * ClippingPlaneCollection.TEXTURE_WIDTH * 4); | ||
| var currentTextureBytes = new ArrayBuffer(ClippingPlaneCollection.TEXTURE_WIDTH * ClippingPlaneCollection.TEXTURE_WIDTH * 4); | ||
|
|
||
| this._textureBytes = new Uint8Array(currentTextureBytes); | ||
|
|
@@ -129,9 +130,6 @@ define([ | |
| this._previousUint32View = new Uint32Array(previousTextureBytes); | ||
| this._currentUint32View = new Uint32Array(currentTextureBytes); | ||
|
|
||
| this._clippingPlanesTexture = undefined; | ||
|
|
||
| // Packed uniform for plane count, denormalization parameters, and clipping union (0 false, 1 true) | ||
|
|
@@ -353,9 +351,10 @@ define([ | |
| } | ||
|
|
||
| var octEncodeScratch = new Cartesian2(); | ||
| var rightShift = 1.0 / 256.0; | ||
| /** | ||
| * Encodes a normalized vector into 4 SNORM values in the range [0-255] following the 'oct' encoding. | ||
|
There was a problem hiding this comment. Should the comment be [0-65535]? There was a problem hiding this comment. It's two 16-bit values spread out over 4 8-bit values. I was following the convention in There was a problem hiding this comment. It's storing 4 values for precision purposes right? You might want to note that in the comment. |
||
| * oct32 precision is higher than the default oct16, hence the additional 2 uint16 values. | ||
| */ | ||
| function oct32EncodeNormal(vector, result) { | ||
|
There was a problem hiding this comment. Is this specific to the clipping plane? Or should this be a utility function? Maybe a static function in There was a problem hiding this comment. That makes a lot of sense, but it seems like we'd also want to rename a bunch of stuff in @lilleyse what do you think? I guess we could also deprecate the current functions over time? There was a problem hiding this comment. Maybe the new function could be called |
||
| AttributeCompression.octEncodeInRange(vector, 65535, octEncodeScratch); | ||
|
|
@@ -373,7 +372,7 @@ define([ | |
| var textureBytes = clippingPlaneCollection._textureBytes; | ||
| var planes = clippingPlaneCollection._planes; | ||
| var length = planes.length; | ||
| var dirty = false; | ||
| var byteIndex, uint32Index; | ||
|
|
||
| var lengthRangeUnion = clippingPlaneCollection._lengthRangeUnion; | ||
|
|
@@ -396,9 +395,9 @@ define([ | |
| textureBytes[byteIndex + 2] = oct32Normal.z; | ||
| textureBytes[byteIndex + 3] = oct32Normal.w; | ||
|
|
||
| uint32Index = i * 2; | ||
| dirty = dirty || previousUint32View[uint32Index] !== currentUint32View[uint32Index]; | ||
| if (dirty) { | ||
| previousUint32View[uint32Index] = currentUint32View[uint32Index]; | ||
| } | ||
|
|
||
|
|
@@ -420,17 +419,17 @@ define([ | |
| byteIndex = i * 8 + 4; | ||
| insertFloat(textureBytes, normalizedDistance, byteIndex); | ||
|
|
||
| uint32Index = i * 2 + 1; | ||
| dirty = dirty || previousUint32View[uint32Index] !== currentUint32View[uint32Index]; | ||
| if (dirty) { | ||
| previousUint32View[uint32Index] = currentUint32View[uint32Index]; | ||
| } | ||
| } | ||
| var lengthRange = clippingPlaneCollection._lengthRange; | ||
| lengthRange.x = lengthRangeUnion.x; | ||
| lengthRange.y = lengthRangeUnion.y; | ||
| lengthRange.z = lengthRangeUnion.z; | ||
| return dirty; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -458,9 +457,6 @@ define([ | |
| }); | ||
| this._clippingPlanesTexture = clippingPlanesTexture; | ||
| } | ||
| // pack planes to currentTextureBytes, do a texture update if anything changed. | ||
| if (packAndReturnChanged(this)) { | ||
| clippingPlanesTexture.copyFrom({ | ||
|
|
@@ -556,12 +552,12 @@ define([ | |
|
|
||
| /** | ||
| * The maximum number of supported clipping planes. | ||
| * @private | ||
| * | ||
| * @type {number} | ||
| * @constant | ||
| */ | ||
| ClippingPlaneCollection.MAX_CLIPPING_PLANES = 2048; // See maxClippingPlanes.glsl | ||
|
|
||
| /** | ||
| * The pixel width of a square, power-of-two RGBA UNSIGNED_BYTE texture | ||
|
|
@@ -574,27 +570,62 @@ define([ | |
| */ | ||
| ClippingPlaneCollection.TEXTURE_WIDTH = CesiumMath.nextPowerOfTwo(Math.ceil(Math.sqrt(ClippingPlaneCollection.MAX_CLIPPING_PLANES * 2))); | ||
|
There was a problem hiding this comment. This should probably be marked There was a problem hiding this comment. Bump. Edit: add There was a problem hiding this comment. I wonder if it would make more sense to make this a 1D texture, at least it would simplify some shader code. I can't seem to load https://webglstats.com/webgl right now but we can check what % of devices support 2048 pixel textures. There was a problem hiding this comment. It might still be better to keep it as a POT though. I think I remember something about those being more efficient for GPU memory or something, although I also vaguely remember someone saying something about how that's not necessarily true anymore... There was a problem hiding this comment. We try to use 1D textures for the 3D Tiles batch table, so there is some precedent there. But it doesn't matter much in the end. |
||
|
|
||
| /** | ||
| * Returns true if this object was destroyed; otherwise, false. | ||
| * <br /><br /> | ||
| * If this object was destroyed, it should not be used; calling any function other than | ||
| * <code>isDestroyed</code> will result in a {@link DeveloperError} exception. | ||
| * | ||
| * @returns {Boolean} <code>true</code> if this object was destroyed; otherwise, <code>false</code>. | ||
| * | ||
| * @see ClippingPlaneCollection#destroy | ||
| */ | ||
| ClippingPlaneCollection.prototype.isDestroyed = function() { | ||
|
There was a problem hiding this comment. Add the usual doc above this: |
||
| return false; | ||
| }; | ||
|
|
||
| /** | ||
| * Destroys this ClippingPlaneCollectionand its WebGL resources. | ||
| * @private | ||
| */ | ||
|
There was a problem hiding this comment. This doc indentation is slightly misaligned. |
||
| ClippingPlaneCollection.prototype.destroy = function() { | ||
|
There was a problem hiding this comment. Round out the doc for this. For example: There was a problem hiding this comment. Also remove |
||
| this._clippingPlanesTexture = this._clippingPlanesTexture && this._clippingPlanesTexture.destroy(); | ||
| return destroyObject(this); | ||
| }; | ||
|
|
||
| /** | ||
| * Sets the owner for the input ClippingPlaneCollection if there wasn't another owner. | ||
| * Destroys the owner's previous ClippingPlaneCollection if setting is successful. | ||
| * | ||
| * @param {ClippingPlaneCollection} [clippingPlaneCollection] A ClippingPlaneCollection (or undefined) being attached to an object | ||
| * @param {Object} newOwner An Object that should receive the new ClippingPlaneCollection | ||
|
|
||
| * @param {String} key The Key for the Object to reference the ClippingPlaneCollection | ||
| * @private | ||
| */ | ||
| ClippingPlaneCollection.setOwnership = function(clippingPlaneCollection, newOwner, key) { | ||
|
There was a problem hiding this comment. Place |
||
| // Don't destroy the ClippingPlaneCollection if it is already owned by newOwner | ||
| if (clippingPlaneCollection === newOwner[key]) { | ||
| return; | ||
| } | ||
| // Destroy the existing ClippingPlaneCollection, if any | ||
| newOwner[key] = newOwner[key] && newOwner[key].destroy(); | ||
| if (defined(clippingPlaneCollection)) { | ||
| if (defined(clippingPlaneCollection._owner)) { | ||
| throw new DeveloperError('ClippingPlaneCollection should only be assigned to one object'); | ||
| } | ||
|
There was a problem hiding this comment. Wrap the developer error: |
||
| clippingPlaneCollection._owner = newOwner; | ||
| newOwner[key] = clippingPlaneCollection; | ||
| } | ||
|
There was a problem hiding this comment. We often simplify this type of destroy code to |
||
| }; | ||
|
|
||
| /** | ||
| * Determines if rendering with clipping planes is supported. | ||
| * | ||
| * @returns {Boolean} <code>true</code> if ClippingPlaneCollections are supported | ||
| * @deprecated | ||
| */ | ||
| ClippingPlaneCollection.isSupported = function() { | ||
| return true; | ||
|
There was a problem hiding this comment. This should have a There was a problem hiding this comment. @likangning93 did you add the deprecation warning here? |
||
| }; | ||
|
|
||
| return ClippingPlaneCollection; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -696,16 +696,5 @@ define([ | |
| return destroyObject(this); | ||
| }; | ||
|
|
||
| return Globe; | ||
| }); | ||
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.
Usually in Cesium code we condense this to: