Polyline Gaps

[bagnell]

For #2136.

CC @shunter @mramato

[pjcozzi]

Code looks good (I didn't run it though). Does this warrant a mention in CHANGES.md?

[bagnell]

@pjcozzi CHANGES.md has been updated.

[pjcozzi]

@shunter @mramato merge when you are happy.

[mramato]

@shunter since you have the complete CZML polyline that originally detected this problem, can you merge this after verifying everything works?

[shunter]

Still getting gaps. Sandcastle:

var viewer = new Cesium.Viewer('cesiumContainer');
var scene = viewer.scene;

var czml =
[
  {
    "id":"document",
    "version":"1.0"
  },
  {
    "id":"route",
    "polyline":{
      "followSurface":false,
      "width":1,
      "material":{
        "solidColor":{
          "color":{
            "rgba":[
              255,0,0,255
            ]
          }
        }
      },
      "positions":{
        "cartesian":[
-3591987.63453649,5161973.3486444,1190251.05756476,
-3628083.55387411,5165232.94262105,1060369.2289924,
-3663885.23771364,5166176.8496425,924863.604149131,
-3699175.1634344,5164520.94347153,783745.252709972,
-3699175.1634336,5164520.94347154,783745.252713645,
-3699502.86098311,5164543.69243247,782058.090405673,
-3699671.57770435,5164682.19775128,780354.861274571,
-3699679.5349048,5164934.3585661,778657.738656253,
-3699527.00703476,5165296.63612803,776988.727947579,
-3699216.30934082,5165764.10253274,775369.403766365,
-3698751.76552228,5166330.50307426,773820.651264288,
-3698139.65485485,5166988.33245244,772362.412582204
        ]
      }
    }
  }
];

var czmlDataSource = new Cesium.CzmlDataSource();
czmlDataSource.load(czml, 'Built-in CZML');
viewer.dataSources.add(czmlDataSource);

scene.camera.viewRectangle(Cesium.Rectangle.fromDegrees(121, 5, 129, 11));

[bagnell]

@shunter I updated the epsilon value. The CZML file you sent me now works. Though any line segment less than a millimeter in length will fail to draw.

[pjcozzi]

@shunter did you want to retest or we can merge?

[shunter]

Seems like the epsilon value is still not high enough to handle all cases.

I changed the positions from my last sandcastle example and can still produce gaps:

-3591987.63453649,5161973.3486444,1190251.05756476,
-3628083.55387411,5165232.94262105,1060369.2289924,
-3663885.23771364,5166176.8496425,924863.604149131,
-3699175.1634344,5164520.94347153,783745.252709972,
-3699175.2,5164520.94347154,783745.252713645,
-3699502.86098311,5164543.69243247,782058.090405673,
-3699671.57770435,5164682.19775128,780354.861274571,
-3699679.5349048,5164934.3585661,778657.738656253,
-3699527.00703476,5165296.63612803,776988.727947579,
-3699216.30934082,5165764.10253274,775369.403766365,
-3698751.76552228,5166330.50307426,773820.651264288,
-3698139.65485485,5166988.33245244,772362.412582204

(the two closest points have a distance of about 0.037 meters)

In my testing, this seems to be approximately the farthest apart where the segment disappears. Given the Earth's radius, that seems to more or less line up with the limitation being a result of the values being stored in single precision.

[shunter]

I guess I should point out that the error will vary depending on the magnitude of the components, which means that equalsEpsilon with an absolute epsilon won't really work in all cases. Perhaps equalsEpsilon should scale the two input values to [0, 1] before comparing the deltas.

[GatorScott]

If the goal is to remove duplicates, shouldn't epsilon15 (possibly 16) be used since that is the resolution of double precision, 64-bit floating point numbers?

[shunter]

Well, the values end up as 32-bit floats in the shader, so that's the precision that we work with. I sent Dan a link to this post which goes into the details of why the absolute epsilon approach does not work for detecting precision problems, and what to do instead: http://randomascii.wordpress.com/2012/02/25/comparing-floating-point-numbers-2012-edition/

[pjcozzi]

Also check out http://realtimecollisiondetection.net/blog/?p=89

[bagnell]

@shunter I added functions for testing relative tolerance. If these look OK to you, I'll take a look at all of the existing equalsEpsilon calls in another PR.

[shunter]

Is this logic right? Shouldn't it allow left.x to be near right.x in absolute tolerance, and allow left.y to be near right.y in relative tolerance?

Basically,

((Math.abs(left.x - right.x) <= absoluteEpsilon) ||
 (Math.abs(left.x - right.x) <= relativeEpsilon * Math.max(Math.abs(left.x), Math.abs(right.x)))) &&
((Math.abs(left.y - right.y) <= absoluteEpsilon) ||
 (Math.abs(left.y - right.y) <= relativeEpsilon * Math.max(Math.abs(left.y), Math.abs(right.y))))

Or alternatively this can be simplified using the logic from http://realtimecollisiondetection.net/blog/?p=89

(Math.abs(left.x - right.x) <= Math.max(absoluteEpsilon, relativeEpsilon * Math.max(Math.abs(left.x), Math.abs(right.x)))) &&
(Math.abs(left.y - right.y) <= Math.max(absoluteEpsilon, relativeEpsilon * Math.max(Math.abs(left.y), Math.abs(right.y))))

[shunter]

Also, you can just use the function in CesiumMath.

CesiumMath.equalsEpsilonRelativeAndAbsolute(left.x, right.x, absoluteEpsilon, relativeEpsilon) && 
CesiumMath.equalsEpsilonRelativeAndAbsolute(left.y, right.y, absoluteEpsilon, relativeEpsilon)

[bagnell]

@shunter This is ready for another look.

[pjcozzi]

Do we want this for 1.4?

[mramato]

I moved the changes to 1.5 and merged in master. @shunter do you have time to take a final look at this? If not, I can do it.

[bagnell]

@shunter I made the changes that we discussed offline. This is ready for another review.

[shunter]

These functions can just call CesiumMath.equalsEpsilon(left.x, right.x, relativeEpsilon, absoluteEpsilon) can't they? Then you can drop the defaultValue above. Same throughout.

[bagnell]

@shunter This is ready for another look. Implementing Cartesian*.equalsEpsilon with CesiumMath.equalsEpsilon uncovered a bug in RectangleGeometry where arrays were being indexed past they're length.

[shunter]

typo here: EPAILON7. I'll fix and merge.