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

basic perspective rendering #1049

Merged
merged 21 commits into from
Mar 6, 2015
Merged

basic perspective rendering #1049

merged 21 commits into from
Mar 6, 2015

Conversation

ansis
Copy link
Contributor

@ansis ansis commented Mar 4, 2015

This merges current work on perspective rendering so that it doesn't get more stale.

Two transform properties are added:

pitch

The angle at which the camera is looking at the ground. 0 is straight down, 90 would be horizontal. The bigger the pitch, the more vertically squashed everything gets.

examples (with altitude 1.5):
pitch = 0
pitch = 30
pitch = 60

altitude

How high above the surface the camera is. The altitude units are "canvas heights". So an altitude of 3 should be 3x the vertical distance shown on the canvas. Decreasing the altitude makes everything more perspectivy.

examples (with pitch 55):
altitude = 0.75
altitude = 1.5
altitude = 100

Apple uses the names pitch and altitude in their sdk. Google calls pitch viewing angle and doesn't expose altitude.

A pitch argument was added to setView() and easeTo. setPitch() and getPitch() also added.

The implementation has three parts:

  • the api for setting these properties
  • calculating the projection matrix (and converting points back to coordinates)
  • adapting antialiasing so that it works for everything. This is the hard part.

cc @jfirebaugh

Todo:

  • Fix or update render tests. Most render tests are failing because everything is rendered a tiny bit more zoomed in. I still need to figure out if this wrong, and if it is how/whether it can be fixed.

ansis added 19 commits January 5, 2015 20:48
Conflicts:
	js/geo/transform.js
	js/render/draw_line.js
	js/render/draw_symbol.js
	js/render/painter.js
	js/style/style.js
	js/ui/easings.js
	shaders/line.fragment.glsl
	shaders/line.vertex.glsl
Since projection is more complex with support for perspective views,
pointCoordinate is less precise. The tests were failing because it
expected exactly `row: 1` and got `row: 1.0000000000000069`. This loaded
an extra tile in the tests, which is acceptable for cases the tile edge
is right at the edge of the screen.
transform.width = 512;
transform.height = 512;
transform.width = 511;
transform.height = 511;
Copy link
Member

Choose a reason for hiding this comment

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

what's up with 1px here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See commit message here e5ee155 The tests were too sensitive.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mourner:

Hmm, I'd rather fix the tests to check row equality within a margin when perspective is set.

The tests are affected by which tiles are in the view's covering tiles. I didn't see an easy way to allow a certain margin of error in the output, so I just changed the size of the test view so that it wouldn't test a case where multiple possible results would be ok.

@mourner
Copy link
Member

mourner commented Mar 4, 2015

This is so awesome, can't wait for this to get merged!

@karenzshea
Copy link
Contributor

sweeeeeet

@@ -194,7 +194,7 @@ util.extend(exports, {
this.flyTo(center, zoom, 0, options);
},

easeTo: function(latlng, zoom, bearing, options) {
easeTo: function(latlng, zoom, bearing, pitch, options) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we change this to accept a single options argument?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think so. Especially if we want to expose altitude too.

Or maybe separating the view from the ease options make sense: easeTo(view, options)?

@jfirebaugh
Copy link
Contributor

Is this ready to merge?

ansis added a commit that referenced this pull request Mar 6, 2015
basic perspective rendering
@ansis ansis merged commit bdaa1d0 into master Mar 6, 2015
@ansis ansis mentioned this pull request Mar 6, 2015
@ansis ansis deleted the perspective branch March 6, 2015 18:59
@bsudekum
Copy link

bsudekum commented Mar 9, 2015

@ansis Could we get an example and/or an update to api.md?

@ansis
Copy link
Contributor Author

ansis commented Mar 18, 2015

@bsudekum we're switching to internal jsdoc documentation that will be eventually processed into external docs. For now:

setView

* @param {Number} pitch The angle at which the camera is looking at the ground

setPitch
* Sets a map angle

getPitch
getPitch: function() { return this.transform.pitch; },

easeTo
* @param {Number} pitch

example issue: #1062

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.

5 participants