Remove Rectangle.intersectWith and Rectangle.isEmpty

[pjcozzi]

From #2313:

• Rectangle.intersectWith was deprecated in Cesium 1.5. It will be removed in Cesium 1.6. Use Rectangle.intersection, which is the same but returns undefined when two rectangles do not intersect.
• Rectangle.isEmpty was deprecated in Cesium 1.5. It will be removed in Cesium 1.6.

[kaktus40]

Hello,
I saw the modifications made in Rectangle.intersection and I fear those induced some regressions. For example, TilingScheme.tileXYToNativeRectangle can return a Rectangle that is not defined in Cartographic reference (see WebMercatorTilingScheme). The tests made in Rectangle.intersection were only made Cartographic Rectangle and not native Rectangle (see between line 544 and line 557)! Is it possible to add an other parameter (a boolean for example) to indicate that Rectangle.intersection works with cartographic data?

[kaktus40]

Moreover the behaviour of Rectangle.intersection is a little strange when we add the third parameter (result):

• when there is an intersection, the third parameter is filled with the result of the computation
• when there is no intersection, the function returns only undefined and doesn't fill the third parameter with undefined.

I think that in the second case, the third parameter should be undefined.

[bagnell]

@kaktus40 We expect the Rectangles to be in radians. Have you found a bug in master that I can try to reproduce?

[kaktus40]

I don't have a use case from master to reproduce and it's not a really a
bug. It's more an inconsistence with the use of
TilingScheme.tileXYToNativeRectangle that returns a Rectangle which
isn't in radians, it's specific to each TilingScheme. For example
WebMercatorTilingScheme.tileXYToNativeRectangle returns a rectangle in
meters.

I'm working with TerrainProvider that must implements
getTileDataAvailable. In my case, where I don't know which tilingScheme
is used, I can only test the availability with the native rectangle (see
https://github.com/kaktus40/Cesium-GeoserverTerrainProvider/blob/master/GeoserverTerrainProvider.js#L346
or
https://github.com/kaktus40/Cesium-GeoserverTerrainProvider/blob/master/GeoserverTerrainProvider.js#L542
).

My remark is just a suggestion and I can use a workaround (like I said
using an option to indicate if it's a radian rectangle).

My remark about undefined is also an inconsistency problem (my point of
view):

• either result (third parameter) should not be used
• either result should always be used

It's only my point of view and it's not a bug. I know you have others
constraints.

On 09/12/2014 22:47, Dan Bagnell wrote:

@kaktus40 https://github.com/kaktus40 We expect the |Rectangle|s to
be in radians. Have you found a bug in master that I can try to reproduce?

—
Reply to this email directly or view it on GitHub
#2314 (comment).

[bagnell]

@kaktus40 Is it possible to use tileXYToRectangle? If not, you might run into the same problem as the linked issue when a tile crosses the IDL.

For your remark about the result parameter, do you mean that we should set the results properties to undefined? Because it would be impossible to set the parameter to undefined. I know the use of a result parameter is odd, but we use it throughout Cesium to reduce the amount of memory used and garbage collection overhead. In this case its not required, so I expect the most common use to be

var intersection = Cesium.Rectangle.intersection(rectangle1, rectangle2);
if (Cesium.defined(intersection)) {
    // handle intersection
}

The pattern we use throughout Cesium is

var scratchRectangle = new Cesium.Rectangle(); // declared in file scope and used for intermediate values
// ...
var intersection = Cesium.Rectangle.intersection(rectangle1, rectangle2, scratchRectangle);
if (Cesium.defined(intersection)) {
    // handle intersection
}

If the function was a success, then intersection will be a reference to scratchRectangle or undefined otherwise.

This is consistent with other intersection tests as well (see IntersectionTests).