diff --git a/CHANGES.md b/CHANGES.md index 7b32c1ef2be5..2ef107612685 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -22,6 +22,7 @@ Change Log * Fixed a crash when using `BingMapsGeocoderService` [#7143](https://github.com/AnalyticalGraphicsInc/cesium/issues/7143) * Fixed entity visibility issue related to setting an entity show property and altering or adding entity geometry [#7156](https://github.com/AnalyticalGraphicsInc/cesium/pull/7156) * Fixed accuracy of rotation matrix generated by `VelocityOrientationProperty` [#6641](https://github.com/AnalyticalGraphicsInc/cesium/pull/6641) +* Fixed clipping plane crash when adding a plane to an empty collection. [#7168](https://github.com/AnalyticalGraphicsInc/cesium/pull/7168) ### 1.50 - 2018-10-01 diff --git a/Source/Scene/ClippingPlaneCollection.js b/Source/Scene/ClippingPlaneCollection.js index 31d9265dff91..416d7592445c 100644 --- a/Source/Scene/ClippingPlaneCollection.js +++ b/Source/Scene/ClippingPlaneCollection.js @@ -470,10 +470,8 @@ define([ function computeTextureResolution(pixelsNeeded, result) { var maxSize = ContextLimits.maximumTextureSize; - var width = Math.min(pixelsNeeded, maxSize); - var height = Math.ceil(pixelsNeeded / width); - result.x = Math.max(width, 1); - result.y = Math.max(height, 1); + result.x = Math.min(pixelsNeeded, maxSize); + result.y = Math.ceil(pixelsNeeded / result.x); return result; } @@ -495,7 +493,6 @@ define([ // In RGBA UNSIGNED_BYTE, A plane is a float in [0, 1) packed to RGBA and an Oct32 quantized normal, // so 8 bits or 2 pixels in RGBA. var pixelsNeeded = useFloatTexture ? this.length : this.length * 2; - var requiredResolution = computeTextureResolution(pixelsNeeded, textureResolutionScratch); if (defined(clippingPlanesTexture)) { var currentPixelCount = clippingPlanesTexture.width * clippingPlanesTexture.height; @@ -508,10 +505,17 @@ define([ pixelsNeeded < 0.25 * currentPixelCount) { clippingPlanesTexture.destroy(); clippingPlanesTexture = undefined; + this._clippingPlanesTexture = undefined; } } + // If there are no clipping planes, there's nothing to update. + if (this.length === 0) { + return; + } + if (!defined(clippingPlanesTexture)) { + var requiredResolution = computeTextureResolution(pixelsNeeded, textureResolutionScratch); // Allocate twice as much space as needed to avoid frequent texture reallocation. // Allocate in the Y direction, since texture may be as wide as context texture support. requiredResolution.y *= 2; @@ -572,7 +576,6 @@ define([ } else { offsetY = Math.floor((dirtyIndex * 2) / clippingPlanesTexture.width); offsetX = Math.floor((dirtyIndex * 2) - offsetY * clippingPlanesTexture.width); - packPlanesAsUint8(this, dirtyIndex, dirtyIndex + 1); clippingPlanesTexture.copyFrom({ width : 2, diff --git a/Source/Scene/GlobeSurfaceShaderSet.js b/Source/Scene/GlobeSurfaceShaderSet.js index 8696f18d7f38..ebf0d4e38ea6 100644 --- a/Source/Scene/GlobeSurfaceShaderSet.js +++ b/Source/Scene/GlobeSurfaceShaderSet.js @@ -147,7 +147,7 @@ define([ (colorCorrect << 22); var currentClippingShaderState = 0; - if (defined(clippingPlanes)) { + if (defined(clippingPlanes) && clippingPlanes.length > 0) { currentClippingShaderState = enableClippingPlanes ? clippingPlanes.clippingPlanesState : 0; } var surfaceShader = surfaceTile.surfaceShader; diff --git a/Source/Scene/Model.js b/Source/Scene/Model.js index ffa122a1b0ed..e804898c2326 100644 --- a/Source/Scene/Model.js +++ b/Source/Scene/Model.js @@ -1150,7 +1150,7 @@ define([ function isClippingEnabled(model) { var clippingPlanes = model._clippingPlanes; - return defined(clippingPlanes) && clippingPlanes.enabled; + return defined(clippingPlanes) && clippingPlanes.enabled && clippingPlanes.length !== 0; } /** @@ -4344,7 +4344,7 @@ define([ // Regenerate shaders if ClippingPlaneCollection state changed or it was removed var clippingPlanes = this._clippingPlanes; var currentClippingPlanesState = 0; - if (defined(clippingPlanes) && clippingPlanes.enabled) { + if (defined(clippingPlanes) && clippingPlanes.enabled && clippingPlanes.length > 0) { var clippingPlaneOffsetMatrix = defaultValue(this.clippingPlaneOffsetMatrix, modelMatrix); Matrix4.multiply(context.uniformState.view3D, clippingPlaneOffsetMatrix, this._clippingPlaneModelViewMatrix); currentClippingPlanesState = clippingPlanes.clippingPlanesState; diff --git a/Specs/Scene/ClippingPlaneCollectionSpec.js b/Specs/Scene/ClippingPlaneCollectionSpec.js index c801d081bc6d..09748c4c5fe6 100644 --- a/Specs/Scene/ClippingPlaneCollectionSpec.js +++ b/Specs/Scene/ClippingPlaneCollectionSpec.js @@ -219,6 +219,21 @@ defineSuite([ scene.destroyForSpecs(); }); + it('only creates texture when planes are added', function() { + var scene = createScene(); + + clippingPlanes = new ClippingPlaneCollection(); + clippingPlanes.update(scene.frameState); + expect(clippingPlanes.texture).toBeUndefined(); + + clippingPlanes.add(planes[0]); + clippingPlanes.update(scene.frameState); + + expect(clippingPlanes.texture).toBeDefined(); + expect(isNaN(clippingPlanes.texture.width)).toBe(false); + expect(isNaN(clippingPlanes.texture.height)).toBe(false); + }); + it('update fills the clipping plane texture with packed planes', function() { var scene = createScene(); @@ -399,6 +414,30 @@ defineSuite([ scene.destroyForSpecs(); }); + it('only creates texture when planes are added', function() { + var scene = createScene(); + + if (!ClippingPlaneCollection.useFloatTexture(scene.context)) { + // Don't fail just because float textures aren't supported + scene.destroyForSpecs(); + return; + } + + clippingPlanes = new ClippingPlaneCollection(); + clippingPlanes.update(scene.frameState); + expect(clippingPlanes.texture).toBeUndefined(); + + clippingPlanes.add(planes[0]); + clippingPlanes.update(scene.frameState); + + expect(clippingPlanes.texture).toBeDefined(); + expect(isNaN(clippingPlanes.texture.width)).toBe(false); + expect(isNaN(clippingPlanes.texture.height)).toBe(false); + + clippingPlanes.destroy(); + scene.destroyForSpecs(); + }); + it('update fills the clipping plane texture with packed planes', function() { var scene = createScene();