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

Improvements to marker transparency #3431

Merged
merged 25 commits into from
Dec 6, 2023
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
78abff5
Marker switching back to fully opaque when removing terrain
SnailBones Dec 1, 2023
bb3494c
Removing log
SnailBones Dec 1, 2023
2b91867
Fix lint
SnailBones Dec 1, 2023
ca079c4
Removing console logs
SnailBones Dec 1, 2023
a9db2dd
Marker visibility depending on depth buffer check, smooth marker tran…
SnailBones Dec 4, 2023
10d8430
Adding unit test
SnailBones Dec 4, 2023
7f7c877
Updating Changelog, fixing test
SnailBones Dec 4, 2023
618bbbc
merge
SnailBones Dec 4, 2023
887936c
Fix test
SnailBones Dec 4, 2023
c7ac0d5
Adding check for center of marker if base fails
SnailBones Dec 4, 2023
f843c26
Fix unit test
SnailBones Dec 4, 2023
57b0661
Removing console.log
SnailBones Dec 4, 2023
01ee6fa
Fix another unit test
SnailBones Dec 4, 2023
39488de
Clarifying clip space
SnailBones Dec 5, 2023
7b700e8
Phrasing: gl space to clip space
SnailBones Dec 5, 2023
a7f7710
Reverting change to near plane, adjusting forgiveness
SnailBones Dec 5, 2023
dafc2ec
Adding unit test for lngLatToCameraDepth
SnailBones Dec 5, 2023
8e765be
Increasing timeout to prevent flaking test
SnailBones Dec 5, 2023
1d7963d
Increasing timeout more
SnailBones Dec 5, 2023
1c98295
Increasing timeout for another flaky test
SnailBones Dec 5, 2023
6d13483
Reverting attempted fix to flaky tests
SnailBones Dec 6, 2023
de2d57f
Changing comments to tsdoc style
SnailBones Dec 6, 2023
c626026
Don't add opacity: 1 with no terrain
SnailBones Dec 6, 2023
b6ae897
Merge main
SnailBones Dec 6, 2023
55fd5f0
Improving marker test coverage
SnailBones Dec 6, 2023
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
## main

### ✨ Features and improvements

- Improved precision and added a subtle fade transition to marker opacity changes ([#3431](https://github.com/maplibre/maplibre-gl-js/pull/3431))
- _...Add new stuff here..._

### 🐞 Bug fixes

- Fix the shifted mouse events after a css transform scale on the map container ([#3437](https://github.com/maplibre/maplibre-gl-js/pull/3437))
- Fix markers remaining transparent when disabling terrain ([#3431](https://github.com/maplibre/maplibre-gl-js/pull/3431))
- _...Add new stuff here..._

## 4.0.0-pre.2
Expand Down Expand Up @@ -51,6 +54,7 @@
## 3.6.1

### 🐞 Bug fixes

- Fix `undefined` `_onEaseFrame` call in `Camera._renderFrameCallback()` while doing `Camera.jumpTo` during a `Camera.easeTo` ([#3332](https://github.com/maplibre/maplibre-gl-js/pull/3332))

## 3.6.0
Expand Down
1 change: 1 addition & 0 deletions src/css/maplibre-gl.css
Original file line number Diff line number Diff line change
Expand Up @@ -711,6 +711,7 @@ a.maplibregl-ctrl-logo.maplibregl-compact {
top: 0;
left: 0;
will-change: transform;
transition: opacity 0.2s;
}

.maplibregl-user-location-dot {
Expand Down
23 changes: 17 additions & 6 deletions src/geo/transform.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -395,10 +395,10 @@ describe('transform', () => {

expect(transform.customLayerMatrix()[0].toString().length).toBeGreaterThan(10);
expect(transform.glCoordMatrix[0].toString().length).toBeGreaterThan(10);
expect(transform.maxPitchScaleFactor()).toBeCloseTo(2.366025418080343, 10);
expect(transform.maxPitchScaleFactor()).toBeCloseTo(2.366025418080343, 5);
});

test('recalcuateZoom', () => {
test('recalculateZoom', () => {
const transform = new Transform(0, 22, 0, 60, true);
transform.elevation = 200;
transform.center = new LngLat(10.0, 50.0);
Expand All @@ -416,16 +416,16 @@ describe('transform', () => {
// expect new zoom because of elevation change
terrain.getElevationForLngLatZoom = () => 400;
transform.recalculateZoom(terrain as any);
expect(transform.zoom).toBe(14.127997275621933);
expect(transform.zoom).toBeCloseTo(14.127997275621933, 10);
expect(transform.elevation).toBe(400);

expect(transform._center.lng).toBe(10.00000000000071);
expect(transform._center.lat).toBe(50.00000000000017);
expect(transform._center.lng).toBeCloseTo(10, 10);
expect(transform._center.lat).toBeCloseTo(50, 10);

// expect new zoom because of elevation change to point below sea level
terrain.getElevationForLngLatZoom = () => -200;
transform.recalculateZoom(terrain as any);
expect(transform.zoom).toBe(13.773740316343467);
expect(transform.zoom).toBeCloseTo(13.773740316343467, 10);
expect(transform.elevation).toBe(-200);
});

Expand Down Expand Up @@ -461,4 +461,15 @@ describe('transform', () => {
expect(top).toBeCloseTo(79.1823898251593, 10);
expect(transform.getBounds().getNorthWest().toArray()).toStrictEqual(transform.pointLocation(new Point(0, top)).toArray());
});

test('lngLatToCameraDepth', () => {
const transform = new Transform(0, 22, 0, 85, true);
transform.resize(500, 500);
transform.center = new LngLat(10.0, 50.0);

expect(transform.lngLatToCameraDepth(new LngLat(10, 50), 4)).toBeCloseTo(0.9997324396231673);
transform.pitch = 60;
expect(transform.lngLatToCameraDepth(new LngLat(10, 50), 4)).toBeCloseTo(0.9865782165762236);

});
});
52 changes: 38 additions & 14 deletions src/geo/transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -458,13 +458,23 @@ export class Transform {
zoomScale(zoom: number) { return Math.pow(2, zoom); }
scaleZoom(scale: number) { return Math.log(scale) / Math.LN2; }

/**
* Convert from LngLat to world coordinates (Mercator coordinates scaled by 512)
* @param lngLat - the lngLat
* @returns Point
*/
project(lnglat: LngLat) {
const lat = clamp(lnglat.lat, -this.maxValidLatitude, this.maxValidLatitude);
return new Point(
mercatorXfromLng(lnglat.lng) * this.worldSize,
mercatorYfromLat(lat) * this.worldSize);
}

/**
* Convert from world coordinates ([0, 512],[0, 512]) to LngLat ([-180, 180], [-90, 90])
* @param point - world coordinate
* @returns LngLat
*/
unproject(point: Point): LngLat {
return new MercatorCoordinate(point.x / this.worldSize, point.y / this.worldSize).toLngLat();
}
Expand Down Expand Up @@ -527,7 +537,7 @@ export class Transform {
}

/**
* Given a location, return the screen point that corresponds to it
* Given a LngLat location, return the screen point that corresponds to it
* @param lnglat - location
* @param terrain - optional terrain
* @returns screen point
Expand All @@ -550,7 +560,7 @@ export class Transform {

/**
* Given a geographical lnglat, return an unrounded
* coordinate that represents it at this transform's zoom level.
* coordinate that represents it at low zoom level.
* @param lnglat - the location
* @returns The mercator coordinate
*/
Expand All @@ -560,7 +570,7 @@ export class Transform {

/**
* Given a Coordinate, return its geographical position.
* @param coord - mercator coordivates
* @param coord - mercator coordinates
* @returns lng and lat
*/
coordinateLocation(coord: MercatorCoordinate): LngLat {
Expand Down Expand Up @@ -588,8 +598,8 @@ export class Transform {
// unproject two points to get a line and then find the point on that
// line with z=0

const coord0 = [p.x, p.y, 0, 1] as any;
const coord1 = [p.x, p.y, 1, 1] as any;
const coord0 = [p.x, p.y, 0, 1] as vec4;
const coord1 = [p.x, p.y, 1, 1] as vec4;

vec4.transformMat4(coord0, coord0, this.pixelMatrixInverse);
vec4.transformMat4(coord1, coord1, this.pixelMatrixInverse);
Expand Down Expand Up @@ -618,7 +628,7 @@ export class Transform {
* @returns screen point
*/
coordinatePoint(coord: MercatorCoordinate, elevation: number = 0, pixelMatrix = this.pixelMatrix): Point {
const p = [coord.x * this.worldSize, coord.y * this.worldSize, elevation, 1] as any;
const p = [coord.x * this.worldSize, coord.y * this.worldSize, elevation, 1] as vec4;
vec4.transformMat4(p, p, pixelMatrix);
return new Point(p[0] / p[3], p[1] / p[3]);
}
Expand Down Expand Up @@ -833,12 +843,12 @@ export class Transform {
// - the more depth precision is available for features (good)
// - clipping starts appearing sooner when the camera is close to 3d features (bad)
//
// Smaller values worked well for mapbox-gl-js but deckgl was encountering precision issues
// when rendering it's layers using custom layers. This value was experimentally chosen and
// Other values work for mapbox-gl-js but deckgl was encountering precision issues
// when rendering custom layers. This value was experimentally chosen and
// seems to solve z-fighting issues in deckgl while not clipping buildings too close to the camera.
const nearZ = this.height / 50;

// matrix for conversion from location to GL coordinates (-1 .. 1)
// matrix for conversion from location to clip space(-1 .. 1)
m = new Float64Array(16) as any;
mat4.perspective(m, this._fov, this.width / this.height, nearZ, farZ);

Expand All @@ -853,21 +863,21 @@ export class Transform {
mat4.translate(m, m, [-x, -y, 0]);

// The mercatorMatrix can be used to transform points from mercator coordinates
// ([0, 0] nw, [1, 1] se) to GL coordinates.
// ([0, 0] nw, [1, 1] se) to clip space.
this.mercatorMatrix = mat4.scale([] as any, m, [this.worldSize, this.worldSize, this.worldSize]);

// scale vertically to meters per pixel (inverse of ground resolution):
mat4.scale(m, m, [1, 1, this._pixelPerMeter]);

// matrix for conversion from location to screen coordinates in 2D
// matrix for conversion from world space to screen coordinates in 2D
this.pixelMatrix = mat4.multiply(new Float64Array(16) as any, this.labelPlaneMatrix, m);

// matrix for conversion from location to GL coordinates (-1 .. 1)
// matrix for conversion from world space to clip space (-1 .. 1)
mat4.translate(m, m, [0, 0, -this.elevation]); // elevate camera over terrain
this.projMatrix = m;
this.invProjMatrix = mat4.invert([] as any, m);

// matrix for conversion from location to screen coordinates in 2D
// matrix for conversion from world space to screen coordinates in 3D
this.pixelMatrix3D = mat4.multiply(new Float64Array(16) as any, this.labelPlaneMatrix, m);

// Make a second projection matrix that is aligned to a pixel grid for rendering raster tiles.
Expand All @@ -884,7 +894,7 @@ export class Transform {
mat4.translate(alignedM, alignedM, [dx > 0.5 ? dx - 1 : dx, dy > 0.5 ? dy - 1 : dy, 0]);
this.alignedProjMatrix = alignedM;

// inverse matrix for conversion from screen coordinaes to location
// inverse matrix for conversion from screen coordinates to location
m = mat4.invert(new Float64Array(16) as any, this.pixelMatrix);
if (!m) throw new Error('failed to invert matrix');
this.pixelMatrixInverse = m;
Expand Down Expand Up @@ -955,4 +965,18 @@ export class Transform {
];
}
}
/**
* Return the distance to the camera in clip space from a LngLat.
* This can be compared to the value from the depth buffer (terrain.depthAtPoint)
* to determine whether a point is occluded.
* @param lngLat - the point
* @param elevation - the point's elevation
* @returns depth value in clip space (between 0 and 1)
*/
lngLatToCameraDepth(lngLat: LngLat, elevation: number) {
const coord = this.locationCoordinate(lngLat);
const p = [coord.x * this.worldSize, coord.y * this.worldSize, elevation, 1] as vec4;
HarelM marked this conversation as resolved.
Show resolved Hide resolved
vec4.transformMat4(p, p, this.projMatrix);
return (p[2] / p[3]);
}
}
2 changes: 1 addition & 1 deletion src/gl/framebuffer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export class Framebuffer {
if (hasDepth) {
this.depthAttachment = hasStencil ? new DepthStencilAttachment(context, fbo) : new DepthAttachment(context, fbo);
} else if (hasStencil) {
throw new Error('Stencil cannot be setted without depth');
throw new Error('Stencil cannot be set without depth');
}
if (gl.checkFramebufferStatus(gl.FRAMEBUFFER) !== gl.FRAMEBUFFER_COMPLETE) {
throw new Error('Framebuffer is not complete');
Expand Down
6 changes: 3 additions & 3 deletions src/render/terrain.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe('Terrain', () => {
jest.restoreAllMocks();
});

test('pointCoordiate should not return null', () => {
test('pointCoordinate should not return null', () => {
expect.assertions(1);
const painter = {
context: new Context(gl),
Expand Down Expand Up @@ -91,7 +91,7 @@ describe('Terrain', () => {
};

test(
`pointCoordiate should return negative mercator x
`pointCoordinate should return negative mercator x
if the point is on the LEFT outside the central globe`,
() => {
expect.assertions(1);
Expand All @@ -103,7 +103,7 @@ describe('Terrain', () => {
});

test(
`pointCoordiate should return mercator x greater than 1
`pointCoordinate should return mercator x greater than 1
if the point is on the RIGHT outside the central globe`,
() => {
expect.assertions(1);
Expand Down
19 changes: 18 additions & 1 deletion src/render/terrain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ export class Terrain {
*/
_coordsTexture: Texture;
/**
* accuracy of the coords. 2 * tileSize should be enoughth.
* accuracy of the coords. 2 * tileSize should be enough.
*/
_coordsTextureSize: number;
/**
Expand Down Expand Up @@ -348,6 +348,23 @@ export class Terrain {
);
}

/**
* Reads the depth value from the depth-framebuffer at a given screen pixel
* @param p - Screen coordinate
* @returns depth value in clip space (between 0 and 1)
*/

depthAtPoint(p: Point): number {
const rgba = new Uint8Array(4);
const context = this.painter.context, gl = context.gl;
context.bindFramebuffer.set(this.getFramebuffer('depth').framebuffer);
gl.readPixels(p.x, this.painter.height / devicePixelRatio - p.y - 1, 1, 1, gl.RGBA, gl.UNSIGNED_BYTE, rgba);
context.bindFramebuffer.set(null);
// decode coordinates (encoding see terran_depth.fragment.glsl)
const depthValue = (rgba[0] / (256 * 256 * 256) + rgba[1] / (256 * 256) + rgba[2] / 256 + rgba[3]) / 256;
return depthValue;
}

/**
* create a regular mesh which will be used by all terrain-tiles
* @returns the created regular mesh
Expand Down
2 changes: 1 addition & 1 deletion src/style/style_layer/custom_style_layer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {LayerSpecification} from '@maplibre/maplibre-gl-style-spec';
/**
* @param gl - The map's gl context.
* @param matrix - The map's camera matrix. It projects spherical mercator
* coordinates to gl coordinates. The spherical mercator coordinate `[0, 0]` represents the
* coordinates to gl clip space coordinates. The spherical mercator coordinate `[0, 0]` represents the
* top left corner of the mercator world and `[1, 1]` represents the bottom right corner. When
* the `renderingMode` is `"3d"`, the z coordinate is conformal. A box with identical x, y, and z
* lengths in mercator units would be rendered as a cube. {@link MercatorCoordinate.fromLngLat}
Expand Down
9 changes: 5 additions & 4 deletions src/symbol/projection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@ export {updateLineLabels, hideGlyphs, getLabelPlaneMatrix, getGlCoordMatrix, pro
* The points for both anchors and lines are stored in tile units. Each tile has it's own
* coordinate space going from (0, 0) at the top left to (EXTENT, EXTENT) at the bottom right.
*
* ## GL coordinate space
* At the end of everything, the vertex shader needs to produce a position in GL coordinate space,
* ## Clip space (GL coordinate space)
* At the end of everything, the vertex shader needs to produce a position in clip space,
* which is (-1, 1) at the top left and (1, -1) in the bottom right.
* In the depth buffer, values are between 0 (near plane) to 1 (far plane).
*
* ## Map pixel coordinate spaces
* Each tile has a pixel coordinate space. It's just the tile units scaled so that one unit is
Expand All @@ -51,7 +52,7 @@ export {updateLineLabels, hideGlyphs, getLabelPlaneMatrix, getGlCoordMatrix, pro
* - viewport pixel space pitch-alignment=viewport rotation-alignment=*
* 2. if the label follows a line, find the point along the line that is the correct distance from the anchor.
* 3. add the glyph's corner offset to the point from step 3
* 4. convert from the label coordinate space to gl coordinates
* 4. convert from the label coordinate space to clip space
*
* For horizontal labels we want to do step 1 in the shader for performance reasons (no cpu work).
* This is what `u_label_plane_matrix` is used for.
Expand Down Expand Up @@ -83,7 +84,7 @@ function getLabelPlaneMatrix(posMatrix: mat4,
}

/*
* Returns a matrix for converting from the correct label coordinate space to gl coords.
* Returns a matrix for converting from the correct label coordinate space to clip space.
*/
function getGlCoordMatrix(posMatrix: mat4,
pitchWithMap: boolean,
Expand Down
24 changes: 23 additions & 1 deletion src/ui/marker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -800,8 +800,10 @@ describe('marker', () => {
.setLngLat([0, 0])
.addTo(map);
map.terrain = {
getElevationForLngLatZoom: () => 0
getElevationForLngLatZoom: () => 0,
depthAtPoint: () => .9
} as any as Terrain;
map.transform.lngLatToCameraDepth = () => .95;

marker.setOffset([10, 10]);

Expand Down Expand Up @@ -836,4 +838,24 @@ describe('marker', () => {
expect(map._oneTimeListeners.render).toHaveLength(0);
map.remove();
});

test('Marker changes opacity behind terrain and when terrain is removed', () => {
const map = createMap();
map.terrain = {
getElevationForLngLatZoom: () => 0,
depthAtPoint: () => .9 // Mocking distance to terrain
} as any as Terrain;
map.transform.lngLatToCameraDepth = () => .95; // Mocking distance to marker
const marker = new Marker()
.setLngLat([0, 0])
.addTo(map);

expect(marker.getElement().style.opacity).toMatch('.2');

map.terrain = null;
map.fire('terrain');
expect(marker.getElement().style.opacity).toMatch('1');

map.remove();
});
});
Loading