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

An abstract projection class and child mercprojection class #7

Merged
merged 4 commits into from
Sep 19, 2014

Conversation

tallytalwar
Copy link
Member

No description provided.

return latLon;
}

glm::vec2 MercProjection::PixelsToMeters(glm::vec2 _pix, int _zoom) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not clear on the purpose of the "pixel" functions, the results of any pixel measurements here aren't meaningful without knowing the display size (which only the view module would have, at the moment). Even then, "pixels" are a problematic unit of measurement on mobile devices where native pixel doubling or tripling is common. Measuring tiles in pixels is a common practice on legacy raster tile maps, but I don't know that it has a use for us.

Copy link
Member

Choose a reason for hiding this comment

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

Generally agree, but there are still cases where you want to refer to & project or un-project a particular point in screenspace. On the WebGL side, I track "CSS pixels" (which are "logical" pixels), and "device pixels" (which are the 2x or 3x ones). We don't need to do that here but we do need a way to transform the position of an individual pixel.

@@ -0,0 +1,124 @@
#pragma one
Copy link
Member

Choose a reason for hiding this comment

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

Typo: should be "once"

Copy link
Member Author

Choose a reason for hiding this comment

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

Ahh ... :|

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.

3 participants