From eb7b76ece10dd4422a9377b94df8ac725c97b730 Mon Sep 17 00:00:00 2001 From: Srinivas Kaza Date: Wed, 7 Jun 2017 11:58:16 -0400 Subject: [PATCH 1/5] Adds normalShading option to the repo --- Source/Scene/PointCloud3DTileContent.js | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/Source/Scene/PointCloud3DTileContent.js b/Source/Scene/PointCloud3DTileContent.js index 13aad77dc4b3..1210634657ab 100644 --- a/Source/Scene/PointCloud3DTileContent.js +++ b/Source/Scene/PointCloud3DTileContent.js @@ -101,6 +101,10 @@ define([ this.backFaceCulling = false; this._backFaceCulling = false; + // Whether or not the user enabled normal shading + this.normalShading = true; + this._normalShading = true; + this._opaqueRenderState = undefined; this._translucentRenderState = undefined; @@ -781,8 +785,11 @@ define([ var hasNormals = content._hasNormals; var hasBatchIds = content._hasBatchIds; var backFaceCulling = content._backFaceCulling; + var normalShading = content._normalShading; var vertexArray = content._drawCommand.vertexArray; + var normalsEnabled = hasNormals && normalShading; + var colorStyleFunction; var showStyleFunction; var pointSizeStyleFunction; @@ -834,7 +841,7 @@ define([ var userProperties = styleableProperties.filter(function(property) { return defaultProperties.indexOf(property) === -1; }); //>>includeStart('debug', pragmas.debug); - if (usesNormalSemantic && !hasNormals) { + if (usesNormalSemantic && !normalsEnabled) { throw new DeveloperError('Style references the NORMAL semantic but the point cloud does not have normals'); } //>>includeEnd('debug'); @@ -863,7 +870,7 @@ define([ if (usesColors) { attributeLocations.a_color = colorLocation; } - if (hasNormals) { + if (normalsEnabled) { attributeLocations.a_normal = normalLocation; } if (hasBatchIds) { @@ -919,7 +926,7 @@ define([ vs += 'attribute vec3 a_color; \n'; } } - if (hasNormals) { + if (normalsEnabled) { if (isOctEncoded16P) { vs += 'attribute vec2 a_normal; \n'; } else { @@ -976,7 +983,7 @@ define([ } vs += ' vec3 position_absolute = vec3(czm_model * vec4(position, 1.0)); \n'; - if (hasNormals) { + if (normalsEnabled) { if (isOctEncoded16P) { vs += ' vec3 normal = czm_octDecode(a_normal); \n'; } else { @@ -1002,7 +1009,7 @@ define([ vs += ' color = color * u_highlightColor; \n'; - if (hasNormals) { + if (normalsEnabled) { vs += ' normal = czm_normal * normal; \n' + ' float diffuseStrength = czm_getLambertDiffuse(czm_sunDirectionEC, normal); \n' + ' diffuseStrength = max(diffuseStrength, 0.4); \n' + // Apply some ambient lighting @@ -1012,7 +1019,7 @@ define([ vs += ' v_color = color; \n' + ' gl_Position = czm_modelViewProjection * vec4(position, 1.0); \n'; - if (hasNormals && backFaceCulling) { + if (normalsEnabled && backFaceCulling) { vs += ' float visible = step(-normal.z, 0.0); \n' + ' gl_Position *= visible; \n'; } @@ -1215,6 +1222,11 @@ define([ createShaders(this, frameState, tileset.style); } + if (this.normalShading !== this._normalShading) { + this._normalShading = this.normalShading; + createShaders(this, frameState, tileset.style); + } + // Update the render state var isTranslucent = (this._highlightColor.alpha < 1.0) || (this._constantColor.alpha < 1.0) || this._styleTranslucent; this._drawCommand.renderState = isTranslucent ? this._translucentRenderState : this._opaqueRenderState; From 62c6e54904dc9a5d054b6e024ff7766bf4ddc84f Mon Sep 17 00:00:00 2001 From: Srinivas Kaza Date: Thu, 8 Jun 2017 10:45:49 -0400 Subject: [PATCH 2/5] Adds test for changing normal shading settings --- Specs/Scene/PointCloud3DTileContentSpec.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Specs/Scene/PointCloud3DTileContentSpec.js b/Specs/Scene/PointCloud3DTileContentSpec.js index 027b178e36bc..470c4a8b8b1e 100644 --- a/Specs/Scene/PointCloud3DTileContentSpec.js +++ b/Specs/Scene/PointCloud3DTileContentSpec.js @@ -549,6 +549,17 @@ defineSuite([ }); }); + it('rebuilds shader when normalShading changes', function() { + return Cesium3DTilesTester.loadTileset(scene, pointCloudQuantizedOctEncodedUrl).then(function(tileset) { + var content = tileset._root.content; + content.normalShading = true; + expect(scene).toRenderAndCall(function(rgba) { + content.normalShading = false; + expect(scene).notToRender(rgba); + }); + }); + }); + it('applies shader style to point cloud with normals', function() { return Cesium3DTilesTester.loadTileset(scene, pointCloudQuantizedOctEncodedUrl).then(function(tileset) { tileset.style = new Cesium3DTileStyle({ From 106436cfcd9d97a60a69059284d1e174b3da8492 Mon Sep 17 00:00:00 2001 From: Srinivas Kaza Date: Thu, 8 Jun 2017 11:37:56 -0400 Subject: [PATCH 3/5] backFaceCulling no longer depends on normalShading --- Source/Scene/PointCloud3DTileContent.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/Scene/PointCloud3DTileContent.js b/Source/Scene/PointCloud3DTileContent.js index 1210634657ab..78a43917bffe 100644 --- a/Source/Scene/PointCloud3DTileContent.js +++ b/Source/Scene/PointCloud3DTileContent.js @@ -1009,7 +1009,7 @@ define([ vs += ' color = color * u_highlightColor; \n'; - if (normalsEnabled) { + if (hasNormals) { vs += ' normal = czm_normal * normal; \n' + ' float diffuseStrength = czm_getLambertDiffuse(czm_sunDirectionEC, normal); \n' + ' diffuseStrength = max(diffuseStrength, 0.4); \n' + // Apply some ambient lighting @@ -1019,7 +1019,7 @@ define([ vs += ' v_color = color; \n' + ' gl_Position = czm_modelViewProjection * vec4(position, 1.0); \n'; - if (normalsEnabled && backFaceCulling) { + if (hasNormals && backFaceCulling) { vs += ' float visible = step(-normal.z, 0.0); \n' + ' gl_Position *= visible; \n'; } From b6355d9446efcd3b01e82f257f58555e94ce076a Mon Sep 17 00:00:00 2001 From: Srinivas Kaza Date: Fri, 9 Jun 2017 09:51:26 -0400 Subject: [PATCH 4/5] normalShading should hopefully not break backFaceCulling now --- Source/Scene/PointCloud3DTileContent.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Source/Scene/PointCloud3DTileContent.js b/Source/Scene/PointCloud3DTileContent.js index 78a43917bffe..dfe7e95c0922 100644 --- a/Source/Scene/PointCloud3DTileContent.js +++ b/Source/Scene/PointCloud3DTileContent.js @@ -788,7 +788,7 @@ define([ var normalShading = content._normalShading; var vertexArray = content._drawCommand.vertexArray; - var normalsEnabled = hasNormals && normalShading; + var normalsEnabled = (hasNormals && normalShading) || backFaceCulling; var colorStyleFunction; var showStyleFunction; @@ -1009,7 +1009,7 @@ define([ vs += ' color = color * u_highlightColor; \n'; - if (hasNormals) { + if (normalsEnabled) { vs += ' normal = czm_normal * normal; \n' + ' float diffuseStrength = czm_getLambertDiffuse(czm_sunDirectionEC, normal); \n' + ' diffuseStrength = max(diffuseStrength, 0.4); \n' + // Apply some ambient lighting @@ -1019,7 +1019,7 @@ define([ vs += ' v_color = color; \n' + ' gl_Position = czm_modelViewProjection * vec4(position, 1.0); \n'; - if (hasNormals && backFaceCulling) { + if (normalsEnabled) { vs += ' float visible = step(-normal.z, 0.0); \n' + ' gl_Position *= visible; \n'; } From e6f6de17de43c69ef95713569cd65a2e0df67afb Mon Sep 17 00:00:00 2001 From: Sean Lilley Date: Fri, 27 Oct 2017 14:37:45 -0400 Subject: [PATCH 5/5] Adjustments --- Source/Scene/PointCloud3DTileContent.js | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/Source/Scene/PointCloud3DTileContent.js b/Source/Scene/PointCloud3DTileContent.js index dfe7e95c0922..c982127b164a 100644 --- a/Source/Scene/PointCloud3DTileContent.js +++ b/Source/Scene/PointCloud3DTileContent.js @@ -101,7 +101,7 @@ define([ this.backFaceCulling = false; this._backFaceCulling = false; - // Whether or not the user enabled normal shading + // Whether to enable normal shading this.normalShading = true; this._normalShading = true; @@ -788,8 +788,6 @@ define([ var normalShading = content._normalShading; var vertexArray = content._drawCommand.vertexArray; - var normalsEnabled = (hasNormals && normalShading) || backFaceCulling; - var colorStyleFunction; var showStyleFunction; var pointSizeStyleFunction; @@ -841,7 +839,7 @@ define([ var userProperties = styleableProperties.filter(function(property) { return defaultProperties.indexOf(property) === -1; }); //>>includeStart('debug', pragmas.debug); - if (usesNormalSemantic && !normalsEnabled) { + if (usesNormalSemantic && !hasNormals) { throw new DeveloperError('Style references the NORMAL semantic but the point cloud does not have normals'); } //>>includeEnd('debug'); @@ -864,13 +862,20 @@ define([ colorVertexAttribute.enabled = usesColors; } + var usesNormals = hasNormals && (normalShading || backFaceCulling || usesNormalSemantic); + if (hasNormals) { + // Disable the normal vertex attribute if normals are not used + var normalVertexAttribute = getVertexAttribute(vertexArray, normalLocation); + normalVertexAttribute.enabled = usesNormals; + } + var attributeLocations = { a_position : positionLocation }; if (usesColors) { attributeLocations.a_color = colorLocation; } - if (normalsEnabled) { + if (usesNormals) { attributeLocations.a_normal = normalLocation; } if (hasBatchIds) { @@ -926,7 +931,7 @@ define([ vs += 'attribute vec3 a_color; \n'; } } - if (normalsEnabled) { + if (usesNormals) { if (isOctEncoded16P) { vs += 'attribute vec2 a_normal; \n'; } else { @@ -983,7 +988,7 @@ define([ } vs += ' vec3 position_absolute = vec3(czm_model * vec4(position, 1.0)); \n'; - if (normalsEnabled) { + if (usesNormals) { if (isOctEncoded16P) { vs += ' vec3 normal = czm_octDecode(a_normal); \n'; } else { @@ -1009,7 +1014,7 @@ define([ vs += ' color = color * u_highlightColor; \n'; - if (normalsEnabled) { + if (usesNormals && normalShading) { vs += ' normal = czm_normal * normal; \n' + ' float diffuseStrength = czm_getLambertDiffuse(czm_sunDirectionEC, normal); \n' + ' diffuseStrength = max(diffuseStrength, 0.4); \n' + // Apply some ambient lighting @@ -1019,7 +1024,7 @@ define([ vs += ' v_color = color; \n' + ' gl_Position = czm_modelViewProjection * vec4(position, 1.0); \n'; - if (normalsEnabled) { + if (usesNormals && backFaceCulling) { vs += ' float visible = step(-normal.z, 0.0); \n' + ' gl_Position *= visible; \n'; }