Distance display conditions #4309

bagnell · 2016-09-09T19:11:41Z

Adds distance display conditions to primitives.

For billboards, points and models, there is a new distanceDisplayCondition property. For Primitives, there is a new distanceDisplayCondition per-instance geometry attribute.

TODO:

…anceAttribute tests.

mramato · 2016-09-09T20:34:22Z

I haven't really looked at this yet, but to go along with translucencyByDistance and scaleByDistance should this be called showByDistance?

pjcozzi · 2016-09-09T20:51:57Z

JSHint warnings: https://travis-ci.org/AnalyticalGraphicsInc/cesium/builds/158843026

Specs/Core/DistanceDisplayConditionSpec.js
line 2 col 1 'defineSuite' is not defined.
line 1 col 1 'definedSuite' is defined but never used.
⚠ 2 warnings

pjcozzi · 2016-09-09T21:01:09Z

Source/Scene/Model.js

@@ -3719,7 +3721,8 @@ define([
            }
        }

-        var show = this.show && (this.scale !== 0.0);
+        var displayConditionPassed = defined(this.distanceDisplayCondition) ? this.distanceDisplayCondition.isVisible(this, frameState) : true;
+        var show = this.show && (this.scale !== 0.0) && displayConditionPassed;


Perhaps slightly faster in the reject case (and the same speed in the accept) to do

this.show && displayConditionPassed && (this.scale !== 0.0)

pjcozzi · 2016-09-09T21:05:55Z

I haven't really looked at this yet, but to go along with translucencyByDistance and scaleByDistance should this be called showByDistance?

We could if we were sure we'll never add another display condition like pixel size, viewer altitude, etc. since those would want to be named showByPixelSize, etc. In practice, distance should get us 95% of the cases so up to @bagnell.

pjcozzi · 2016-09-09T21:06:31Z

Just those comments. This is looking good despite all the vertex data required.

pjcozzi · 2016-09-09T21:08:36Z

Source/Scene/Primitive.js

+            'void main() \n' +
+            '{ \n' +
+            '    czm_non_distanceDisplayCondition_main(); \n' +
+            '    vec4 centerRTE = czm_computeBoundingSphereCenter(); \n' +


Where is this function? Did you forget to submit a file?

Nope, its generated just like czm_computePosition for all other geometries and the functions for getting adjacent points for polylines.

I don't see it in a repo search or this diff.

See this function: https://github.com/AnalyticalGraphicsInc/cesium/blob/9efeb5a4d1ef96fbdbb9c42477db6d1da79e131b/Source/Scene/Primitive.js#L594

Ah, yes, thanks!

pjcozzi · 2016-09-09T21:11:05Z

Part of #758. Perhaps even close that and let the KML region support stay as part of the KML roadmap.

…s, and labels clone the set display condition.

…rds and points.

…and add tests.

…olylines and paths.

bagnell · 2016-09-19T19:58:40Z

This requires the batch table from #4328, so please review and merge that PR before this one.

bagnell · 2016-09-27T00:00:58Z

@pjcozzi The reference doc is ready.

pjcozzi · 2016-09-27T14:45:28Z

Are you sure that we don't want distanceDisplayCondition to be part of the Model constructor (and billboard/label add just like scaleByDistance)?

pjcozzi · 2016-09-27T14:46:51Z

Just those comments.

bagnell · 2016-09-27T17:30:06Z

Are you sure that we don't want distanceDisplayCondition to be part of the Model constructor (and billboard/label add just like scaleByDistance)?

It is part of the constructors. The doc was missing for Model, but there is no doc for billboard/label options either on the constructor or on add.

pjcozzi · 2016-09-27T19:28:59Z

All good with me. @mramato please merge when ready. The changes to the entity layer are minor.

mramato · 2016-09-28T13:52:50Z

Source/Core/DistanceDisplayCondition.js

+            },
+            set : function(value) {
+                this._near = value;
+                this._near2 = value * value;


Nitpicky, but as a generate practice, we would normally call this nearSquared.

mramato · 2016-09-28T13:55:38Z

Source/Core/DistanceDisplayCondition.js

+     * @param {DistanceDisplayCondition} [result] The result onto which to store the result.
+     * @return {DistanceDisplayCondition} The duplicated instance.
+     */
+    DistanceDisplayCondition.clone = function(value, result) {


Since this is used directly by the Entity api as well, it should have a prototype clone too. Right now the Entity API ends up making a new instance every time get value is called because no prototype clone is defined.

mramato · 2016-09-28T13:57:34Z

Point & Model does not seem to work at all in 2D or Columbus View.

mramato · 2016-09-28T13:59:05Z

Actually, I wasn't zoomed in enough. It appears to partially work but the model is never drawn.

mramato · 2016-09-28T14:00:28Z

This may be in master too, so create an issue if it's not related to this branch.

Run the Sandcastle example.
Switch to 2D
Double click the point
Switch to 3D. (The morph is all screwy)

mramato · 2016-09-28T14:42:13Z

Source/DataSources/EllipsoidGeometryUpdater.js

@@ -649,7 +679,8 @@ define([
                    modelMatrix : !in3D ? modelMatrix : undefined,
                    attributes : {
                        show : new ShowGeometryInstanceAttribute(showOutline),
-                        color : ColorGeometryInstanceAttribute.fromColor(outlineColor)
+                        color : ColorGeometryInstanceAttribute.fromColor(outlineColor),
+                        distanceDisplayCondition : distanceDisplayConditionAttribute


This needs to be handled in the else path as well, just like show and color.

mramato · 2016-09-28T14:48:04Z

It doesn't look like any of the DynamicGeometryUpdater objects have been updated to set the displayCondition property on the underlying primitives. These are private classes that are part of each GeometryUpdater, just search for DynamicGeometryUpdater and you'll find them.

For the above, a corresponding test needs to be added to the dynamic updater sets properties test of each updated (again, searching for that phrase should reveal them all).

Basically, it looks like dynamic geometry never gets its display condition updated.

mramato · 2016-09-28T14:48:24Z

That's all I have for now. Thanks @bagnell

bagnell · 2016-09-28T18:07:08Z

This may be in master too, so create an issue if it's not related to this branch.

Run the Sandcastle example.

Switch to 2D

Double click the point

Switch to 3D. (The morph is all screwy)

Models in 2D crash in master so I'll open an issue for it. In this branch the same weird morph happens for the model example.

…ic geometry.

bagnell · 2016-09-28T18:32:45Z

Basically, it looks like dynamic geometry never gets its display condition updated.

All the dynamic geometries do update the distance display condition. It was only missing from that else block you mentioned above.

For the above, a corresponding test needs to be added to the dynamic updater sets properties test of each updated (again, searching for that phrase should reveal them all).

I didn't modify the test because it doesn't change any per-instance attributes besides show. Then, it makes sure that a primitive is created when true.

@mramato This is ready for another look. I'll add an issue for the model crash in 2D.

mramato · 2016-09-29T18:31:16Z

Scene/GroundPrimitive renders with distance display condition per instance attribute is failing on my machine.

bagnell · 2016-09-29T18:45:35Z

@mramato The test is fixed. It wasn't updated with the other tests that were failing in master.

mramato · 2016-09-29T18:52:42Z

👍

bagnell added 7 commits September 8, 2016 15:54

Add distance display conditions to geometry primitives.

960234f

Add distance display conditions to billboards.

28bf84c

Add distance display conditions to labels.

396548a

Add distance display condition to point primitives.

f19aa4d

Add development Sandcastle example.

108cdcf

Merge branch 'master' into distance-display-condition

d27b1ad

Add DistanceDisplayCondition and DistanceDisplayConditionGeometryInst…

b7a3997

…anceAttribute tests.

pjcozzi reviewed Sep 9, 2016
View reviewed changes

bagnell added 9 commits September 9, 2016 17:13

Add distance display condition tests for billboards, points and labels.

9efeb5a

Add model distance display condition tests.

00a2e10

Fix distance display conditions for primitives in 2D.

366522b

Add tests for primitives with distance display conditions.

91d56c1

Add DistanceDisplayCondition.clone and have models, billboards, point…

0035a66

…s, and labels clone the set display condition.

Add entity support for distance display conditions to labels, billboa…

e1ee9fe

…rds and points.

Modify GroundPrimitive shader to support distance display conditions …

9ed2ab0

…and add tests.

Add distance display condition support to all entity geometries but p…

5a8068c

…olylines and paths.

Fix jsHint errors.

090f57e

bagnell mentioned this pull request Sep 16, 2016

Batch table #4328

Merged

bagnell added 2 commits September 19, 2016 15:56

Merge branch 'master' into distance-display-condition

22c156f

Merge branch 'batch-table' into distance-display-condition

d7c442f

bagnell added 2 commits September 20, 2016 14:00

Add distance display conditions to polylines.

93d8d4b

Merge branch 'batch-table' into distance-display-condition

b768d5a

bagnell added 3 commits September 27, 2016 13:21

Merge branch 'master' into distance-display-condition

0d79c09

Update CHANGES.md.

ac57dbd

Add missing distance display condition option doc to model constructor.

34a2d6c

mramato reviewed Sep 28, 2016

View reviewed changes

Fix model distance display conditions in 2D/CV.

a72dfd1

Update ellipsoid distance display condition when it changes for dynam…

380bbf6

…ic geometry.

bagnell mentioned this pull request Sep 29, 2016

Morphing from 2D while tracking a model entity is broken #4383

Closed

Merge branch 'master' into distance-display-condition

1ea0f48

Update GroundPrimitive distance display condition test.

6dedebe

mramato merged commit f1a5ff4 into master Sep 29, 2016

mramato deleted the distance-display-condition branch September 29, 2016 18:52

shunter mentioned this pull request Apr 18, 2017

Add display conditions AnalyticalGraphicsInc/czml-writer#131

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

Distance display conditions #4309

Distance display conditions #4309

bagnell commented Sep 9, 2016 •

edited

Loading

mramato commented Sep 9, 2016

pjcozzi commented Sep 9, 2016

pjcozzi Sep 9, 2016

pjcozzi commented Sep 9, 2016

pjcozzi commented Sep 9, 2016

pjcozzi Sep 9, 2016

bagnell Sep 9, 2016

pjcozzi Sep 9, 2016

bagnell Sep 9, 2016

pjcozzi Sep 9, 2016

pjcozzi commented Sep 9, 2016

bagnell commented Sep 19, 2016

bagnell commented Sep 27, 2016

pjcozzi commented Sep 27, 2016

pjcozzi commented Sep 27, 2016

bagnell commented Sep 27, 2016

pjcozzi commented Sep 27, 2016

mramato Sep 28, 2016

mramato Sep 28, 2016

mramato commented Sep 28, 2016

mramato commented Sep 28, 2016

mramato commented Sep 28, 2016

mramato Sep 28, 2016

mramato commented Sep 28, 2016

mramato commented Sep 28, 2016

bagnell commented Sep 28, 2016

bagnell commented Sep 28, 2016

mramato commented Sep 29, 2016

bagnell commented Sep 29, 2016

mramato commented Sep 29, 2016

Distance display conditions #4309

Distance display conditions #4309

Conversation

bagnell commented Sep 9, 2016 • edited Loading

mramato commented Sep 9, 2016

pjcozzi commented Sep 9, 2016

Choose a reason for hiding this comment

pjcozzi commented Sep 9, 2016

pjcozzi commented Sep 9, 2016

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

pjcozzi commented Sep 9, 2016

bagnell commented Sep 19, 2016

bagnell commented Sep 27, 2016

pjcozzi commented Sep 27, 2016

pjcozzi commented Sep 27, 2016

bagnell commented Sep 27, 2016

pjcozzi commented Sep 27, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mramato commented Sep 28, 2016

mramato commented Sep 28, 2016

mramato commented Sep 28, 2016

Choose a reason for hiding this comment

mramato commented Sep 28, 2016

mramato commented Sep 28, 2016

bagnell commented Sep 28, 2016

bagnell commented Sep 28, 2016

mramato commented Sep 29, 2016

bagnell commented Sep 29, 2016

mramato commented Sep 29, 2016

bagnell commented Sep 9, 2016 •

edited

Loading