Remove popup when marker is behind terrain (maplibre#3865)

* Avoid opening popup when marker is behind terrain

* Remove popup when marker goes behind terrain

* Rename Marker._opacity to _fullOpacity

* Wait in marker tests

* Check that popup is open before removing it

* Add test (fixes coverage)

* Rename Marker._fullOpacity to _opacity

* Add changelog

* Compare to _opacityWhenCovered when toggling a popup

* Add test

CHANGELOG.md

@@ -1,6 +1,7 @@
 ## main
 
 ### ✨ Features and improvements
+- Hide Popup when its parent Marker is behind terrain ([#3865](https://github.com/maplibre/maplibre-gl-js/pull/3865))
 - _...Add new stuff here..._
 
 ### 🐞 Bug fixes

src/ui/marker.test.ts

@@ -157,13 +157,15 @@ describe('marker', () => {
         expect(!marker.getPopup()).toBeTruthy();
     });
 
-    test('Marker#togglePopup opens a popup that was closed', () => {
+    test('Marker#togglePopup opens a popup that was closed', async () => {
         const map = createMap();
         const marker = new Marker()
             .setLngLat([0, 0])
             .addTo(map)
-            .setPopup(new Popup())
-            .togglePopup();
+            .setPopup(new Popup());
+
+        await sleep(500);
+        marker.togglePopup();
 
         expect(marker.getPopup().isOpen()).toBeTruthy();
 
@@ -184,13 +186,15 @@ describe('marker', () => {
         map.remove();
     });
 
-    test('Enter key on Marker opens a popup that was closed', () => {
+    test('Enter key on Marker opens a popup that was closed', async () => {
         const map = createMap();
         const marker = new Marker()
             .setLngLat([0, 0])
             .addTo(map)
             .setPopup(new Popup());
 
+        await sleep(500);
+
         // popup not initially open
         expect(marker.getPopup().isOpen()).toBeFalsy();
 
@@ -202,13 +206,15 @@ describe('marker', () => {
         map.remove();
     });
 
-    test('Space key on Marker opens a popup that was closed', () => {
+    test('Space key on Marker opens a popup that was closed', async () => {
         const map = createMap();
         const marker = new Marker()
             .setLngLat([0, 0])
             .addTo(map)
             .setPopup(new Popup());
 
+        await sleep(500);
+
         // popup not initially open
         expect(marker.getPopup().isOpen()).toBeFalsy();
 
@@ -292,14 +298,16 @@ describe('marker', () => {
         map.remove();
     });
 
-    test('Popup anchors around default Marker', () => {
+    test('Popup anchors around default Marker', async () => {
         const map = createMap();
 
         const marker = new Marker()
             .setLngLat([0, 0])
             .setPopup(new Popup().setText('Test'))
             .addTo(map);
 
+        await sleep(500);
+
         // open the popup
         marker.togglePopup();
 
@@ -361,14 +369,16 @@ describe('marker', () => {
         map.remove();
     });
 
-    test('Popup is opened at its marker position after marker is moved to another globe', () => {
+    test('Popup is opened at its marker position after marker is moved to another globe', async () => {
         const map = createMap({width: 3000});
 
         const marker = new Marker()
             .setLngLat([0, 0])
             .setPopup(new Popup().setText('Test'))
             .addTo(map);
 
+        await sleep(500);
+
         marker._pos = new Point(2999, 242);
         marker._lngLat = map.unproject(marker._pos);
         marker.togglePopup();
@@ -377,7 +387,7 @@ describe('marker', () => {
         map.remove();
     });
 
-    test('Popup is re-opened at its marker position after marker is moved to another globe', () => {
+    test('Popup is re-opened at its marker position after marker is moved to another globe', async () => {
         const map = createMap({width: 3000});
 
         const marker = new Marker()
@@ -387,6 +397,7 @@ describe('marker', () => {
             .togglePopup()
             .togglePopup();
 
+        await sleep(500);
         marker._pos = new Point(2999, 242);
         marker._lngLat = map.unproject(marker._pos);
         marker.togglePopup();
@@ -1029,6 +1040,56 @@ describe('marker', () => {
         map.remove();
     });
 
+    test('Removes an open popup when going behind 3d terrain', async () => {
+        const map = createMap();
+        const marker = new Marker()
+            .setLngLat([0, 0])
+            .addTo(map)
+            .setPopup(new Popup());
+
+        await sleep(500);
+        marker.togglePopup();
+
+        expect(marker._popup.isOpen()).toBeTruthy();
+
+        map.transform.lngLatToCameraDepth = () => .95; // Mocking distance to marker
+
+        map.terrain = {
+            getElevationForLngLatZoom: () => 0,
+            depthAtPoint: () => .92
+        } as any as Terrain;
+        map.fire('terrain');
+
+        await sleep(500);
+
+        expect(marker._popup?.isOpen()).toBeFalsy();
+        map.remove();
+    });
+
+    test('Does not open a popup when behind 3d terrain', async () => {
+        const map = createMap();
+        const marker = new Marker()
+            .setLngLat([0, 0])
+            .addTo(map)
+            .setPopup(new Popup());
+
+        map.transform.lngLatToCameraDepth = () => .95;
+
+        map.terrain = {
+            getElevationForLngLatZoom: () => 0,
+            depthAtPoint: () => .92
+        } as any as Terrain;
+        map.fire('terrain');
+
+        await sleep(500);
+
+        marker.togglePopup();
+
+        expect(marker._popup.isOpen()).toBeFalsy();
+
+        map.remove();
+    });
+
     test('Marker\'s lng is wrapped when slightly crossing 180 with {renderWorldCopies: false}', () => {
         const map = createMap({width: 1024, renderWorldCopies: false});
         const marker = new Marker()

src/ui/marker.ts

@@ -515,6 +515,8 @@ export class Marker extends Evented {
     togglePopup(): this {
         const popup = this._popup;
 
+        if (this._element.style.opacity === this._opacityWhenCovered) return this;
+
         if (!popup) return this;
         else if (popup.isOpen()) popup.remove();
         else {
@@ -559,6 +561,8 @@ export class Marker extends Evented {
         const markerDistanceCenter = map.transform.lngLatToCameraDepth(this._lngLat, elevation + elevationToCenter);
         // Display at full opacity if center is visible.
         const centerIsInvisible = markerDistanceCenter - terrainDistanceCenter > forgiveness;
+
+        if (this._popup?.isOpen() && centerIsInvisible) this._popup.remove();
         this._element.style.opacity = centerIsInvisible ? this._opacityWhenCovered : this._opacity;
     }