Billboard depth fail #5166

bagnell · 2017-03-28T23:40:05Z

Adds disableDepthDistance and alwaysDisableDepth properties to labels/billboards/points. alwaysDisableDepth will always draw the billboard in front of everything. disableDepthDistance will always draw the billboard when the distance to the camera is less than that disableDepthDistance.

Fixes #5121.

Add doc
Add tests
Update CHANGES.md

…rds/labels/points.

pjcozzi · 2017-03-29T00:01:24Z

Fixes #5121

pjcozzi · 2017-03-29T00:02:12Z

Also fixes #2694

pjcozzi · 2017-03-29T00:09:20Z

For the API and naming, perhaps just:

disableDepthTestDistance with a default of 0.0.

When it's Number.POSITIVE_INFINITY, depth is never tested. When 0.0, it is always tested.

pjcozzi · 2017-03-29T00:11:44Z

Source/Scene/Billboard.js

-    Billboard.NUMBER_OF_PROPERTIES = 15;
+    var DISABLE_DEPTH_DISTANCE = Billboard.DISABLE_DEPTH_DISTANCE = 15;
+    var ALWAYS_DISABLE_DEPTH = Billboard.ALWAYS_DISABLE_DEPTH = 16;
+    Billboard.NUMBER_OF_PROPERTIES = 17;


Do you have ideas for avoiding going above 16 attributes?

Nevermind, I see that we are good.

pjcozzi · 2017-03-29T00:14:21Z

Source/Scene/PointPrimitive.js

@@ -100,7 +102,9 @@ define([
    var PIXEL_SIZE_INDEX = PointPrimitive.PIXEL_SIZE_INDEX = 5;
    var SCALE_BY_DISTANCE_INDEX = PointPrimitive.SCALE_BY_DISTANCE_INDEX = 6;
    var TRANSLUCENCY_BY_DISTANCE_INDEX = PointPrimitive.TRANSLUCENCY_BY_DISTANCE_INDEX = 7;
-    var DISTANCE_DISPLAY_CONDITION_INDEX = PointPrimitive.DISTANCE_DISPLAY_CONDITION = 8;
+    var DISTANCE_DISPLAY_CONDITION_INDEX = PointPrimitive.DISTANCE_DISPLAY_CONDITION_INDEX = 8;
+    var DISABLE_DEPTH_DISTANCE_INDEX = PointPrimitive.DISABLE_DEPTH_DISTANCE_INDEX = 15;


Why 15 and 16? Not 9 and 10?

pjcozzi · 2017-03-29T00:16:36Z

Source/Shaders/BillboardCollectionVS.glsl

+attribute vec4 eyeOffset;                                  // eye offset in meters, 4 bytes free (texture range)
+attribute vec4 scaleByDistance;                            // near, nearScale, far, farScale
+attribute vec4 pixelOffsetScaleByDistance;                 // near, nearScale, far, farScale
+attribute vec4 distanceDisplayConditionAndDisableDepth;    // near, far, disableDepthDistance, alwaysDisableDepth


Could alwaysDisableDepth be the sign of disableDepthDistance?

pjcozzi · 2017-03-29T00:17:41Z

Source/Shaders/BillboardCollectionVS.glsl

@@ -246,8 +246,8 @@ void main()
 #endif

 #ifdef DISTANCE_DISPLAY_CONDITION
-    float nearSq = distanceDisplayCondition.x * distanceDisplayCondition.x;
-    float farSq = distanceDisplayCondition.y * distanceDisplayCondition.y;
+    float nearSq = distanceDisplayConditionAndDisableDepth.x * distanceDisplayConditionAndDisableDepth.x;


Unrelated to this PR, but why don't we store the squared distance in the vertex attribute to avoid this computation?

Not sure, updated here: 18b6769

denverpierce · 2017-03-29T15:00:13Z

Is it possible to get this somewhere higher in the API, like dataSources or viewer/scene? Label clipping issues especially have been a pain point, and I don't see an issue overriding labels especially to never depth test.

…depth test. If it is positive infinity, we never depth test.

ghost · 2017-03-30T11:20:37Z

Seconded @denverpierce's comment. We load pins from KML files with label clipping.

pjcozzi · 2017-03-30T18:23:48Z

@bagnell what do you think of a min distance on Scene that could set a czm_ uniform for @denverpierce and @Zizekftw's request?

bagnell · 2017-03-31T18:11:42Z

@bagnell what do you think of a min distance on Scene that could set a czm_ uniform for @denverpierce and @Zizekftw's request?

Added in e990328

bagnell · 2017-03-31T21:52:46Z

@pjcozzi This is ready for another look.

pjcozzi · 2017-04-03T17:32:47Z

Source/Shaders/BillboardCollectionVS.glsl

@@ -258,6 +258,24 @@ void main()
    gl_Position = czm_viewportOrthographic * vec4(positionWC.xy, -positionWC.z, 1.0);
    v_textureCoordinates = textureCoordinates;

+#ifdef DISABLE_DEPTH_DISTANCE
+    float disableDepthTestDistance = distanceDisplayConditionAndDisableDepth.z;
+    if (disableDepthTestDistance == 0.0 && czm_minimumDisableDepthTestDistance != 0.0) {


Put { on a new line in GLSL.

pjcozzi · 2017-04-03T17:34:53Z

Source/Shaders/BillboardCollectionVS.glsl

+
+        bool clipped = gl_Position.z < -gl_Position.w || gl_Position.z > gl_Position.w;
+        float distance = length(positionEC.xyz);
+        if (!clipped && (disableDepthTestDistance < 0.0 || (distance > 0.0 && distance < disableDepthTestDistance)))


I might have already asked this, but should be store squared distance instead to avoid the length above? Or are you concerned about overflow? We would treat infinity as a special case.

You asked about it for the distance display conditions, which I fixed, but somehow forgot this. It's been updated.

pjcozzi · 2017-04-03T17:35:59Z

Add this to a Sandcastle example.

pjcozzi · 2017-04-03T17:36:15Z

@bagnell feel free to push this 1.33, up to you.

pjcozzi · 2017-04-03T18:02:20Z

@tfili @mramato does it make sense to use this by default for KML when ground clamping is used?

denverpierce · 2017-04-03T20:02:15Z

Google Earth doesn't depth test labels, but it does test icons.

bagnell · 2017-04-03T23:34:21Z

@pjcozzi This is ready for another look.

pjcozzi · 2017-04-06T12:41:19Z

All looks good.

Please also merge into 3d-tiles.

bagnell added 3 commits March 28, 2017 16:32

Add disableDepthDistance and alwaysDisableDepth properties to billboa…

e4fee87

…rds/labels/points.

Minor fix for labels.

a8f0788

Fix far plane clipping.

6ad2b13

pjcozzi reviewed Mar 29, 2017

View reviewed changes

bagnell added 2 commits March 29, 2017 13:53

Remove alwaysDisableDepth. If disableDepthDistance is 0.0, we always …

914826d

…depth test. If it is positive infinity, we never depth test.

Rename disableDepthDistance -> disableDepthTestDistance.

a8575f6

bagnell added 2 commits March 31, 2017 14:08

Add scene.minimumDisableDepthTestDistance.

e990328

Merge branch 'master' into billboard-depth-fail

91e4967

bagnell added 4 commits March 31, 2017 14:31

Add doc.

bdfbf42

Store squared distance display condition.

18b6769

Add tests.

3daf653

Update CHANGES.md.

5069273

Tweak CHANGES.md

9f84f71

pjcozzi reviewed Apr 3, 2017

View reviewed changes

bagnell added 4 commits April 3, 2017 19:19

Store the squared distance and fix whitespace.

faed91c

Update billboard Sandcastle example.

9c139ab

Merge branch 'master' into billboard-depth-fail

00e6861

Update CHANGES.md.

9b0a846

pjcozzi merged commit 7cf1df8 into master Apr 6, 2017

pjcozzi deleted the billboard-depth-fail branch April 6, 2017 12:41

lilleyse mentioned this pull request Apr 7, 2017

Per tile stats #5188

Merged

SunBlack mentioned this pull request Jul 12, 2017

Useless/Wrong GLSL code in depth test #5621

Closed

shunter mentioned this pull request Nov 17, 2017

Add missing entity properties to CZML. #5986

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Billboard depth fail #5166

Billboard depth fail #5166

bagnell commented Mar 28, 2017 •

edited

Loading

pjcozzi commented Mar 29, 2017

pjcozzi commented Mar 29, 2017

pjcozzi commented Mar 29, 2017

pjcozzi Mar 29, 2017

pjcozzi Mar 29, 2017

pjcozzi Mar 29, 2017

pjcozzi Mar 29, 2017

pjcozzi Mar 29, 2017

bagnell Mar 31, 2017

denverpierce commented Mar 29, 2017

ghost commented Mar 30, 2017

pjcozzi commented Mar 30, 2017

bagnell commented Mar 31, 2017

bagnell commented Mar 31, 2017

pjcozzi Apr 3, 2017

pjcozzi Apr 3, 2017

bagnell Apr 3, 2017

pjcozzi commented Apr 3, 2017

pjcozzi commented Apr 3, 2017

pjcozzi commented Apr 3, 2017

denverpierce commented Apr 3, 2017

bagnell commented Apr 3, 2017

pjcozzi commented Apr 6, 2017

Billboard depth fail #5166

Billboard depth fail #5166

Conversation

bagnell commented Mar 28, 2017 • edited Loading

pjcozzi commented Mar 29, 2017

pjcozzi commented Mar 29, 2017

pjcozzi commented Mar 29, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

denverpierce commented Mar 29, 2017

ghost commented Mar 30, 2017

pjcozzi commented Mar 30, 2017

bagnell commented Mar 31, 2017

bagnell commented Mar 31, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pjcozzi commented Apr 3, 2017

pjcozzi commented Apr 3, 2017

pjcozzi commented Apr 3, 2017

denverpierce commented Apr 3, 2017

bagnell commented Apr 3, 2017

pjcozzi commented Apr 6, 2017

bagnell commented Mar 28, 2017 •

edited

Loading