Skip to content
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 crash due to undefined clipping planes texture #6576

Merged
merged 7 commits into from
May 25, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ Change Log
* Fixed incorrect 3D Tiles statistics when a tile fails during processing. [#6558](https://github.com/AnalyticalGraphicsInc/cesium/pull/6558)
* Fixed race condition causing intermittent crash when changing geometry show value [#3061](https://github.com/AnalyticalGraphicsInc/cesium/issues/3061)
* `ProviderViewModel`s with no category are displayed in an untitled group in `BaseLayerPicker` instead of being labeled as `'Other'` [#6574](https://github.com/AnalyticalGraphicsInc/cesium/pull/6574)
* Fixed a bug causing intermittent crashes with clipping planes due to uninitialized textures. [#6576](https://github.com/AnalyticalGraphicsInc/cesium/pull/6576)
* Added a workaround for clipping planes causing a picking shader compilation failure for gltf models and 3D Tilesets in Internet Explorer [#6575](https://github.com/AnalyticalGraphicsInc/cesium/issues/6575)

### 1.45 - 2018-05-01
Expand Down
6 changes: 6 additions & 0 deletions Source/Scene/Batched3DModel3DTileContent.js
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,12 @@ define([
this._model._clippingPlanes = (tilesetClippingPlanes.enabled && this._tile._isClipped) ? tilesetClippingPlanes : undefined;
}

// If the model references a different ClippingPlaneCollection due to the tileset's collection being replaced with a
// ClippingPlaneCollection that gives this tile the same clipping status, update the model to use the new ClippingPlaneCollection.
if (defined(tilesetClippingPlanes) && defined(this._model._clippingPlanes) && this._model._clippingPlanes !== tilesetClippingPlanes) {
this._model._clippingPlanes = tilesetClippingPlanes;
}

this._model.update(frameState);

// If any commands were pushed, add derived commands
Expand Down
40 changes: 37 additions & 3 deletions Source/Scene/ClippingPlaneCollection.js
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,8 @@ define([

if (!defined(clippingPlanesTexture)) {
// Allocate twice as much space as needed to avoid frequent texture reallocation.
requiredResolution.x *= 2;
// Allocate in the Y direction, since texture may be as wide as context texture support.
requiredResolution.y *= 2;

var sampler = new Sampler({
wrapS : TextureWrap.CLAMP_TO_EDGE,
Expand Down Expand Up @@ -502,16 +503,22 @@ define([
}
if (!this._multipleDirtyPlanes) {
// partial updates possible
var offsetY = Math.floor(dirtyIndex / clippingPlanesTexture.width);
var offsetX = Math.floor(dirtyIndex - offsetY * clippingPlanesTexture.width);
var offsetX = 0;
var offsetY = 0;
if (useFloatTexture) {
offsetY = Math.floor(dirtyIndex / clippingPlanesTexture.width);
offsetX = Math.floor(dirtyIndex - offsetY * clippingPlanesTexture.width);

packPlanesAsFloats(this, dirtyIndex, dirtyIndex + 1);
clippingPlanesTexture.copyFrom({
width : 1,
height : 1,
arrayBufferView : this._float32View
}, offsetX, offsetY);
} 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,
Expand Down Expand Up @@ -665,6 +672,33 @@ define([
return context.floatingPointTexture;
};

/**
* Function for getting the clipping plane collection's texture resolution.
* If the ClippingPlaneCollection hasn't been updated, returns the resolution that will be
* allocated based on the current plane count.
*
* @param {ClippingPlaneCollection} clippingPlaneCollection The clipping plane collection
* @param {Context} context The rendering context
* @param {Cartesian2} result A Cartesian2 for the result.
* @returns {Cartesian2} The required resolution.
* @private
*/
ClippingPlaneCollection.getTextureResolution = function(clippingPlaneCollection, context, result) {
var texture = clippingPlaneCollection.texture;
if (defined(texture)) {
result.x = texture.width;
result.y = texture.height;
return result;
}

var pixelsNeeded = ClippingPlaneCollection.useFloatTexture(context) ? clippingPlaneCollection.length : clippingPlaneCollection.length * 2;
var requiredResolution = computeTextureResolution(pixelsNeeded, result);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the error message it looks like this might be returning NaN because pixelsNeeded is 0.
Don't know how that's happening though, because the clipping plane shader regen shouldn't be called in that case. Wish we were able to reproduce the error for debugging... I'll keep trying to trace this in the meantime.


// Allocate twice as much space as needed to avoid frequent texture reallocation.
requiredResolution.y *= 2;
return requiredResolution;
};

/**
* Returns true if this object was destroyed; otherwise, false.
* <br /><br />
Expand Down
2 changes: 1 addition & 1 deletion Source/Scene/GlobeSurfaceShaderSet.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ define([
var fs = this.baseFragmentShaderSource.clone();

if (currentClippingShaderState !== 0) {
fs.sources.unshift(getClippingFunction(clippingPlanes)); // Need to go before GlobeFS
fs.sources.unshift(getClippingFunction(clippingPlanes, frameState.context)); // Need to go before GlobeFS
}

vs.defines.push(quantizationDefine);
Expand Down
6 changes: 6 additions & 0 deletions Source/Scene/Instanced3DModel3DTileContent.js
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,12 @@ define([
// Link/Dereference directly to avoid ownership checks.
model._clippingPlanes = (tilesetClippingPlanes.enabled && this._tile._isClipped) ? tilesetClippingPlanes : undefined;
}

// If the model references a different ClippingPlaneCollection due to the tileset's collection being replaced with a
// ClippingPlaneCollection that gives this tile the same clipping status, update the model to use the new ClippingPlaneCollection.
if (defined(tilesetClippingPlanes) && defined(model._clippingPlanes) && model._clippingPlanes !== tilesetClippingPlanes) {
model._clippingPlanes = tilesetClippingPlanes;
}
}

this._modelInstanceCollection.update(frameState);
Expand Down
8 changes: 4 additions & 4 deletions Source/Scene/Model.js
Original file line number Diff line number Diff line change
Expand Up @@ -2043,7 +2043,7 @@ define([
finalFS = Model._modifyShaderForColor(finalFS, model._hasPremultipliedAlpha);
}
if (addClippingPlaneCode) {
finalFS = modifyShaderForClippingPlanes(finalFS, clippingPlaneCollection);
finalFS = modifyShaderForClippingPlanes(finalFS, clippingPlaneCollection, context);
}

var drawVS = modifyShader(vs, id, model._vertexShaderLoaded);
Expand All @@ -2066,7 +2066,7 @@ define([
}

if (addClippingPlaneCode) {
pickFS = modifyShaderForClippingPlanes(pickFS, clippingPlaneCollection);
pickFS = modifyShaderForClippingPlanes(pickFS, clippingPlaneCollection, context);
}

if (!FeatureDetection.isInternetExplorer()) {
Expand Down Expand Up @@ -3960,9 +3960,9 @@ define([
}
}

function modifyShaderForClippingPlanes(shader, clippingPlaneCollection) {
function modifyShaderForClippingPlanes(shader, clippingPlaneCollection, context) {
shader = ShaderSource.replaceMain(shader, 'gltf_clip_main');
shader += Model._getClippingFunction(clippingPlaneCollection) + '\n';
shader += Model._getClippingFunction(clippingPlaneCollection, context) + '\n';
shader +=
'uniform sampler2D gltf_clippingPlanes; \n' +
'uniform mat4 gltf_clippingPlanesMatrix; \n' +
Expand Down
2 changes: 1 addition & 1 deletion Source/Scene/PointCloud3DTileContent.js
Original file line number Diff line number Diff line change
Expand Up @@ -1126,7 +1126,7 @@ define([
'uniform mat4 u_clippingPlanesMatrix; \n' +
'uniform vec4 u_clippingPlanesEdgeStyle; \n';
fs += '\n';
fs += getClippingFunction(clippingPlanes);
fs += getClippingFunction(clippingPlanes, context);
fs += '\n';
}

Expand Down
27 changes: 17 additions & 10 deletions Source/Scene/getClippingFunction.js
Original file line number Diff line number Diff line change
@@ -1,36 +1,43 @@
define([
'../Core/Cartesian2',
'../Core/Check',
'../Renderer/PixelDatatype'
'../Renderer/PixelDatatype',
'./ClippingPlaneCollection'
], function(
Cartesian2,
Check,
PixelDatatype) {
PixelDatatype,
ClippingPlaneCollection) {
'use strict';

var textureResolutionScratch = new Cartesian2();
/**
* Gets the GLSL functions needed to retrieve clipping planes from a ClippingPlaneCollection's texture.
*
* @param {ClippingPlaneCollection} clippingPlaneCollection ClippingPlaneCollection with a defined texture.
* @param {Context} context The current rendering context.
* @returns {String} A string containing GLSL functions for retrieving clipping planes.
* @private
*/
function getClippingFunction(clippingPlaneCollection) {
function getClippingFunction(clippingPlaneCollection, context) {
//>>includeStart('debug', pragmas.debug);
Check.typeOf.object('clippingPlaneCollection', clippingPlaneCollection);
Check.typeOf.object('context', context);
//>>includeEnd('debug');
var unionClippingRegions = clippingPlaneCollection.unionClippingRegions;
var clippingPlanesLength = clippingPlaneCollection.length;
var texture = clippingPlaneCollection.texture;
var usingFloatTexture = texture.pixelDatatype === PixelDatatype.FLOAT;
var width = texture.width;
var height = texture.height;
var usingFloatTexture = ClippingPlaneCollection.useFloatTexture(context);
var textureResolution = ClippingPlaneCollection.getTextureResolution(clippingPlaneCollection, context, textureResolutionScratch);
var width = textureResolution.x;
var height = textureResolution.y;

var functions = usingFloatTexture ? getClippingPlaneFloat(width, height) : getClippingPlaneUint8(width, height);
functions += '\n';
functions += unionClippingRegions ? clippingFunctionUnion(usingFloatTexture, clippingPlanesLength) : clippingFunctionIntersect(usingFloatTexture, clippingPlanesLength);
functions += unionClippingRegions ? clippingFunctionUnion(clippingPlanesLength) : clippingFunctionIntersect(clippingPlanesLength);
return functions;
}

function clippingFunctionUnion(usingFloatTexture, clippingPlanesLength) {
function clippingFunctionUnion(clippingPlanesLength) {
var functionString =
'float clip(vec4 fragCoord, sampler2D clippingPlanes, mat4 clippingPlanesMatrix)\n' +
'{\n' +
Expand Down Expand Up @@ -66,7 +73,7 @@ define([
return functionString;
}

function clippingFunctionIntersect(usingFloatTexture, clippingPlanesLength) {
function clippingFunctionIntersect(clippingPlanesLength) {
var functionString =
'float clip(vec4 fragCoord, sampler2D clippingPlanes, mat4 clippingPlanesMatrix)\n' +
'{\n' +
Expand Down
33 changes: 33 additions & 0 deletions Specs/Scene/Batched3DModel3DTileContentSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,39 @@ defineSuite([
});
});

it('Links model to tileset clipping planes if tileset clipping planes are reassigned', function() {
return Cesium3DTilesTester.loadTileset(scene, withBatchTableUrl).then(function(tileset) {
var tile = tileset._root;
var model = tile.content._model;

expect(model.clippingPlanes).toBeUndefined();

var clippingPlaneCollection = new ClippingPlaneCollection({
planes : [
new ClippingPlane(Cartesian3.UNIT_X, 0.0)
]
});
tileset.clippingPlanes = clippingPlaneCollection;
clippingPlaneCollection.update(scene.frameState);
tile.update(tileset, scene.frameState);

expect(model.clippingPlanes).toBeDefined();
expect(model.clippingPlanes).toBe(tileset.clippingPlanes);

var newClippingPlaneCollection = new ClippingPlaneCollection({
planes : [
new ClippingPlane(Cartesian3.UNIT_X, 0.0)
]
});
tileset.clippingPlanes = newClippingPlaneCollection;
newClippingPlaneCollection.update(scene.frameState);
expect(model.clippingPlanes).not.toBe(tileset.clippingPlanes);

tile.update(tileset, scene.frameState);
expect(model.clippingPlanes).toBe(tileset.clippingPlanes);
});
});

it('rebuilds Model shaders when clipping planes change', function() {
spyOn(Model, '_getClippingFunction').and.callThrough();

Expand Down
Loading