diff --git a/CHANGELOG.md b/CHANGELOG.md index 06bf845b82..3ae119e83c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 @@ -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 diff --git a/src/css/maplibre-gl.css b/src/css/maplibre-gl.css index ccd87a9d62..55bbc3c6b3 100644 --- a/src/css/maplibre-gl.css +++ b/src/css/maplibre-gl.css @@ -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 { diff --git a/src/geo/transform.test.ts b/src/geo/transform.test.ts index 065a0d4d7f..84edcf4fc4 100644 --- a/src/geo/transform.test.ts +++ b/src/geo/transform.test.ts @@ -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); @@ -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); }); @@ -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); + + }); }); diff --git a/src/geo/transform.ts b/src/geo/transform.ts index b2f8ad9665..180e83cb02 100644 --- a/src/geo/transform.ts +++ b/src/geo/transform.ts @@ -458,6 +458,11 @@ 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( @@ -465,6 +470,11 @@ export class Transform { 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(); } @@ -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 @@ -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 */ @@ -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 { @@ -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); @@ -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]); } @@ -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); @@ -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. @@ -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; @@ -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; + vec4.transformMat4(p, p, this.projMatrix); + return (p[2] / p[3]); + } } diff --git a/src/gl/framebuffer.ts b/src/gl/framebuffer.ts index 384d48b99f..7f935a1fea 100644 --- a/src/gl/framebuffer.ts +++ b/src/gl/framebuffer.ts @@ -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'); diff --git a/src/render/terrain.test.ts b/src/render/terrain.test.ts index 30c0ae2403..69bca347ca 100644 --- a/src/render/terrain.test.ts +++ b/src/render/terrain.test.ts @@ -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), @@ -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); @@ -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); diff --git a/src/render/terrain.ts b/src/render/terrain.ts index 248f82f712..f6d7d0f010 100644 --- a/src/render/terrain.ts +++ b/src/render/terrain.ts @@ -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; /** @@ -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 diff --git a/src/style/style_layer/custom_style_layer.ts b/src/style/style_layer/custom_style_layer.ts index 88a58b6b24..cb539c3f7a 100644 --- a/src/style/style_layer/custom_style_layer.ts +++ b/src/style/style_layer/custom_style_layer.ts @@ -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} diff --git a/src/symbol/projection.ts b/src/symbol/projection.ts index ac186a7c73..b613ccaaf2 100644 --- a/src/symbol/projection.ts +++ b/src/symbol/projection.ts @@ -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 @@ -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. @@ -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, diff --git a/src/ui/marker.test.ts b/src/ui/marker.test.ts index 92fa686fe9..e2fd0d84a7 100644 --- a/src/ui/marker.test.ts +++ b/src/ui/marker.test.ts @@ -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]); @@ -836,4 +838,36 @@ 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.transform.lngLatToCameraDepth = () => .95; // Mocking distance to marker + const marker = new Marker() + .setLngLat([0, 0]) + .addTo(map); + + expect(marker.getElement().style.opacity).toMatch(''); + + // Add terrain, not blocking marker + map.terrain = { + getElevationForLngLatZoom: () => 0, + depthAtPoint: () => .95 // Mocking distance to terrain + } as any as Terrain; + map.fire('terrain'); + + expect(marker.getElement().style.opacity).toMatch('1'); + + // Terrain blocks marker + map.terrain.depthAtPoint = () => .92; // Mocking terrain blocking marker + map.fire('moveend'); + + expect(marker.getElement().style.opacity).toMatch('.2'); + + // Remove terrain + map.terrain = null; + map.fire('terrain'); + expect(marker.getElement().style.opacity).toMatch('1'); + + map.remove(); + }); }); diff --git a/src/ui/marker.ts b/src/ui/marker.ts index e5c6908a31..fddb39555d 100644 --- a/src/ui/marker.ts +++ b/src/ui/marker.ts @@ -506,6 +506,43 @@ export class Marker extends Evented { return this; } + _updateOpacity(force: boolean = false) { + const terrain = this._map.terrain; + if (!terrain) { + if (this._element.style.opacity === '0.2') { this._element.style.opacity = '1'; } + return; + } + if (force) { + this._opacityTimeout = null; + } else { + if (this._opacityTimeout) { return; } + this._opacityTimeout = setTimeout(() => { + this._opacityTimeout = null; + }, 100); + } + + const map = this._map; + + // Read depth framebuffer, getting position of terrain in line of sight to marker + const terrainDistance = map.terrain.depthAtPoint(this._pos); + // Transform marker position to clip space + const elevation = map.terrain.getElevationForLngLatZoom(this._lngLat, map.transform.tileZoom); + const markerDistance = map.transform.lngLatToCameraDepth(this._lngLat, elevation); + + const forgiveness = .006; + if (markerDistance - terrainDistance < forgiveness) { + this._element.style.opacity = '1'; + return; + } + // If the base is obscured, use the offset to check if the marker's center is obscured. + const metersToCenter = -this._offset.y / map.transform._pixelPerMeter; + const elevationToCenter = Math.sin(map.getPitch() * Math.PI / 180) * metersToCenter; + const terrainDistanceCenter = map.terrain.depthAtPoint(new Point(this._pos.x, this._pos.y - this._offset.y)); + const markerDistanceCenter = map.transform.lngLatToCameraDepth(this._lngLat, elevation + elevationToCenter); + // Display at full opacity if center is visible. + this._element.style.opacity = (markerDistanceCenter - terrainDistanceCenter > forgiveness) ? '0.2' : '1.0'; + } + _update = (e?: { type: 'move' | 'moveend' | 'terrain' | 'render' }) => { if (!this._map) return; @@ -542,15 +579,7 @@ export class Marker extends Evented { } DOM.setTransform(this._element, `${anchorTranslate[this._anchor]} translate(${this._pos.x}px, ${this._pos.y}px) ${pitch} ${rotation}`); - - // in case of 3D, ask the terrain coords-framebuffer for this pos and check if the marker is visible - // call this logic in setTimeout with a timeout of 100ms to save performance in map-movement - if (this._map.terrain && !this._opacityTimeout) this._opacityTimeout = setTimeout(() => { - const lnglat = this._map.unproject(this._pos); - const metresPerPixel = 40075016.686 * Math.abs(Math.cos(this._lngLat.lat * Math.PI / 180)) / Math.pow(2, this._map.transform.tileZoom + 8); - this._element.style.opacity = lnglat.distanceTo(this._lngLat) > metresPerPixel * 20 ? '0.2' : '1.0'; - this._opacityTimeout = null; - }, 100); + this._updateOpacity(e && e.type === 'moveend'); }; /**