Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Consider EdgeInsets in the projection matrix #12107

Closed
brunoabinader opened this issue Jun 9, 2018 · 2 comments
Closed

Consider EdgeInsets in the projection matrix #12107

brunoabinader opened this issue Jun 9, 2018 · 2 comments
Labels
bug Core The cross-platform C++ core, aka mbgl

Comments

@brunoabinader
Copy link
Member

Our projection matrix does not consider the values from EdgeInsets when calculating perspective, causing the focal point to be always based on the view's horizontal center.

This is relevant in cases where we e.g. update the map position snapped on a street that is a straight line, but because EdgeInsets are applied, the map center differs from the view center and thus causes the street line to be skewed.

Example: on the screenshot below, the map occupies the whole view area, but EdgeInsets are applied so the effective map center is the center of the bounding box (red rectangle):

/cc @mapbox/gl-core

@brunoabinader brunoabinader added bug Core The cross-platform C++ core, aka mbgl labels Jun 9, 2018
@pozdnyakov pozdnyakov self-assigned this Jun 19, 2018
@pozdnyakov pozdnyakov removed their assignment Jul 3, 2018
@stale stale bot added the archived Archived because of inactivity label Dec 30, 2018
@stale
Copy link

stale bot commented Dec 30, 2018

This issue has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

@stale stale bot closed this as completed Dec 30, 2018
@julianrex julianrex reopened this Jan 2, 2019
@stale stale bot removed the archived Archived because of inactivity label Jan 2, 2019
@julianrex
Copy link
Contributor

/cc @d-prukop

astojilj added a commit that referenced this issue May 14, 2019
The change is implemented in TransformState::getProjMatrix, the rest of the code is making sure that existing API contracts stay and there are tests verifyingrendering and render query processing only items within screen and given tolerance around screen edges.

MapView: don't bake edge insets into relalculated camera center. Keep edge insets as property of camera in TransformState (similar to pitch, zoom, bearing) independent from specified camera center. Interpolate edge insets in animation.

iOS Demo app: "Turn On/Off Content Insets" pitch the camera and navigate to convenient location in Denver, where streets are parallel to cardinal directions, to illustrate viewport center offset when edge insets are set.

Tests:
ViewFrustumCulling: although Annotations are deprecated, queryRenderedFeatures related tests in Annotations would need to get ported and decided to add the edge insets related query tests next to them. Verify frustum culling (render+queryRenderedFeatures) With different camera and edge insets setups. TODO: port Annotations tests.

Transform.Padding: Verify that coordinates take proper place on screen after applying edge insets.

LocalGlyphRasterizer: verify text rendering when applying padding.

Related to #11882: both use projection matrix elements [8] and [9].

Alternative approach to this was to increase and offset map origin so that the screen would be a sub-rectangle in larger map viewport. This approach has a drawback of unecessary processing the items that are outside screen area.

Fixes #12107, #12728, navigation-sdks/issues/120
astojilj added a commit that referenced this issue May 28, 2019
The change is implemented in TransformState::getProjMatrix, the rest of the code is making sure that existing API contracts stay and there are tests verifyingrendering and render query processing only items within screen and given tolerance around screen edges.

MapView: don't bake edge insets into relalculated camera center. Keep edge insets as property of camera in TransformState (similar to pitch, zoom, bearing) independent from specified camera center. Interpolate edge insets in animation.

iOS Demo app: "Turn On/Off Content Insets" pitch the camera and navigate to convenient location in Denver, where streets are parallel to cardinal directions, to illustrate viewport center offset when edge insets are set.

Tests:
ViewFrustumCulling: although Annotations are deprecated, queryRenderedFeatures related tests in Annotations would need to get ported and decided to add the edge insets related query tests next to them. Verify frustum culling (render+queryRenderedFeatures) With different camera and edge insets setups. TODO: port Annotations tests.

Transform.Padding: Verify that coordinates take proper place on screen after applying edge insets.

LocalGlyphRasterizer: verify text rendering when applying padding.

Related to #11882: both use projection matrix elements [8] and [9].

Alternative approach to this was to increase and offset map origin so that the screen would be a sub-rectangle in larger map viewport. This approach has a drawback of unecessary processing the items that are outside screen area.

Fixes #12107, #12728, navigation-sdks/issues/120
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Core The cross-platform C++ core, aka mbgl
Projects
None yet
Development

No branches or pull requests

3 participants