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.

Sign up for GitHub

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

Type matrix and vector calculations #11400

Merged
merged 12 commits into from
Jan 19, 2022
Merged

Type matrix and vector calculations #11400

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
a5ef619
add Flow types for gl-matrix
mourner Jan 14, 2022
2867fcd
update vec/matrix type references to proper types
mourner Jan 14, 2022
09d3ab0
fix more type references
mourner Jan 14, 2022
a288b3a
type more vec3/quat functions
mourner Jan 14, 2022
26f237c
various vec/mat typing fixes
mourner Jan 14, 2022
a717dcc
fix remaining matrix types
mourner Jan 17, 2022
ffc4c78
bump flow by 5 versions (last version that does not make new errors)
mourner Jan 17, 2022
5f01645
Merge branch 'main' into flow-type-gl-matrix
mourner Jan 19, 2022
4877111
fix up flow after merge
mourner Jan 19, 2022
0180b2a
revert globe matrix to double precision
mourner Jan 19, 2022
7e79b0b
revert globe tile matrix to double precision
mourner Jan 19, 2022
d758023
address feedback
mourner Jan 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .flowconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
.*/_site/.*

[version]
0.103.0
0.108.0

[options]
server.max_workers=4
Expand Down
102 changes: 102 additions & 0 deletions flow-typed/gl-matrix.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@

type VecType = Array<number> | Float32Array | Float64Array;

declare module "gl-matrix" {
declare type Vec2 = VecType;
declare type Vec3 = VecType;
declare type Vec4 = VecType;
declare type Quat = VecType;
declare type Mat2 = VecType;
declare type Mat3 = VecType;
declare type Mat4 = VecType;

declare var vec2: {
exactEquals(Vec2, Vec2): boolean
};

declare var vec3: {
create(): Float32Array,
fromValues(number, number, number): Float32Array,
length(Vec3): number,
len(Vec3): number,
squaredLength(Vec3): number,
dot(Vec3, Vec3): number,
equals(Vec3, Vec3): boolean,
exactEquals(Vec3, Vec3): boolean,

clone<T: Vec3>(T): T,
normalize<T: Vec3>(T, Vec3): T,
add<T: Vec3>(T, Vec3, Vec3): T,
sub<T: Vec3>(T, Vec3, Vec3): T,
subtract<T: Vec3>(T, Vec3, Vec3): T,
cross<T: Vec3>(T, Vec3, Vec3): T,
negate<T: Vec3>(T, Vec3): T,
scale<T: Vec3>(T, Vec3, number): T,
scaleAndAdd<T: Vec3>(T, Vec3, Vec3, number): T,
multiply<T: Vec3>(T, Vec3, Vec3): T,
mul<T: Vec3>(T, Vec3, Vec3): T,
div<T: Vec3>(T, Vec3, Vec3): T,
min<T: Vec3>(T, Vec3, Vec3): T,
max<T: Vec3>(T, Vec3, Vec3): T,
transformQuat<T: Vec3>(T, Vec3, Quat): T,
transformMat3<T: Vec3>(T, Vec3, Mat3): T,
transformMat4<T: Vec3>(T, Vec3, Mat4): T
};

declare var vec4: {
scale<T: Vec4>(T, Vec4, number): T,
mul<T: Vec4>(T, Vec4, Vec4): T,
transformMat4<T: Vec4>(T, Vec4, Mat4): T
};

declare var mat2: {
create(): Float32Array,
rotate<T: Mat2>(T, Mat2, number): T,
invert<T: Mat2>(T, Mat2): T,
scale<T: Mat2>(T, Mat2, Vec2): T
};

declare var mat3: {
create(): Float32Array,

fromMat4<T: Mat3>(T, Mat4): T,
fromRotation<T: Mat3>(T, number): T,
mul<T: Mat3>(T, Mat3, Mat3): T,
multiply<T: Mat3>(T, Mat3, Mat3): T,
adjoint<T: Mat3>(T, Mat3): T,
transpose<T: Mat3>(T, Mat3): T
};

declare var mat4: {
create(): Float32Array,

fromScaling<T: Mat4>(T, Vec3): T,
fromQuat<T: Mat4>(T, Quat): T,
ortho<T: Mat4>(T, number, number, number, number, number, number): T,
perspective<T: Mat4>(T, number, number, number, number): T,
identity<T: Mat4>(T): T,
scale<T: Mat4>(T, Mat4, Vec3): T,
mul<T: Mat4>(T, Mat4, Mat4): T,
multiply<T: Mat4>(T, Mat4, Mat4): T,
rotateX<T: Mat4>(T, Mat4, number): T,
rotateY<T: Mat4>(T, Mat4, number): T,
rotateZ<T: Mat4>(T, Mat4, number): T,
translate<T: Mat4>(T, Mat4, Vec3): T,
invert<T: Mat4>(T, Mat4): T,
copy<T: Mat4>(T, Mat4): T,
clone<T: Mat4>(T): T
};

declare var quat: {
create(): Float32Array,
length(Quat): number,
exactEquals(Quat, Quat): boolean,

normalize<T: Quat>(T, Quat): T,
conjugate<T: Quat>(T, Quat): T,
identity<T: Quat>(T): T,
rotateX<T: Quat>(T, Quat, number): T,
rotateY<T: Quat>(T, Quat, number): T,
rotateZ<T: Quat>(T, Quat, number): T
}
}
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
"eslint-plugin-html": "^6.1.1",
"eslint-plugin-import": "^2.22.1",
"eslint-plugin-jsdoc": "^32.3.4",
"flow-bin": "0.103.0",
"flow-bin": "0.108.0",
"gl": "^4.9.0",
"glob": "^7.1.6",
"is-builtin-module": "^3.0.0",
Expand Down
5 changes: 3 additions & 2 deletions src/data/bucket/symbol_bucket.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ export type SingleCollisionBox = {
elevation?: number;
tileID?: OverscaledTileID;
};
import type {Mat4} from 'gl-matrix';

export type CollisionArrays = {
textBox?: SingleCollisionBox;
Expand Down Expand Up @@ -349,8 +350,8 @@ class SymbolBucket implements Bucket {
featureSortOrder: Array<number>;

collisionCircleArray: Array<number>;
placementInvProjMatrix: mat4;
placementViewportMatrix: mat4;
placementInvProjMatrix: Mat4;
placementViewportMatrix: Mat4;

text: SymbolBuffers;
icon: SymbolBuffers;
Expand Down
10 changes: 5 additions & 5 deletions src/data/dem_tree.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {vec3} from 'gl-matrix';
import {number as interpolate} from '../style-spec/util/interpolate.js';
import {clamp} from '../util/util.js';

type vec3Like = vec3 | [number, number, number];
import type {Vec3} from 'gl-matrix';

class MipLevel {
size: number;
Expand Down Expand Up @@ -37,7 +37,7 @@ class MipLevel {
}
}

function aabbRayIntersect(min: vec3Like, max: vec3Like, pos: vec3Like, dir: vec3Like): ?number {
function aabbRayIntersect(min: Vec3, max: Vec3, pos: Vec3, dir: Vec3): ?number {
let tMin = 0;
let tMax = Number.MAX_VALUE;

Expand Down Expand Up @@ -69,7 +69,7 @@ function aabbRayIntersect(min: vec3Like, max: vec3Like, pos: vec3Like, dir: vec3
return tMin;
}

function triangleRayIntersect(ax, ay, az, bx, by, bz, cx, cy, cz, pos: vec3Like, dir: vec3Like): ?number {
function triangleRayIntersect(ax, ay, az, bx, by, bz, cx, cy, cz, pos: Vec3, dir: Vec3): ?number {
// Compute barycentric coordinates u and v to find the intersection
const abX = bx - ax;
const abY = by - ay;
Expand Down Expand Up @@ -178,13 +178,13 @@ export default class DemMinMaxQuadTree {
}

// Performs raycast against the tree root only. Min and max coordinates defines the size of the root node
raycastRoot(minx: number, miny: number, maxx: number, maxy: number, p: vec3Like, d: vec3Like, exaggeration: number = 1): ?number {
raycastRoot(minx: number, miny: number, maxx: number, maxy: number, p: Vec3, d: Vec3, exaggeration: number = 1): ?number {
const min = [minx, miny, -aabbSkirtPadding];
const max = [maxx, maxy, this.maximums[0] * exaggeration];
return aabbRayIntersect(min, max, p, d);
}

raycast(rootMinx: number, rootMiny: number, rootMaxx: number, rootMaxy: number, p: vec3Like, d: vec3Like, exaggeration: number = 1): ?number {
raycast(rootMinx: number, rootMiny: number, rootMaxx: number, rootMaxy: number, p: Vec3, d: Vec3, exaggeration: number = 1): ?number {
if (!this.nodeCount)
return null;

Expand Down
10 changes: 6 additions & 4 deletions src/geo/projection/flat_tile_transform.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@
import type Transform from '../transform.js';
import type {ElevationScale} from './index.js';
import {UnwrappedTileID} from '../../source/tile_id.js';
import {mat4, vec3} from 'gl-matrix';
import {mat4} from 'gl-matrix';
import MercatorCoordinate from '../mercator_coordinate.js';
import Point from '@mapbox/point-geometry';
import EXTENT from '../../data/extent.js';
import tileTransform from './tile_transform.js';

import type {Mat4, Vec3} from 'gl-matrix';

const identity = mat4.identity(new Float64Array(16));

export default class FlatTileTransform {
Expand All @@ -20,11 +22,11 @@ export default class FlatTileTransform {
this._worldSize = worldSize;
}

createInversionMatrix(): mat4 {
createInversionMatrix(): Mat4 {
return identity;
}

createTileMatrix(id: UnwrappedTileID): mat4 {
createTileMatrix(id: UnwrappedTileID): Mat4 {
let scale, scaledX, scaledY;
const canonical = id.canonical;
const posMatrix = mat4.identity(new Float64Array(16));
Expand Down Expand Up @@ -55,7 +57,7 @@ export default class FlatTileTransform {
return this._tr.rayIntersectionCoordinate(this._tr.pointRayIntersection(clamped, z));
}

upVector(): vec3 {
upVector(): Vec3 {
return [0, 0, 1];
}

Expand Down
14 changes: 7 additions & 7 deletions src/geo/projection/globe.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ export function globeECEFUnitsToPixelScale(worldSize: number) {
return wsRadius / localRadius;
}

export function calculateGlobeMatrix(tr: Transform, worldSize: number, offset?: [number, number]): mat4 {
export function calculateGlobeMatrix(tr: Transform, worldSize: number, offset?: [number, number]): Float32Array {
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe reducing precision of some of the intermediate matrices from 64 bits to 32 bits will become problematic. Globe matrix is pretty much equivalent of tile matrices (createTileMatrix for other projections) meaning that it requires high precision for representing pixel coordinates at all zoom levels. Any precision issues might be masked though by the low zoom threshold value we have for the globe view but it will eventually bite us back.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, that's the moment I want to check up on — which of these matrices are intermediary and need high precision? I tried to keep them Float64Array but had a hard time reconciling typing errors when some of these matrices eventually get passed to WebGL methods that expect Float32Array.

Copy link
Member Author

Choose a reason for hiding this comment

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

@mpulkki-mapbox I reverted globe matrix to double precision — any other matrices I need to do this for?

Copy link
Contributor

Choose a reason for hiding this comment

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

The rule of thumb would be that any matrix that is multiplied with transform.projMatrix should be kept 64bit. I'll go through the list quickly.

Copy link
Contributor

Choose a reason for hiding this comment

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

@mourner: it's actually quite difficult to tell whether current type annotations for matrices are correct or not. For example the function signature calculateProjMatrix(unwrappedTileID: UnwrappedTileID, aligned: boolean = false): Float32Array in transform.js actually returns 64bit matrices as all possible code paths inside the function returns Float64Arrays. Similar patterns exists in other parts too. Or am I overlooking something perhaps?

Copy link
Member Author

Choose a reason for hiding this comment

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

@mpulkki-mapbox how so? It's explicitly returning Float32Array at the moment:

mapbox-gl-js/src/geo/transform.js

Lines 1444 to 1445 in 74559af

cache[projMatrixKey] = new Float32Array(posMatrix);
return cache[projMatrixKey];

The new type mismatches in the new branch are caught pretty well — certainly a huge improvement over silently passing mistypes in main.

Copy link
Contributor

Choose a reason for hiding this comment

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

oh you're correct! The conversion is done at the end of the function call :). It's perhaps the calculatePosMatrix I was looking at. That function will forward the matrix creation to a specific TileTransform class and both FlatTileTransform and GlobeTileTransform will return 64 bit matrices. Am I looking things correctly now? :)

Copy link
Member Author

Choose a reason for hiding this comment

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

@mpulkki-mapbox I reverted createTileMatrix to Float64Array, so now it should at least be consistent. I guess it can't fully figure out the indirection in this instance to see that it doesn't match the type defined in projection/index.js, but upgrading Flow to the latest (2.5 years newer) version with much more sound typing architecture (which is my end goal) might help.

const wsRadius = worldSize / (2.0 * Math.PI);
const scale = globeECEFUnitsToPixelScale(worldSize);

Expand All @@ -239,10 +239,10 @@ export function calculateGlobeMatrix(tr: Transform, worldSize: number, offset?:
mat4.rotateX(posMatrix, posMatrix, degToRad(-tr._center.lat));
mat4.rotateY(posMatrix, posMatrix, degToRad(-tr._center.lng));

return posMatrix;
return Float32Array.from(posMatrix);
}

export function calculateGlobeMercatorMatrix(tr: Transform): mat4 {
export function calculateGlobeMercatorMatrix(tr: Transform): Float32Array {
const worldSize = tr.worldSize;
const lat = clamp(tr.center.lat, -MAX_MERCATOR_LATITUDE, MAX_MERCATOR_LATITUDE);
const point = new Point(
Expand All @@ -258,7 +258,7 @@ export function calculateGlobeMercatorMatrix(tr: Transform): mat4 {
mat4.translate(posMatrix, posMatrix, [point.x, point.y, 0.0]);
mat4.scale(posMatrix, posMatrix, [ws, ws, zScale]);

return posMatrix;
return Float32Array.from(posMatrix);
karimnaaji marked this conversation as resolved.
Show resolved Hide resolved
}

export const GLOBE_ZOOM_THRESHOLD_MIN = 5;
Expand Down Expand Up @@ -288,11 +288,11 @@ export function globeBuffersForTileMesh(painter: Painter, tile: Tile, coord: Ove
return [gridBuffer, poleBuffer];
}

export function globeMatrixForTile(id: CanonicalTileID, globeMatrix: mat4) {
export function globeMatrixForTile(id: CanonicalTileID, globeMatrix: Float32Array) {
const decode = globeDenormalizeECEF(globeTileBounds(id));
const posMatrix = mat4.copy(new Float64Array(16), globeMatrix);
mat4.mul(posMatrix, posMatrix, decode);
return posMatrix;
return Float32Array.from(posMatrix);
}

export function globePoleMatrixForTile(id: CanonicalTileID, south: boolean, tr: Transform) {
Expand All @@ -315,7 +315,7 @@ export function globePoleMatrixForTile(id: CanonicalTileID, south: boolean, tr:
mat4.scale(poleMatrix, poleMatrix, [1, -1, 1]);
}

return poleMatrix;
return Float32Array.from(poleMatrix);
karimnaaji marked this conversation as resolved.
Show resolved Hide resolved
}

export class GlobeSharedBuffers {
Expand Down
15 changes: 9 additions & 6 deletions src/geo/projection/globe_tile_transform.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,26 +17,28 @@ import {
globeDenormalizeECEF
} from './globe.js';

import type {Vec3} from 'gl-matrix';

const GLOBE_RADIUS = EXTENT / Math.PI / 2.0;
const GLOBE_METERS_TO_ECEF = mercatorZfromAltitude(1, 0.0) * 2.0 * GLOBE_RADIUS * Math.PI;

export default class GlobeTileTransform {
_tr: Transform;
_worldSize: number;
_globeMatrix: Float64Array;
_globeMatrix: Float32Array;

constructor(tr: Transform, worldSize: number) {
this._tr = tr;
this._worldSize = worldSize;
this._globeMatrix = calculateGlobeMatrix(tr, worldSize);
}

createTileMatrix(id: UnwrappedTileID): mat4 {
createTileMatrix(id: UnwrappedTileID): Float32Array {
mourner marked this conversation as resolved.
Show resolved Hide resolved
const decode = globeDenormalizeECEF(globeTileBounds(id.canonical));
return mat4.multiply([], this._globeMatrix, decode);
return mat4.multiply(mat4.create(), this._globeMatrix, decode);
}

createInversionMatrix(id: UnwrappedTileID): mat4 {
createInversionMatrix(id: UnwrappedTileID): Float32Array {
const identity = mat4.identity(new Float64Array(16));

const center = this._tr.center;
Expand All @@ -57,11 +59,12 @@ export default class GlobeTileTransform {
const ecefUnitsToMercatorPixels = wsRadius / localRadius;

mat4.scale(identity, identity, [ecefUnitsToMercatorPixels, ecefUnitsToMercatorPixels, 1.0]);
mat4.multiply(matrix, matrix, identity);

return mat4.multiply(matrix, matrix, identity);
return Float32Array.from(matrix);
}

upVector(id: CanonicalTileID, x: number, y: number): vec3 {
upVector(id: CanonicalTileID, x: number, y: number): Vec3 {
const tiles = 1 << id.z;
const mercX = (x / EXTENT + id.x) / tiles;
const mercY = (y / EXTENT + id.y) / tiles;
Expand Down
7 changes: 3 additions & 4 deletions src/geo/projection/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import cylindricalEqualArea from './cylindrical_equal_area.js';
import {extend} from '../../util/util.js';
import type {ProjectionSpecification} from '../../style-spec/types.js';
import globe from './globe.js';
import {mat4, vec3} from 'gl-matrix';
import {CanonicalTileID, UnwrappedTileID} from '../../source/tile_id.js';
import Transform from '../transform.js';
import LngLat from '../lng_lat.js';
Expand Down Expand Up @@ -92,9 +91,9 @@ export type ElevationScale = {
}

export type TileTransform = {
createTileMatrix: (id: UnwrappedTileID) => mat4,
createInversionMatrix: (id: UnwrappedTileID) => mat4,
upVector: (id: CanonicalTileID, x: number, y: number) => vec3,
createTileMatrix: (id: UnwrappedTileID) => Float32Array,
createInversionMatrix: (id: UnwrappedTileID) => Float32Array,
upVector: (id: CanonicalTileID, x: number, y: number) => [number, number, number],
upVectorScale: (id: CanonicalTileID, latitude: number, worldSize: number) => ElevationScale,
pointCoordinate: (x: number, y: number, z?: number) => MercatorCoordinate
};
4 changes: 3 additions & 1 deletion src/geo/projection/tile_transform.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import type Transform from '../transform.js';
import {UnwrappedTileID, CanonicalTileID} from '../../source/tile_id.js';
import assert from 'assert';

import type {Vec3} from 'gl-matrix';

export type TileTransform = {
scale: number,
x: number,
Expand Down Expand Up @@ -134,7 +136,7 @@ export function getTilePoint(tileTransform: TileTransform, {x, y}: {x: number, y
(y * tileTransform.scale - tileTransform.y) * EXTENT);
}

export function getTileVec3(tileTransform: TileTransform, coord: MercatorCoordinate, wrap: number = 0): vec3 {
export function getTileVec3(tileTransform: TileTransform, coord: MercatorCoordinate, wrap: number = 0): Vec3 {
const x = ((coord.x - wrap) * tileTransform.scale - tileTransform.x) * EXTENT;
const y = (coord.y * tileTransform.scale - tileTransform.y) * EXTENT;
return vec3.fromValues(x, y, altitudeFromMercatorZ(coord.z, coord.y));
Expand Down
Loading