Skip to content

Commit

Permalink
Refactor usage of implicit parameter to an explicit one when rendering (
Browse files Browse the repository at this point in the history
#5020)

* Use explicit parameter instead of implicit one

* Add parameter to all draw functions

* Add predicates

* Don't use default parameter

* Move to apply instead of ignore

* Move booleans to type

* Fixes

* Rename RenderFlags to RenderOptions

* Rename parameter and change switch

* More fixes

* More renaming

* Add explicit values

* Fix tests
  • Loading branch information
ibesora authored Nov 15, 2024
1 parent 3d318e8 commit 5de0876
Show file tree
Hide file tree
Showing 38 changed files with 211 additions and 162 deletions.
12 changes: 6 additions & 6 deletions src/geo/projection/globe_transform.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ function planeDistance(point: Array<number>, plane: Array<number>) {
}

function createGlobeTransform(globeProjection: GlobeProjection) {
const globeTransform = new GlobeTransform(globeProjection);
const globeTransform = new GlobeTransform(globeProjection, true);
globeTransform.resize(640, 480);
globeTransform.setFov(45);
return globeTransform;
Expand All @@ -46,14 +46,14 @@ describe('GlobeTransform', () => {
expectToBeCloseToArray(projectionData.tileMercatorCoords, [0.5, 0, 0.5 / EXTENT, 0.5 / EXTENT]);
});

test('Globe transition is not 0 when not ignoring the globe matrix', () => {
test('Globe transition is 0 when not applying the globe matrix', () => {
const projectionData = globeTransform.getProjectionData({overscaledTileID: new OverscaledTileID(1, 0, 1, 1, 0)});
expect(projectionData.projectionTransition).not.toBe(0);
expect(projectionData.projectionTransition).toBe(0);
});

test('Ignoring the globe matrix sets transition to 0', () => {
const projectionData = globeTransform.getProjectionData({overscaledTileID: new OverscaledTileID(1, 0, 1, 1, 0), ignoreGlobeMatrix: true});
expect(projectionData.projectionTransition).toBe(0);
test('Applying the globe matrix sets transition to something different than 0', () => {
const projectionData = globeTransform.getProjectionData({overscaledTileID: new OverscaledTileID(1, 0, 1, 1, 0), applyGlobeMatrix: true});
expect(projectionData.projectionTransition).not.toBe(0);
});
});

Expand Down
10 changes: 5 additions & 5 deletions src/geo/projection/globe_transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -454,16 +454,16 @@ export class GlobeTransform implements ITransform {
}

getProjectionData(params: ProjectionDataParams): ProjectionData {
const {overscaledTileID, aligned, ignoreTerrainMatrix, ignoreGlobeMatrix} = params;
const data = this._mercatorTransform.getProjectionData({overscaledTileID, aligned, ignoreTerrainMatrix});
const {overscaledTileID, aligned, applyTerrainMatrix, applyGlobeMatrix} = params;
const data = this._mercatorTransform.getProjectionData({overscaledTileID, aligned, applyTerrainMatrix});

// Set 'projectionMatrix' to actual globe transform
if (this.isGlobeRendering) {
data.mainMatrix = this._globeViewProjMatrix;
}

data.clippingPlane = this._cachedClippingPlane as [number, number, number, number];
data.projectionTransition = ignoreGlobeMatrix ? 0 : this._globeness;
data.projectionTransition = applyGlobeMatrix ? this._globeness : 0;

return data;
}
Expand Down Expand Up @@ -1184,8 +1184,8 @@ export class GlobeTransform implements ITransform {
return m;
}

getProjectionDataForCustomLayer(): ProjectionData {
const projectionData = this.getProjectionData({overscaledTileID: new OverscaledTileID(0, 0, 0, 0, 0)});
getProjectionDataForCustomLayer(applyGlobeMatrix: boolean = true): ProjectionData {
const projectionData = this.getProjectionData({overscaledTileID: new OverscaledTileID(0, 0, 0, 0, 0), applyGlobeMatrix});
projectionData.tileMercatorCoords = [0, 0, 1, 1];

// Even though we requested projection data for the mercator base tile which covers the entire mercator range,
Expand Down
8 changes: 4 additions & 4 deletions src/geo/projection/mercator_transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -759,9 +759,9 @@ export class MercatorTransform implements ITransform {
}

getProjectionData(params: ProjectionDataParams): ProjectionData {
const {overscaledTileID, aligned, ignoreTerrainMatrix} = params;
const {overscaledTileID, aligned, applyTerrainMatrix} = params;
const matrix = overscaledTileID ? this.calculatePosMatrix(overscaledTileID, aligned) : null;
return getBasicProjectionData(overscaledTileID, matrix, ignoreTerrainMatrix);
return getBasicProjectionData(overscaledTileID, matrix, applyTerrainMatrix);
}

isLocationOccluded(_: LngLat): boolean {
Expand Down Expand Up @@ -833,9 +833,9 @@ export class MercatorTransform implements ITransform {
return m;
}

getProjectionDataForCustomLayer(): ProjectionData {
getProjectionDataForCustomLayer(applyGlobeMatrix: boolean = true): ProjectionData {
const tileID = new OverscaledTileID(0, 0, 0, 0, 0);
const projectionData = this.getProjectionData({overscaledTileID: tileID, ignoreTerrainMatrix: true});
const projectionData = this.getProjectionData({overscaledTileID: tileID, applyGlobeMatrix});

const tileMatrix = calculateTileMatrix(tileID, this.worldSize);
mat4.multiply(tileMatrix, this._viewProjMatrix, tileMatrix);
Expand Down
4 changes: 2 additions & 2 deletions src/geo/projection/mercator_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export function getMercatorHorizon(transform: {pitch: number; cameraToCenterDist
Math.tan(degreesToRadians(maxMercatorHorizonAngle - transform.pitch)));
}

export function getBasicProjectionData(overscaledTileID: OverscaledTileID, tilePosMatrix?: mat4, ignoreTerrainMatrix?: boolean): ProjectionData {
export function getBasicProjectionData(overscaledTileID: OverscaledTileID, tilePosMatrix?: mat4, applyTerrainMatrix: boolean = true): ProjectionData {
let tileOffsetSize: [number, number, number, number];

if (overscaledTileID) {
Expand All @@ -110,7 +110,7 @@ export function getBasicProjectionData(overscaledTileID: OverscaledTileID, tileP
}

let mainMatrix: mat4;
if (overscaledTileID && overscaledTileID.terrainRttPosMatrix && !ignoreTerrainMatrix) {
if (overscaledTileID && overscaledTileID.terrainRttPosMatrix && applyTerrainMatrix) {
mainMatrix = overscaledTileID.terrainRttPosMatrix;
} else if (tilePosMatrix) {
mainMatrix = tilePosMatrix;
Expand Down
8 changes: 4 additions & 4 deletions src/geo/projection/projection_data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,11 @@ export type ProjectionDataParams = {
*/
aligned?: boolean;
/**
* Set to true if the terrain matrix should be ignored
* Set to true if the terrain matrix should be applied (i.e. when rendering terrain)
*/
ignoreTerrainMatrix?: boolean;
applyTerrainMatrix?: boolean;
/**
* Set to true if the globe matrix should be ignored (i.e. when rendering to texture for terrain)
* Set to true if the globe matrix should be applied (i.e. when rendering globe)
*/
ignoreGlobeMatrix?: boolean;
applyGlobeMatrix?: boolean;
}
2 changes: 1 addition & 1 deletion src/geo/transform_interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,7 @@ export interface IReadonlyTransform extends ITransformGetters {
/**
* Return projection data such that coordinates in mercator projection in range 0..1 will get projected to the map correctly.
*/
getProjectionDataForCustomLayer(): ProjectionData;
getProjectionDataForCustomLayer(applyGlobeMatrix: boolean): ProjectionData;

/**
* Returns a tile-specific projection matrix. Used for symbol placement fast-path for mercator transform.
Expand Down
9 changes: 5 additions & 4 deletions src/render/draw_background.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,25 @@ import {
backgroundPatternUniformValues
} from './program/background_program';

import type {Painter} from './painter';
import type {Painter, RenderOptions} from './painter';
import type {SourceCache} from '../source/source_cache';
import type {BackgroundStyleLayer} from '../style/style_layer/background_style_layer';
import {OverscaledTileID} from '../source/tile_id';
import {coveringTiles} from '../geo/projection/covering_tiles';

export function drawBackground(painter: Painter, sourceCache: SourceCache, layer: BackgroundStyleLayer, coords?: Array<OverscaledTileID>) {
export function drawBackground(painter: Painter, sourceCache: SourceCache, layer: BackgroundStyleLayer, coords: Array<OverscaledTileID>, renderOptions: RenderOptions) {
const color = layer.paint.get('background-color');
const opacity = layer.paint.get('background-opacity');

if (opacity === 0) return;

const {isRenderingToTexture} = renderOptions;
const context = painter.context;
const gl = context.gl;
const projection = painter.style.projection;
const transform = painter.transform;
const tileSize = transform.tileSize;
const image = layer.paint.get('background-pattern');
const globeWithTerrain = painter.style.map.terrain && painter.style.projection.name === 'globe';

if (painter.isPatternMissing(image)) return;

Expand All @@ -47,7 +47,8 @@ export function drawBackground(painter: Painter, sourceCache: SourceCache, layer
for (const tileID of tileIDs) {
const projectionData = transform.getProjectionData({
overscaledTileID: tileID,
ignoreGlobeMatrix: globeWithTerrain
applyGlobeMatrix: !isRenderingToTexture,
applyTerrainMatrix: true
});

const uniformValues = image ?
Expand Down
7 changes: 4 additions & 3 deletions src/render/draw_circle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {circleUniformValues} from './program/circle_program';
import {SegmentVector} from '../data/segment';
import {OverscaledTileID} from '../source/tile_id';

import type {Painter} from './painter';
import type {Painter, RenderOptions} from './painter';
import type {SourceCache} from '../source/source_cache';
import type {CircleStyleLayer} from '../style/style_layer/circle_style_layer';
import type {CircleBucket} from '../data/bucket/circle_bucket';
Expand Down Expand Up @@ -35,9 +35,10 @@ type SegmentsTileRenderState = {
state: TileRenderState;
};

export function drawCircles(painter: Painter, sourceCache: SourceCache, layer: CircleStyleLayer, coords: Array<OverscaledTileID>) {
export function drawCircles(painter: Painter, sourceCache: SourceCache, layer: CircleStyleLayer, coords: Array<OverscaledTileID>, renderOptions: RenderOptions) {
if (painter.renderPass !== 'translucent') return;

const {isRenderingToTexture} = renderOptions;
const opacity = layer.paint.get('circle-opacity');
const strokeWidth = layer.paint.get('circle-stroke-width');
const strokeOpacity = layer.paint.get('circle-stroke-opacity');
Expand Down Expand Up @@ -80,7 +81,7 @@ export function drawCircles(painter: Painter, sourceCache: SourceCache, layer: C
const terrainData = painter.style.map.terrain && painter.style.map.terrain.getTerrainData(coord);
const uniformValues = circleUniformValues(painter, tile, layer, translateForUniforms, radiusCorrectionFactor);

const projectionData = transform.getProjectionData({overscaledTileID: coord});
const projectionData = transform.getProjectionData({overscaledTileID: coord, applyGlobeMatrix: !isRenderingToTexture, applyTerrainMatrix: true});

const state: TileRenderState = {
programConfiguration,
Expand Down
2 changes: 1 addition & 1 deletion src/render/draw_collision_debug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export function drawCollisionDebug(painter: Painter, sourceCache: SourceCache, l
CullFaceMode.disabled,
collisionUniformValues(painter.transform),
painter.style.map.terrain && painter.style.map.terrain.getTerrainData(coord),
transform.getProjectionData({overscaledTileID: coord}),
transform.getProjectionData({overscaledTileID: coord, applyGlobeMatrix: true, applyTerrainMatrix: true}),
layer.id, buffers.layoutVertexBuffer, buffers.indexBuffer,
buffers.segments, null, painter.transform.zoom, null, null,
buffers.collisionVertexBuffer);
Expand Down
5 changes: 3 additions & 2 deletions src/render/draw_custom.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {OverscaledTileID} from '../source/tile_id';
import {SourceCache} from '../source/source_cache';
import {Tile} from '../source/tile';
import {Painter} from './painter';
import {Painter, RenderOptions} from './painter';
import type {Map} from '../ui/map';
import {drawCustom} from './draw_custom';
import {CustomStyleLayer} from '../style/style_layer/custom_style_layer';
Expand Down Expand Up @@ -60,7 +60,8 @@ describe('drawCustom', () => {
};
},
});
drawCustom(mockPainter, sourceCacheMock, mockLayer);
const renderOptions: RenderOptions = {isRenderingToTexture: false, isRenderingGlobe: false};
drawCustom(mockPainter, sourceCacheMock, mockLayer, renderOptions);
expect(result.gl).toBeDefined();
expect(result.args.farZ).toBeCloseTo(804.8028169246645, 6);
expect(result.args.farZ).toBe(mockPainter.transform.farZ);
Expand Down
7 changes: 4 additions & 3 deletions src/render/draw_custom.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
import {DepthMode} from '../gl/depth_mode';
import {StencilMode} from '../gl/stencil_mode';

import type {Painter} from './painter';
import type {Painter, RenderOptions} from './painter';
import type {SourceCache} from '../source/source_cache';
import type {CustomRenderMethodInput, CustomStyleLayer} from '../style/style_layer/custom_style_layer';

export function drawCustom(painter: Painter, sourceCache: SourceCache, layer: CustomStyleLayer) {
export function drawCustom(painter: Painter, sourceCache: SourceCache, layer: CustomStyleLayer, renderOptions: RenderOptions) {

const {isRenderingGlobe} = renderOptions;
const context = painter.context;
const implementation = layer.implementation;
const projection = painter.style.projection;
const transform = painter.transform;

const projectionData = transform.getProjectionDataForCustomLayer();
const projectionData = transform.getProjectionDataForCustomLayer(isRenderingGlobe);

const customLayerArgs: CustomRenderMethodInput = {
farZ: transform.farZ,
Expand Down
2 changes: 1 addition & 1 deletion src/render/draw_debug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ function drawDebugTile(painter: Painter, sourceCache: SourceCache, coord: Oversc
const tileLabel = `${tileIdText} ${tileSizeKb}kB`;
drawTextToOverlay(painter, tileLabel);

const projectionData = painter.transform.getProjectionData({overscaledTileID: coord});
const projectionData = painter.transform.getProjectionData({overscaledTileID: coord, applyGlobeMatrix: true, applyTerrainMatrix: true});

program.draw(context, gl.TRIANGLES, depthMode, stencilMode, ColorMode.alphaBlended, CullFaceMode.disabled,
debugUniformValues(Color.transparent, scaleRatio), null, projectionData, id,
Expand Down
5 changes: 3 additions & 2 deletions src/render/draw_fill.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {mat4} from 'gl-matrix';
import {OverscaledTileID} from '../source/tile_id';
import {SourceCache} from '../source/source_cache';
import {Tile} from '../source/tile';
import {Painter} from './painter';
import {Painter, RenderOptions} from './painter';
import {Program} from './program';
import type {ZoomHistory} from '../style/zoom_history';
import type {Map} from '../ui/map';
Expand Down Expand Up @@ -38,7 +38,8 @@ describe('drawFill', () => {
(sourceCacheMock.getTile as jest.Mock).mockReturnValue(mockTile);
sourceCacheMock.map = {showCollisionBoxes: false} as any as Map;

drawFill(painterMock, sourceCacheMock, layer, [mockTile.tileID]);
const renderOptions: RenderOptions = {isRenderingToTexture: false, isRenderingGlobe: false};
drawFill(painterMock, sourceCacheMock, layer, [mockTile.tileID], renderOptions);

// twice: first for fill, second for stroke
expect(programMock.draw).toHaveBeenCalledTimes(2);
Expand Down
18 changes: 10 additions & 8 deletions src/render/draw_fill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
fillOutlinePatternUniformValues
} from './program/fill_program';

import type {Painter} from './painter';
import type {Painter, RenderOptions} from './painter';
import type {SourceCache} from '../source/source_cache';
import type {FillStyleLayer} from '../style/style_layer/fill_style_layer';
import type {FillBucket} from '../data/bucket/fill_bucket';
Expand All @@ -18,14 +18,15 @@ import {updatePatternPositionsInProgram} from './update_pattern_positions_in_pro
import {StencilMode} from '../gl/stencil_mode';
import {translatePosition} from '../util/util';

export function drawFill(painter: Painter, sourceCache: SourceCache, layer: FillStyleLayer, coords: Array<OverscaledTileID>) {
export function drawFill(painter: Painter, sourceCache: SourceCache, layer: FillStyleLayer, coords: Array<OverscaledTileID>, renderOptions: RenderOptions) {
const color = layer.paint.get('fill-color');
const opacity = layer.paint.get('fill-opacity');

if (opacity.constantOr(1) === 0) {
return;
}

const {isRenderingToTexture} = renderOptions;
const colorMode = painter.colorModeForRenderPass();
const pattern = layer.paint.get('fill-pattern');
const pass = painter.opaquePassEnabledForLayer() &&
Expand All @@ -37,7 +38,7 @@ export function drawFill(painter: Painter, sourceCache: SourceCache, layer: Fill
if (painter.renderPass === pass) {
const depthMode = painter.getDepthModeForSublayer(
1, painter.renderPass === 'opaque' ? DepthMode.ReadWrite : DepthMode.ReadOnly);
drawFillTiles(painter, sourceCache, layer, coords, depthMode, colorMode, false);
drawFillTiles(painter, sourceCache, layer, coords, depthMode, colorMode, false, isRenderingToTexture);
}

// Draw stroke
Expand All @@ -53,7 +54,7 @@ export function drawFill(painter: Painter, sourceCache: SourceCache, layer: Fill
// the (non-antialiased) fill.
const depthMode = painter.getDepthModeForSublayer(
layer.getPaintProperty('fill-outline-color') ? 2 : 0, DepthMode.ReadOnly);
drawFillTiles(painter, sourceCache, layer, coords, depthMode, colorMode, true);
drawFillTiles(painter, sourceCache, layer, coords, depthMode, colorMode, true, isRenderingToTexture);
}
}

Expand All @@ -64,7 +65,8 @@ function drawFillTiles(
coords: Array<OverscaledTileID>,
depthMode: Readonly<DepthMode>,
colorMode: Readonly<ColorMode>,
isOutline: boolean) {
isOutline: boolean,
isRenderingToTexture: boolean) {
const gl = painter.context.gl;
const fillPropertyName = 'fill-pattern';
const patternProperty = layer.paint.get(fillPropertyName);
Expand Down Expand Up @@ -106,10 +108,10 @@ function drawFillTiles(

updatePatternPositionsInProgram(programConfiguration, fillPropertyName, constantPattern, tile, layer);

const globeWithTerrain = painter.style.map.terrain && painter.style.projection.name === 'globe';
const projectionData = transform.getProjectionData({
overscaledTileID: coord,
ignoreGlobeMatrix: globeWithTerrain
applyGlobeMatrix: !isRenderingToTexture,
applyTerrainMatrix: true
});

const translateForUniforms = translatePosition(transform, tile, propertyFillTranslate, propertyFillTranslateAnchor);
Expand Down Expand Up @@ -149,7 +151,7 @@ function drawFillTiles(
// greatly increasing subdivision granularity for both fill layers and stencil masks, at least at tile edges.
let stencil: StencilMode;
if (painter.renderPass === 'translucent') {
if (globeWithTerrain) {
if (isRenderingToTexture) {
const [stencilModes] = painter.stencilConfigForOverlap(coords);
stencil = stencilModes[coord.overscaledZ];
} else {
Expand Down
Loading

0 comments on commit 5de0876

Please sign in to comment.