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

removed unneccisary subtraction, combined high and low parameters #3104

Merged
merged 4 commits into from
Oct 16, 2015
Merged

removed unneccisary subtraction, combined high and low parameters #3104

merged 4 commits into from
Oct 16, 2015

Conversation

ggetz
Copy link
Contributor

@ggetz ggetz commented Oct 16, 2015

Closes #965.

Removed simulated double precision subtraction from latitudeToWebMercatorFraction.glsl and combined high and low parameters into one southMercatorY. Changed occurrence of the function call to match the new parameters.

@@ -38,13 +38,12 @@ float get2DMercatorYPositionFraction()
float northLatitude = u_southAndNorthLatitude.y;
if (northLatitude - southLatitude > maxTileWidth)
{
float southMercatorYLow = u_southMercatorYLowAndHighAndOneOverHeight.x;
float southMercatorYHigh = u_southMercatorYLowAndHighAndOneOverHeight.y;
float southMercatorY = u_southMercatorYLowAndHighAndOneOverHeight.y;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

u_southMercatorYLowAndHighAndOneOverHeight doesn't need to store low and high now. It can just store the double value casted to a float.

So it can be renamed to u_southMercatorYAndOneOverHeight and made a vec2. Also, update the JavaScript code that sets this uniform.

@ggetz
Copy link
Contributor Author

ggetz commented Oct 16, 2015

Made the changes.

var northMercatorY = WebMercatorProjection.geodeticLatitudeToMercatorAngle(northLatitude);

float32ArrayScratch[0] = southMercatorY;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the declaration for this.

@ggetz
Copy link
Contributor Author

ggetz commented Oct 16, 2015

removed the declaration

@ggetz
Copy link
Contributor Author

ggetz commented Oct 16, 2015

branchscreenshot
masterscreenshot

It looks like there is a differnce when you remove the code. Top is my branch, bottom is master.

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 16, 2015

@ggetz can you provide a url to Cesium viewer with the camera position?

@kring can you take a look? We are OK with just leaving the code as you originally had it.

@ggetz
Copy link
Contributor Author

ggetz commented Oct 16, 2015

Viewer (my branch)

@ggetz
Copy link
Contributor Author

ggetz commented Oct 16, 2015

@pjcozzi I rebuilt my branch and and refreshed the page, and now I am seeing the same thing as master

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 16, 2015

Looks good on my machine too!

pjcozzi added a commit that referenced this pull request Oct 16, 2015
removed unneccisary subtraction, combined high and low parameters
@pjcozzi pjcozzi merged commit 429d134 into CesiumGS:master Oct 16, 2015
@pjcozzi pjcozzi deleted the remove-dsfun90-sub branch October 16, 2015 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants