fix(layers): hide control-rendered rasters with their group (#1720)

* fix(layers): hide control-rendered rasters with their group

Turning a group off left its COG layers painted on the map. The raster
control's store subscriber only reacted to `state.layers` and diffed each
layer's own `visible`/`opacity`, but group state lives in `layerGroups` and
never mutates a child layer, so the subscriber bailed out before it ran. A
deck.gl-rendered raster has no MapLibre style layer for layer-sync to toggle,
so that control push is its only channel and the raster stayed visible.

The subscriber now wakes on `layerGroups` changes and diffs the group-folded
values, so hiding or fading a group reaches the control. Project restore
replays the folded values too, so a project saved with a hidden group no
longer reopens with its rasters on the map.

Because the control now holds folded values, the control -> store mirror
would burn a hidden group's `false` into the layer's own `visible` on the
next control event, leaving it hidden after the group is shown again. It now
ignores a reported value that matches the fold and only records one that
differs, which is a genuine control-side edit.

Adds `effectiveLayerRenderState`, the single-layer form of
`applyGroupEffects`, so the fold (including the nested-group walk and its
cycle guard) has one definition.

Fixes #1717

* Address Claude review feedback; grey the eye of a group-hidden layer

- raster-layer-sync: the echo guard keyed on a re-derived group fold, so it
  decided "this is an echo" from a value coincidence rather than from
  provenance. It now records the folded value this module actually pushed
  (subscriber and project-restore replay both register) and suppresses only
  that, so any other control-reported value is treated as a user edit. Records
  are dropped when the control loses the raster and cleared on unwire.
- LayerPanel: a layer hidden by its group kept a full-strength eye icon while
  its name was already greyed, so the row contradicted itself. The eye is now
  muted with the name and its tooltip says why. It deliberately stays an Eye
  rather than flipping to EyeOff: the button is the layer's *own* switch, and
  an EyeOff would offer a "Show layer" that turns that switch off, making the
  layer take two clicks to reveal once the group is shown.
- LayerPanel: that cue read only the immediate parent, so a hidden grandparent
  left the row looking fully visible. It now folds through
  effectiveLayerRenderState and covers nested groups.

* Address Claude review feedback (round 2)

- raster-layer-sync: the echo guard remembered only values this module pushed,
  never ones it accepted, so a control-side toggle away from the pushed value
  and back landed on that value again and was misread as an echo and dropped —
  the raster would then reappear when the group was shown. The map now tracks
  the last value the control is known to hold, updated on accepted edits as
  well as pushes, and is renamed controlRenderState to say so. Adds the
  toggle-away-and-back regression test, which the round-1 tests missed.
- effectiveLayerRenderState now also takes a prebuilt id -> group map, and
  LayerPanel passes its memoized groupById, so folding every row no longer
  rebuilds that map once per layer on each render.

* Address Claude review feedback (round 3)

- maplibre-raster: build the restore-path group lookup as a Map once instead
  of handing effectiveLayerRenderState the array inside the loop, which
  rebuilt it per grouped raster — the pattern that helper's own JSDoc warns
  against.
- LayerPanel: the visibility button's tooltip explained why the eye was muted
  but its aria-label did not, so the group-hidden context reached a hover and
  not a screen reader. Both now render the same string.

Author: giswqs

apps/geolibre-desktop/src/components/panels/LayerPanel.tsx

@@ -16,6 +16,7 @@ import type { ParseKeys, TFunction } from "i18next";
 import {
   NETCDF_IMAGE_SOURCE_KIND,
   DEFAULT_BASEMAP,
+  effectiveLayerRenderState,
   getPlanetaryBasemapById,
   getPlanetaryBasemapByStyleUrl,
   isDuckDBQueryLayer,
@@ -2950,12 +2951,22 @@ export function LayerPanel({
             const isFirstOfGroup = group ? firstMemberIdByGroup.get(group.id) === layer.id : false;
             const groupCollapsed = group?.collapsed ?? false;
             const groupAncestorCollapsed = group ? hasCollapsedAncestor(group) : false;
-            // When the parent group is hidden, a layer whose own visibility
+            // When an ancestor group is hidden, a layer whose own visibility
             // toggle is still on is not rendered — a surprising state. Grey its
-            // name out as a cue that the group-level setting is what's hiding
-            // it (issue #430). If the layer's own toggle is also off, the
-            // EyeOff icon already explains it, so skip the group cue then.
-            const groupHidden = group ? !group.visible && layer.visible : false;
+            // name and eye out as a cue that the group-level setting is what's
+            // hiding it (issue #430). If the layer's own toggle is also off,
+            // the EyeOff icon already explains it, so skip the group cue then.
+            // Folded through effectiveLayerRenderState rather than read off the
+            // immediate parent, so a hidden grandparent gets the cue too. Given
+            // the memoized `groupById` rather than the array, so folding every
+            // row does not rebuild that map once per layer.
+            const groupHidden =
+              layer.visible && !effectiveLayerRenderState(layer, groupById).visible;
+            const visibilityToggleLabel = groupHidden
+              ? `${t("layers.hiddenByGroup")} — ${t("layers.hideLayer")}`
+              : layer.visible
+                ? t("layers.hideLayer")
+                : t("layers.showLayer");
             const canIdentify =
               layer.type === "geojson" ||
               isDuckDBQueryLayer(layer) ||
@@ -3172,15 +3183,25 @@ export function LayerPanel({
                       <button
                         type="button"
                         className="rounded p-0.5 hover:bg-muted"
-                        title={layer.visible ? t("layers.hideLayer") : t("layers.showLayer")}
-                        aria-label={layer.visible ? t("layers.hideLayer") : t("layers.showLayer")}
+                        // The eye stays the layer's *own* switch even while a
+                        // group hides it — showing EyeOff here would offer a
+                        // "Show layer" that turns the layer's own toggle off,
+                        // so revealing it later would take two clicks. The
+                        // muted icon plus the tooltip say why it is not drawn.
+                        // Same string for the tooltip and the accessible name,
+                        // so the group-hidden context reaches a screen reader
+                        // and not only a sighted hover.
+                        title={visibilityToggleLabel}
+                        aria-label={visibilityToggleLabel}
                         onClick={(e) => {
                           e.stopPropagation();
                           setLayerVisibility(layer.id, !layer.visible);
                         }}
                       >
                         {layer.visible ? (
-                          <Eye className="h-3.5 w-3.5" />
+                          <Eye
+                            className={`h-3.5 w-3.5 ${groupHidden ? "text-muted-foreground" : ""}`}
+                          />
                         ) : (
                           <EyeOff className="h-3.5 w-3.5 text-muted-foreground" />
                         )}

packages/core/src/layer-groups.ts

@@ -92,23 +92,67 @@ export function applyGroupEffects(layers: GeoLibreLayer[], groups: LayerGroup[])
   if (groups.length === 0) return layers;
   const groupById = new Map(groups.map((g) => [g.id, g]));
   return layers.map((layer) => {
-    if (!layer.groupId) return layer;
-    let group = groupById.get(layer.groupId);
-    if (!group) return layer;
-    let visible = layer.visible;
-    let opacity = layer.opacity;
-    const visited = new Set<string>();
-    while (group && !visited.has(group.id)) {
-      visited.add(group.id);
-      visible = visible && group.visible;
-      opacity *= group.opacity;
-      group = group.parentId ? groupById.get(group.parentId) : undefined;
-    }
+    if (!layer.groupId || !groupById.has(layer.groupId)) return layer;
+    const { visible, opacity } = foldGroupChain(layer, groupById);
     if (visible === layer.visible && opacity === layer.opacity) return layer;
     return { ...layer, visible, opacity };
   });
 }
 
+/**
+ * Walk a layer's group chain upward, ANDing each group's visibility into the
+ * layer's own and multiplying each group's opacity into it. The `visited` set
+ * makes a corrupted `parentId` cycle terminate instead of hanging.
+ */
+function foldGroupChain(
+  layer: GeoLibreLayer,
+  groupById: ReadonlyMap<string, LayerGroup>,
+): { visible: boolean; opacity: number } {
+  let visible = layer.visible;
+  let opacity = layer.opacity;
+  let group = layer.groupId ? groupById.get(layer.groupId) : undefined;
+  const visited = new Set<string>();
+  while (group && !visited.has(group.id)) {
+    visited.add(group.id);
+    visible = visible && group.visible;
+    opacity *= group.opacity;
+    group = group.parentId ? groupById.get(group.parentId) : undefined;
+  }
+  return { visible, opacity };
+}
+
+/**
+ * The single layer form of {@link applyGroupEffects}: the visibility and
+ * opacity a layer should actually render at once its (possibly nested) parent
+ * groups are folded in.
+ *
+ * Group state never mutates a child's own `visible`/`opacity` fields, so any
+ * renderer that cannot read the folded array — a plugin control that owns its
+ * own paint and is driven layer by layer — asks for a single layer here
+ * instead.
+ *
+ * @param layer The layer to fold.
+ * @param groups Group definitions, or an already-built id → group map. Callers
+ *   in a render path that fold many layers against the same groups (the layer
+ *   panel) should pass a memoized map, since the array form rebuilds one per
+ *   call.
+ * @returns The effective render state (the layer's own values when it is
+ *   ungrouped or its `groupId` is dangling).
+ */
+export function effectiveLayerRenderState(
+  layer: GeoLibreLayer,
+  groups: LayerGroup[] | ReadonlyMap<string, LayerGroup>,
+): { visible: boolean; opacity: number } {
+  const size = Array.isArray(groups) ? groups.length : groups.size;
+  if (size === 0 || !layer.groupId) {
+    return { visible: layer.visible, opacity: layer.opacity };
+  }
+  return foldGroupChain(
+    layer,
+    Array.isArray(groups) ? new Map(groups.map((g) => [g.id, g])) : groups,
+  );
+}
+
 /**
  * Indices, in store order, of every layer that belongs to `groupId`.
  *

packages/plugins/src/plugins/maplibre-raster.ts

@@ -1,4 +1,4 @@
-import { styleValue, useAppStore } from "@geolibre/core";
+import { effectiveLayerRenderState, styleValue, useAppStore } from "@geolibre/core";
 import type { Layer } from "@deck.gl/core";
 import type {
   RasterControl,
@@ -17,6 +17,7 @@ import {
 import {
   isRasterControlStoreLayer,
   rememberLocalRasterPath,
+  rememberControlRasterRenderState,
   rendersNativeMapLibreLayer,
   resetRasterStoreSyncSuspension,
   runWithRasterStoreSyncSuspended,
@@ -565,6 +566,11 @@ export function restoreRasterLayers(app: GeoLibreAppAPI): void {
         if (!storeLayerIds.has(info.id)) control.removeRaster(info.id);
       }
 
+      // Built once here rather than handed to effectiveLayerRenderState as an
+      // array, which would rebuild it for every grouped raster in the loop.
+      const restoredGroups = new Map(
+        useAppStore.getState().layerGroups.map((group) => [group.id, group] as const),
+      );
       for (const layer of useAppStore.getState().layers) {
         if (!isRasterControlStoreLayer(layer)) continue;
         if (control.getRaster(layer.id)) continue;
@@ -587,15 +593,23 @@ export function restoreRasterLayers(app: GeoLibreAppAPI): void {
           continue;
         }
 
+        // A parent group's visibility/opacity never touch the child layer's
+        // own fields, so replay the folded values — otherwise a project saved
+        // with a hidden group reopens with its rasters painted on the map.
+        // Recorded so the first control event after the restore reads them as
+        // this replay's echo rather than as a control-side edit.
+        const effective = effectiveLayerRenderState(layer, restoredGroups);
+        rememberControlRasterRenderState(layer.id, effective);
+
         pending.push(
           control
             .addRaster(source, {
               id: layer.id,
               name: layer.name,
               state: {
                 ...savedRasterState(layer),
-                opacity: layer.opacity,
-                visible: layer.visible,
+                opacity: effective.opacity,
+                visible: effective.visible,
                 // The zoom range lives on layer.style (the shared Style-panel
                 // control), not in metadata.rasterState, so it is replayed here
                 // to survive a project reload / map reinitialisation.

packages/plugins/src/plugins/raster-layer-sync.ts

@@ -1,4 +1,10 @@
-import { DEFAULT_LAYER_STYLE, type GeoLibreLayer, styleValue, useAppStore } from "@geolibre/core";
+import {
+  applyGroupEffects,
+  DEFAULT_LAYER_STYLE,
+  type GeoLibreLayer,
+  styleValue,
+  useAppStore,
+} from "@geolibre/core";
 import type { RasterLayerInfo, RasterLayerState, RenderEngine } from "maplibre-gl-raster";
 
 export const RASTER_SOURCE_KIND = "maplibre-gl-raster";
@@ -82,6 +88,35 @@ let syncingLayersToStore = false;
 // control emits raster* events synchronously from those calls, and syncing
 // mid-mutation would observe a partially updated layer list.
 let storeSyncSuspended = 0;
+// The last visibility/opacity this module knows each raster to hold in the
+// control, by layer id -- whether it got there by being pushed (a group fold,
+// which never lives on the child layer and so reaches a deck.gl raster only
+// through the control) or by a control-side edit this module accepted. The
+// control reports its state back on every later event, and comparing against
+// this is what lets syncRasterLayersToStoreWithOptions tell that echo from a
+// real edit. It must track *accepted* values too, not just pushed ones: with
+// only the pushed value recorded, a user toggling a control's checkbox away
+// and back would land on the pushed value again and have that second edit
+// misread as an echo and dropped.
+const controlRenderState = new Map<string, { visible?: boolean; opacity?: number }>();
+
+/**
+ * Record what the control now holds for a raster, so the next control->store
+ * mirror can recognize it as an echo rather than a user edit. Called wherever
+ * this module forces a group-folded value on the control -- its own store
+ * subscriber and the project-restore replay in maplibre-raster -- and again
+ * whenever the mirror accepts a control-side edit.
+ *
+ * @param id - The raster/store layer id.
+ * @param patch - The fields the control now holds.
+ */
+export function rememberControlRasterRenderState(
+  id: string,
+  patch: { visible?: boolean; opacity?: number },
+): void {
+  const existing = controlRenderState.get(id);
+  controlRenderState.set(id, existing ? { ...existing, ...patch } : { ...patch });
+}
 
 /**
  * Detects a layer panel entry owned by the maplibre-gl-raster control.
@@ -241,6 +276,12 @@ export function syncRasterLayersToStoreWithOptions(
   const infoIds = new Set(infos.map((info) => info.id));
   const panelCollapsed = rasterPanelCollapsedFromControl(control);
 
+  // A raster the control no longer holds can never echo again, and its id
+  // could otherwise mis-suppress a future raster added under the same id.
+  for (const id of controlRenderState.keys()) {
+    if (!infoIds.has(id)) controlRenderState.delete(id);
+  }
+
   syncingLayersToStore = true;
   try {
     for (const storeLayer of useAppStore.getState().layers) {
@@ -272,9 +313,28 @@ export function syncRasterLayersToStoreWithOptions(
       const metadata =
         Object.keys(preserved).length > 0 ? { ...layer.metadata, ...preserved } : layer.metadata;
 
+      // A control still reporting the value this module last knows it to hold
+      // is echoing that value, not recording a user edit. Mirroring the echo
+      // would burn a hidden group's `false` into the layer's own `visible`,
+      // leaving it hidden after the group is shown again. Anything else is a
+      // genuine control-side change: take it, and remember it so the control's
+      // next report is compared against the value it actually holds now.
+      const known = controlRenderState.get(layer.id);
+      const visibleIsEcho = known?.visible !== undefined && layer.visible === known.visible;
+      const opacityIsEcho =
+        known?.opacity !== undefined && numbersEqual(layer.opacity, known.opacity);
+      const visible = visibleIsEcho ? existing.visible : layer.visible;
+      const opacity = opacityIsEcho ? existing.opacity : layer.opacity;
+      if (!visibleIsEcho || !opacityIsEcho) {
+        rememberControlRasterRenderState(layer.id, {
+          ...(visibleIsEcho ? {} : { visible: layer.visible }),
+          ...(opacityIsEcho ? {} : { opacity: layer.opacity }),
+        });
+      }
+
       if (
-        existing.visible !== layer.visible ||
-        existing.opacity !== layer.opacity ||
+        existing.visible !== visible ||
+        existing.opacity !== opacity ||
         existing.sourcePath !== layer.sourcePath ||
         !recordsEqual(existing.source, layer.source) ||
         !recordsEqual(existing.metadata, metadata)
@@ -283,10 +343,10 @@ export function syncRasterLayersToStoreWithOptions(
           // Replace metadata wholesale so stale keys (error, bounds) cannot
           // survive a raster being swapped out under the same id.
           metadata,
-          opacity: layer.opacity,
+          opacity,
           source: layer.source,
           sourcePath: layer.sourcePath,
-          visible: layer.visible,
+          visible,
         });
       }
     }
@@ -314,7 +374,7 @@ export function wireRasterStoreSync(control: RasterSyncableControl): void {
       !activeControl ||
       syncingLayersToStore ||
       isRasterStoreSyncSuspended() ||
-      state.layers === previous.layers
+      (state.layers === previous.layers && state.layerGroups === previous.layerGroups)
     ) {
       return;
     }
@@ -323,22 +383,33 @@ export function wireRasterStoreSync(control: RasterSyncableControl): void {
     // when the previous snapshot held no control-managed rasters at all.
     if (!previous.layers.some(isRasterControlStoreLayer)) return;
 
-    const currentById = new Map(state.layers.map((layer) => [layer.id, layer]));
+    // Diff the *group-folded* visibility and opacity, not the layers' own
+    // fields: hiding or fading a parent group never touches a child layer, so
+    // watching `layer.visible` alone leaves a grouped raster on the map
+    // (GeoLibre#1717). A deck.gl-rendered raster has no MapLibre style layer
+    // for layer-sync to toggle, so this control push is its only channel.
+    const currentById = new Map(
+      applyGroupEffects(state.layers, state.layerGroups).map((layer) => [layer.id, layer]),
+    );
+    const previousLayers = applyGroupEffects(previous.layers, previous.layerGroups);
     runWithRasterStoreSyncSuspended(() => {
-      for (const layer of previous.layers) {
+      for (const layer of previousLayers) {
         if (!isRasterControlStoreLayer(layer)) continue;
 
         const current = currentById.get(layer.id);
         if (!current) {
           activeControl.removeRaster(layer.id);
+          controlRenderState.delete(layer.id);
           continue;
         }
 
         if (current.visible !== layer.visible) {
           activeControl.setVisible(layer.id, current.visible);
+          rememberControlRasterRenderState(layer.id, { visible: current.visible });
         }
         if (current.opacity !== layer.opacity) {
           activeControl.setRasterState(layer.id, { opacity: current.opacity });
+          rememberControlRasterRenderState(layer.id, { opacity: current.opacity });
         }
         const patch = rasterStatePatch(layer, current);
         if (patch) activeControl.setRasterState(layer.id, patch);
@@ -435,6 +506,9 @@ export function unwireRasterStoreSync(): void {
   storeUnsubscribe?.();
   storeUnsubscribe = null;
   syncedControl = null;
+  // The successor control has been told nothing, so no echo of this one's
+  // pushes can arrive; a stale record would only mis-suppress its first sync.
+  controlRenderState.clear();
 }
 
 /**
@@ -616,6 +690,14 @@ function isRecord(value: unknown): value is Record<string, unknown> {
   return typeof value === "object" && value !== null && !Array.isArray(value);
 }
 
+// Opacity round-trips through the control as a float that a group fold has
+// multiplied, so compare the echo with a tolerance rather than by identity —
+// a last-bit difference would read as a user edit and overwrite the layer's
+// own opacity with the folded one.
+function numbersEqual(left: number, right: number): boolean {
+  return Math.abs(left - right) < 1e-9;
+}
+
 function serializableRasterState(state: RasterLayerState): Record<string, unknown> {
   // visible and opacity live on the top-level layer fields (the panel edits
   // them there); persisting copies here would leave two competing values in