-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Port asymmetric viewport rendering to gl-js #8638
Changes from all commits
a9cc2f9
537562c
1ce9ea1
e79edaf
03ac661
e01bafa
c8bb865
8bb8460
649e1d4
aa7fd47
7a479e3
bbba3ee
1cdf37f
64522f5
d0080a4
3833d3e
d10d082
29bd59e
21ff1b6
220673b
3420f3f
e39f561
d45f48f
b98ffea
39f4288
44a2654
00de0c6
372099a
e0871f7
354a555
fbb26fa
b7e8e6c
2d0e390
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,102 @@ | ||
// @flow | ||
import {number} from "../style-spec/util/interpolate"; | ||
import Point from "@mapbox/point-geometry"; | ||
import {clamp} from "../util/util"; | ||
|
||
/** | ||
* An `EdgeInset` object represents screen space padding applied to the edges of the viewport. | ||
* This shifts the apprent center or the vanishing point of the map. This is useful for adding floating UI elements | ||
* on top of the map and having the vanishing point shift as UI elements resize. | ||
* | ||
* @param {number} [top=0] | ||
* @param {number} [bottom=0] | ||
* @param {number} [left=0] | ||
* @param {number} [right=0] | ||
*/ | ||
class EdgeInsets { | ||
top: number; | ||
bottom: number; | ||
left: number; | ||
right: number; | ||
|
||
constructor(top: number = 0, bottom: number = 0, left: number = 0, right: number = 0) { | ||
if (isNaN(top) || top < 0 || | ||
isNaN(bottom) || bottom < 0 || | ||
isNaN(left) || left < 0 || | ||
isNaN(right) || right < 0 | ||
) { | ||
throw new Error('Invalid value for edge-insets, top, bottom, left and right must all be numbers'); | ||
} | ||
|
||
this.top = top; | ||
this.bottom = bottom; | ||
this.left = left; | ||
this.right = right; | ||
} | ||
|
||
/** | ||
* Interpolates the inset in-place. | ||
* This maintains the current inset value for any inset not present in `target`. | ||
* | ||
* @param {PaddingOptions} target | ||
* @param {number} t | ||
* @returns {EdgeInsets} | ||
* @memberof EdgeInsets | ||
*/ | ||
interpolate(start: PaddingOptions | EdgeInsets, target: PaddingOptions, t: number): EdgeInsets { | ||
if (target.top != null && start.top != null) this.top = number(start.top, target.top, t); | ||
if (target.bottom != null && start.bottom != null) this.bottom = number(start.bottom, target.bottom, t); | ||
if (target.left != null && start.left != null) this.left = number(start.left, target.left, t); | ||
if (target.right != null && start.right != null) this.right = number(start.right, target.right, t); | ||
|
||
return this; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This interpolates the padding between the previous frame's value and the target instead of from the starting value to the target. Switching to the more functional approach used for other interpolation could be a good idea. |
||
} | ||
|
||
/** | ||
* Utility method that computes the new apprent center or vanishing point after applying insets. | ||
* This is in pixels and with the top left being (0.0) and +y being downwards. | ||
* | ||
* @param {number} width | ||
* @param {number} height | ||
* @returns {Point} | ||
* @memberof EdgeInsets | ||
*/ | ||
getCenter(width: number, height: number): Point { | ||
// Clamp insets so they never overflow width/height and always calculate a valid center | ||
const x = clamp((this.left + width - this.right) / 2, 0, width); | ||
const y = clamp((this.top + height - this.bottom) / 2, 0, height); | ||
|
||
return new Point(x, y); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would this be simpler as this?
The main difference would be that in the case where There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the simpler one you suggested is better! |
||
} | ||
|
||
equals(other: PaddingOptions): boolean { | ||
return this.top === other.top && | ||
this.bottom === other.bottom && | ||
this.left === other.left && | ||
this.right === other.right; | ||
} | ||
|
||
clone(): EdgeInsets { | ||
return new EdgeInsets(this.top, this.bottom, this.left, this.right); | ||
} | ||
|
||
/** | ||
* Returns the current sdtate as json, useful when you want to have a | ||
* read-only representation of the inset. | ||
* | ||
* @returns {PaddingOptions} | ||
* @memberof EdgeInsets | ||
*/ | ||
toJSON(): PaddingOptions { | ||
return { | ||
top: this.top, | ||
bottom: this.bottom, | ||
left: this.left, | ||
right: this.right | ||
}; | ||
} | ||
} | ||
|
||
export type PaddingOptions = {top: ?number, bottom: ?number, right: ?number, left: ?number}; | ||
|
||
export default EdgeInsets; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we simplify this class somewhat, could it be small enough to the point it's simpler to have this state as a part of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It could be, but this seems easier to write unit tests for, and handles a lot of the validation and updating logic without adding additional private methods to the transform class. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,8 +9,10 @@ import {number as interpolate} from '../style-spec/util/interpolate'; | |
import EXTENT from '../data/extent'; | ||
import {vec4, mat4, mat2, vec2} from 'gl-matrix'; | ||
import {Aabb, Frustum} from '../util/primitives.js'; | ||
import EdgeInsets from './edge_insets'; | ||
|
||
import {UnwrappedTileID, OverscaledTileID, CanonicalTileID} from '../source/tile_id'; | ||
import type {PaddingOptions} from './edge_insets'; | ||
|
||
/** | ||
* A single transform, generally used for a single tile to be | ||
|
@@ -49,6 +51,7 @@ class Transform { | |
_minPitch: number; | ||
_maxPitch: number; | ||
_center: LngLat; | ||
_edgeInsets: EdgeInsets; | ||
_constraining: boolean; | ||
_posMatrixCache: {[string]: Float32Array}; | ||
_alignedPosMatrixCache: {[string]: Float32Array}; | ||
|
@@ -74,6 +77,7 @@ class Transform { | |
this._fov = 0.6435011087932844; | ||
this._pitch = 0; | ||
this._unmodified = true; | ||
this._edgeInsets = new EdgeInsets(); | ||
this._posMatrixCache = {}; | ||
this._alignedPosMatrixCache = {}; | ||
} | ||
|
@@ -90,6 +94,7 @@ class Transform { | |
clone._fov = this._fov; | ||
clone._pitch = this._pitch; | ||
clone._unmodified = this._unmodified; | ||
clone._edgeInsets = this._edgeInsets.clone(); | ||
clone._calcMatrices(); | ||
return clone; | ||
} | ||
|
@@ -137,8 +142,8 @@ class Transform { | |
return this.tileSize * this.scale; | ||
} | ||
|
||
get centerPoint(): Point { | ||
return this.size._div(2); | ||
get centerOffset(): Point { | ||
return this.centerPoint._sub(this.size._div(2)); | ||
} | ||
|
||
get size(): Point { | ||
|
@@ -204,6 +209,52 @@ class Transform { | |
this._calcMatrices(); | ||
} | ||
|
||
get padding(): PaddingOptions { return this._edgeInsets.toJSON(); } | ||
set padding(padding: PaddingOptions) { | ||
if (this._edgeInsets.equals(padding)) return; | ||
this._unmodified = false; | ||
//Update edge-insets inplace | ||
this._edgeInsets.interpolate(this._edgeInsets, padding, 1); | ||
this._calcMatrices(); | ||
} | ||
|
||
/** | ||
* The center of the screen in pixels with the top-left corner being (0,0) | ||
* and +y axis pointing downwards. This accounts for padding. | ||
* | ||
* @readonly | ||
* @type {Point} | ||
* @memberof Transform | ||
*/ | ||
get centerPoint(): Point { | ||
return this._edgeInsets.getCenter(this.width, this.height); | ||
} | ||
|
||
/** | ||
* Returns if the padding params match | ||
* | ||
* @param {PaddingOptions} padding | ||
* @returns {boolean} | ||
* @memberof Transform | ||
*/ | ||
isPaddingEqual(padding: PaddingOptions): boolean { | ||
return this._edgeInsets.equals(padding); | ||
} | ||
|
||
/** | ||
* Helper method to upadte edge-insets inplace | ||
* | ||
* @param {PaddingOptions} target | ||
* @param {number} t | ||
* @memberof Transform | ||
*/ | ||
interpolatePadding(start: PaddingOptions, target: PaddingOptions, t: number) { | ||
this._unmodified = false; | ||
this._edgeInsets.interpolate(start, target, t); | ||
this._constrain(); | ||
this._calcMatrices(); | ||
} | ||
|
||
/** | ||
* Return a zoom level that will cover all tiles the transform | ||
* @param {Object} options | ||
|
@@ -281,9 +332,10 @@ class Transform { | |
const centerPoint = [numTiles * centerCoord.x, numTiles * centerCoord.y, 0]; | ||
const cameraFrustum = Frustum.fromInvProjectionMatrix(this.invProjMatrix, this.worldSize, z); | ||
|
||
// No change of LOD behavior for pitch lower than 60: return only tile ids from the requested zoom level | ||
// No change of LOD behavior for pitch lower than 60 and when there is no top padding: return only tile ids from the requested zoom level | ||
let minZoom = options.minzoom || 0; | ||
if (this.pitch <= 60.0) | ||
// Use 0.1 as an epsilon to avoid for explicit == 0.0 floating point checks | ||
if (this.pitch <= 60.0 && this._edgeInsets.top < 0.1) | ||
minZoom = z; | ||
|
||
// There should always be a certain number of maximum zoom level tiles surrounding the center location | ||
|
@@ -616,15 +668,17 @@ class Transform { | |
_calcMatrices() { | ||
if (!this.height) return; | ||
|
||
this.cameraToCenterDistance = 0.5 / Math.tan(this._fov / 2) * this.height; | ||
const halfFov = this._fov / 2; | ||
const offset = this.centerOffset; | ||
this.cameraToCenterDistance = 0.5 / Math.tan(halfFov) * this.height; | ||
|
||
// Find the distance from the center point [width/2, height/2] to the | ||
// center top point [width/2, 0] in Z units, using the law of sines. | ||
// Find the distance from the center point [width/2 + offset.x, height/2 + offset.y] to the | ||
// center top point [width/2 + offset.x, 0] in Z units, using the law of sines. | ||
// 1 Z unit is equivalent to 1 horizontal px at the center of the map | ||
// (the distance between[width/2, height/2] and [width/2 + 1, height/2]) | ||
const halfFov = this._fov / 2; | ||
const groundAngle = Math.PI / 2 + this._pitch; | ||
const topHalfSurfaceDistance = Math.sin(halfFov) * this.cameraToCenterDistance / Math.sin(clamp(Math.PI - groundAngle - halfFov, 0.01, Math.PI - 0.01)); | ||
const fovAboveCenter = this._fov * (0.5 + offset.y / this.height); | ||
const topHalfSurfaceDistance = Math.sin(fovAboveCenter) * this.cameraToCenterDistance / Math.sin(clamp(Math.PI - groundAngle - fovAboveCenter, 0.01, Math.PI - 0.01)); | ||
const point = this.point; | ||
const x = point.x, y = point.y; | ||
|
||
|
@@ -646,6 +700,10 @@ class Transform { | |
let m = new Float64Array(16); | ||
mat4.perspective(m, this._fov, this.width / this.height, nearZ, farZ); | ||
|
||
//Apply center of perspective offset | ||
m[8] = -offset.x * 2 / this.width; | ||
m[9] = offset.y * 2 / this.height; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this be done using the methods provided by There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the only api method that allows that I think can work is the asymmetric view frustum method but that takes 4 different fov's as input, and back calculating that seemed more janky to me. |
||
mat4.scale(m, m, [1, -1, 1]); | ||
mat4.translate(m, m, [0, 0, -this.cameraToCenterDistance]); | ||
mat4.rotateX(m, m, this._pitch); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,13 +11,63 @@ import StencilMode from '../gl/stencil_mode'; | |
import CullFaceMode from '../gl/cull_face_mode'; | ||
import {debugUniformValues} from './program/debug_program'; | ||
import Color from '../style-spec/util/color'; | ||
import browser from '../util/browser'; | ||
|
||
import type Painter from './painter'; | ||
import type SourceCache from '../source/source_cache'; | ||
import type {OverscaledTileID} from '../source/tile_id'; | ||
|
||
export default drawDebug; | ||
|
||
const topColor = new Color(1, 0, 0, 1); | ||
const btmColor = new Color(0, 1, 0, 1); | ||
const leftColor = new Color(0, 0, 1, 1); | ||
const rightColor = new Color(1, 0, 1, 1); | ||
const centerColor = new Color(0, 1, 1, 1); | ||
|
||
export function drawDebugPadding(painter: Painter) { | ||
const padding = painter.transform.padding; | ||
const lineWidth = 3; | ||
// Top | ||
drawHorizontalLine(painter, painter.transform.height - (padding.top || 0), lineWidth, topColor); | ||
// Bottom | ||
drawHorizontalLine(painter, padding.bottom || 0, lineWidth, btmColor); | ||
// Left | ||
drawVerticalLine(painter, padding.left || 0, lineWidth, leftColor); | ||
// Right | ||
drawVerticalLine(painter, painter.transform.width - (padding.right || 0), lineWidth, rightColor); | ||
// Center | ||
const center = painter.transform.centerPoint; | ||
drawCrosshair(painter, center.x, painter.transform.height - center.y, centerColor); | ||
} | ||
|
||
function drawCrosshair(painter: Painter, x: number, y: number, color: Color) { | ||
const size = 20; | ||
const lineWidth = 2; | ||
//Vertical line | ||
drawDebugSSRect(painter, x - lineWidth / 2, y - size / 2, lineWidth, size, color); | ||
//Horizontal line | ||
drawDebugSSRect(painter, x - size / 2, y - lineWidth / 2, size, lineWidth, color); | ||
} | ||
|
||
function drawHorizontalLine(painter: Painter, y: number, lineWidth: number, color: Color) { | ||
drawDebugSSRect(painter, 0, y + lineWidth / 2, painter.transform.width, lineWidth, color); | ||
} | ||
|
||
function drawVerticalLine(painter: Painter, x: number, lineWidth: number, color: Color) { | ||
drawDebugSSRect(painter, x - lineWidth / 2, 0, lineWidth, painter.transform.height, color); | ||
} | ||
|
||
function drawDebugSSRect(painter: Painter, x: number, y: number, width: number, height: number, color: Color) { | ||
const context = painter.context; | ||
const gl = context.gl; | ||
|
||
gl.enable(gl.SCISSOR_TEST); | ||
gl.scissor(x * browser.devicePixelRatio, y * browser.devicePixelRatio, width * browser.devicePixelRatio, height * browser.devicePixelRatio); | ||
context.clear({color}); | ||
gl.disable(gl.SCISSOR_TEST); | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you anticipate this debug code being used often, or did it already serve its purpose since the asymmetric viewport implementation is complete? Just noting an opportunity to potentially cut this for smaller bundle size impact. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not super often, no actually, but it seems like an useful tool to have for render tests, examples and debug pages. I could make it so it is only available in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've also added some new render tests in the newest commit that represents the distortion caused by asymmetric viewport much better. |
||
function drawDebug(painter: Painter, sourceCache: SourceCache, coords: Array<OverscaledTileID>) { | ||
for (let i = 0; i < coords.length; i++) { | ||
drawDebugTile(painter, sourceCache, coords[i]); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks a little involved — can we get rid of the
if
s by defaulting them to0
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this logic ensures that the
if the key is not present the value does not change change from the existing value
logic for adding padding. Defaulting to 0 would remove the padding. and having it here ensures that that logic does not have to be implemented in every public interface function that accepts padding as input.