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

Allow adjusting camera clipbounds. #899

Merged
merged 3 commits into from
Aug 21, 2018
Merged

Allow adjusting camera clipbounds. #899

merged 3 commits into from
Aug 21, 2018

Conversation

manthey
Copy link
Contributor

@manthey manthey commented Aug 16, 2018

This was motivated by wanting to set the near and far clip values. This can be done by map.camera().clipbounds = {near: <near value>, far: <far value>}.

This simplifies some of the camera code. Before, we were generating a display matrix but separately calculating the transform in a series of steps, and similarly we were doing the inverse via a set of steps instead of a matrix multiplication. This uses the display and world matrices explicitly in the transforms.

The css transform property was ambiguous about the transform origin. We now explicitly expect a transform-origin of 0 0, as we use that elsewhere in the project. This adjusts several tests.

Some of the tests have be rewritten for clarity.

Some of the camera helper functions have been eliminated as they were simple wrappers around matrix functions.

There are now distinct clipbounds for perspective and parallel projections.

The css transform format was rewritten to be more compact and not lose precision in some instances.

Note that the webgl use of the perspective camera projection will need additional refactoring to handle any clipbounds.

This was motivated by wanting to set the near and far clip values.  This
can be done by `map.camera().clipbounds = {near: <near value>, far: <far
value>}`.

This simplifies some of the camera code.  Before, we were generating a
display matrix but separately calculating the transform in a series of
steps, and similarly we were doing the inverse via a set of steps
instead of a matrix multiplication.  This uses the `display` and `world`
matrices explicitly in the transforms.

The `css` transform property was ambiguous about the transform origin.
We now explicitly expect a transform-origin of `0 0`, as we use that
elsewhere in the project.  This adjusts several tests.

Some of the tests have be rewritten for clarity.

Some of the camera helper functions have been eliminated as they were
simple wrappers around matrix functions.

There are now distinct clipbounds for perspective and parallel
projections.

The css transform format was rewritten to be more compact and not lose
precision in some instances.

Note that the webgl use of the perspective camera projection will need
additional refactoring to handle any clipbounds.
@manthey manthey force-pushed the camera-clipbounds branch from 30d2ec8 to 3075e87 Compare August 16, 2018 18:27
far: -2,
near: -1
camera.clipbounds = {
perspective: {
Copy link
Member

Choose a reason for hiding this comment

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

typically for perspective, you use angle, aspect and near far only (e.g. see here: http://www.ntu.edu.sg/home/ehchua/programming/opengl/CG_examples.html). I think setting in terms of gcs bounds is also useful so perhaps we are fine for now.

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 fully expect that the perspective mode will change further when we start using it in earnest.

Copy link
Member

Choose a reason for hiding this comment

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

sounds good

@manthey manthey merged commit 65c4132 into master Aug 21, 2018
@manthey manthey deleted the camera-clipbounds branch August 21, 2018 17:36
@aashish24 aashish24 mentioned this pull request Aug 23, 2018
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