Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rotate back across the globe when points cross the IDL #4507

Merged
merged 3 commits into from Oct 24, 2016

Conversation

Projects
None yet
3 participants
@lasalvavida
Copy link
Contributor

commented Oct 21, 2016

Fix for #3874.

@lasalvavida

This comment has been minimized.

Copy link
Contributor Author

commented Oct 21, 2016

Corrected output:

capture

@hpinkos

This comment has been minimized.

Copy link
Contributor

commented Oct 21, 2016

I don't think that's quite right @lasalvavida. One of the blue rectangles should be rotated. The result should look like the yellow rectangles.

@hpinkos

This comment has been minimized.

Copy link
Contributor

commented Oct 21, 2016

Can you add a unit test for this?

@lasalvavida

This comment has been minimized.

Copy link
Contributor Author

commented Oct 21, 2016

As discussed offline, this is the expected output:

var western2 = createWesternRectangle(0.005);

The blue rectangle is only rotated a small amount.

@hpinkos hpinkos added the bug bash label Oct 21, 2016

@lasalvavida

This comment has been minimized.

Copy link
Contributor Author

commented Oct 21, 2016

Adding a test now. Just to be clear, the crux of this issue is that the rectangle has its northwest corner on the opposite of the IDL from its center. So, when we do the following:

 nwCartesian = Cartesian3.subtract(nwCartesian, centerCartesian, nwCartesian);

Instead of getting the very small difference between them, we subtract a negative value, producing a large offset which blows up small rotations to the point where it causes a translation.

@@ -118,6 +118,10 @@ define([
granYSin = granularityY * sinRotation;
granXSin = granularityX * sinRotation;

if (center.longitude < nwCorner.longitude) {
center.longitude += 2 * Math.PI;

This comment has been minimized.

Copy link
@hpinkos

hpinkos Oct 21, 2016

Contributor

do CesiumMath.TWO_PI

@hpinkos

This comment has been minimized.

Copy link
Contributor

commented Oct 21, 2016

Looks good to me @lasalvavida! I just had that one comment.

@lasalvavida

This comment has been minimized.

Copy link
Contributor Author

commented Oct 21, 2016

Test added!

@hpinkos

This comment has been minimized.

Copy link
Contributor

commented Oct 24, 2016

Great, thanks @lasalvavida!

@hpinkos hpinkos merged commit 8ba579b into AnalyticalGraphicsInc:master Oct 24, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@pjcozzi

This comment has been minimized.

Copy link
Member

commented Oct 25, 2016

@hpinkos please update CHANGES.md in master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.