(skywalking-horizon-ui) 01/01: fix(infra-3d): GPU geometry leak, metrics chunk cap, config-load failure state

[wu...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

wu-sheng pushed a commit to branch fix/infra-3d-review
in repository https://gitbox.apache.org/repos/asf/skywalking-horizon-ui.git

commit 2317e3a59a36522d7cb07b373bb79e2f116bf5a0
Author: Wu Sheng <wu...@foxmail.com>
AuthorDate: Thu May 28 18:42:39 2026 +0800

    fix(infra-3d): GPU geometry leak, metrics chunk cap, config-load failure state
    
    Address review findings on the 3D Infra Map:
    
    - HIGH — TubeGeometry leak. callTubes / crossTubes / verticalTubes /
      hierarchyTubes mint a fresh TubeGeometry per edge on every recompute
      (layer-visibility toggle, selection change), but only the current
      batch was freed on unmount. Dispose the previous batch when each set
      changes (flush:'post', after the new geometries have rendered).
    
    - MED — metricChunkSize cap mismatch. Admin/validation allowed up to 50
      while the metrics route rejects >12 (OAP complexity ceiling), so any
      oversized chunk 5xx'd. Cap config validation + the admin input at 12.
    
    - MED — config-load failure state. /3d/map's ensureLoaded() had no
      try/catch, so a reject (OAP offline, or a role without infra-3d:read)
      hung on "Loading 3D map configuration…" forever. Catch it and show a
      reason + access hint + Back link instead.
    
    - LOW — remove orphaned .plane-label CSS and a stale "label rendering"
      prop comment (no plane-label render path exists).
    
    Validated: UI + BFF type-check, UI build, eslint clean on touched files.
---
 apps/bff/src/logic/infra-3d/validate.ts            |  6 ++-
 .../features/admin/infra-3d/Infra3dAdminView.vue   |  4 +-
 apps/ui/src/features/infra-3d/Infra3DScene.vue     | 38 ++++++++-----------
 apps/ui/src/features/infra-3d/Infra3DView.vue      | 44 +++++++++++++++++++---
 4 files changed, 63 insertions(+), 29 deletions(-)

diff --git a/apps/bff/src/logic/infra-3d/validate.ts b/apps/bff/src/logic/infra-3d/validate.ts
index 5a3d7dc..4ddb1bf 100644
--- a/apps/bff/src/logic/infra-3d/validate.ts
+++ b/apps/bff/src/logic/infra-3d/validate.ts
@@ -90,7 +90,11 @@ const configSchema = z
       .strict(),
     pipeline: z
       .object({
-        metricChunkSize: z.number().int().min(1).max(50),
+        // Cap matches the metrics route's MAX_SERVICES (infra-3d-metrics.ts):
+        // each metric chunk is one GraphQL request, and OAP's complexity
+        // ceiling 5xx's beyond 12 services. A larger chunk size makes every
+        // oversized request fail, so reject it at config-save time.
+        metricChunkSize: z.number().int().min(1).max(12),
         topologyConcurrency: z.number().int().min(1).max(16),
         templateConcurrency: z.number().int().min(1).max(32),
       })
diff --git a/apps/ui/src/features/admin/infra-3d/Infra3dAdminView.vue b/apps/ui/src/features/admin/infra-3d/Infra3dAdminView.vue
index 79871bd..d32f1f9 100644
--- a/apps/ui/src/features/admin/infra-3d/Infra3dAdminView.vue
+++ b/apps/ui/src/features/admin/infra-3d/Infra3dAdminView.vue
@@ -726,7 +726,9 @@ function edgeRef(key: 'hierarchy' | 'crossLevelCall' | 'intraCall'): InfraEdgeSt
           <div class="sect-body">
             <label class="field">
               <span class="lbl">metricChunkSize</span>
-              <input type="number" class="inp" v-model.number="draft.pipeline.metricChunkSize" min="1" max="50" />
+              <!-- Capped at 12: the BFF metrics route (MAX_SERVICES) rejects
+                   larger chunks — OAP's GraphQL complexity ceiling 5xx's. -->
+              <input type="number" class="inp" v-model.number="draft.pipeline.metricChunkSize" min="1" max="12" />
             </label>
             <label class="field">
               <span class="lbl">topologyConcurrency</span>
diff --git a/apps/ui/src/features/infra-3d/Infra3DScene.vue b/apps/ui/src/features/infra-3d/Infra3DScene.vue
index 154088d..bdacde9 100644
--- a/apps/ui/src/features/infra-3d/Infra3DScene.vue
+++ b/apps/ui/src/features/infra-3d/Infra3DScene.vue
@@ -76,8 +76,8 @@ import { useInfra3dMetrics, formatMetricValue } from './composables/useInfra3dMe
 
 interface Props {
   /** Ordered (top-down) list of planes from the admin config. Drives
-   *  vertical stacking AND label rendering; passed as a prop so the
-   *  Scene stays a pure render of whatever the parent resolved. */
+   *  vertical stacking; passed as a prop so the Scene stays a pure
+   *  render of whatever the parent resolved. */
   planeOrder: PlaneSpec[];
   visibleLayers: Set<string>;
   hoveredNodeId: string | null;
@@ -603,6 +603,20 @@ const hierarchyTubes = computed(() =>
   })),
 );
 
+// Each tube computed mints a fresh TubeGeometry per edge on every
+// recompute (a layer visibility toggle or selection change re-runs the
+// `visible*Edges` deps). The old GPU buffers are otherwise abandoned —
+// only the *current* batch was freed, on unmount. Free the previous
+// batch when the set changes; flush:'post' so the new geometries have
+// already rendered and the old meshes are detached before we dispose.
+function disposeTubeBatch(batch: ReadonlyArray<{ geometry: TubeGeometry }>): void {
+  for (const t of batch) t.geometry.dispose();
+}
+watch(callTubes, (_now, prev) => disposeTubeBatch(prev), { flush: 'post' });
+watch(crossTubes, (_now, prev) => disposeTubeBatch(prev), { flush: 'post' });
+watch(verticalTubes, (_now, prev) => disposeTubeBatch(prev), { flush: 'post' });
+watch(hierarchyTubes, (_now, prev) => disposeTubeBatch(prev), { flush: 'post' });
+
 const crossArrowGeometry = new ConeGeometry(0.18, 0.5, 10);
 
 // ── Animated traffic packets — call edges only. ────────────────────────
@@ -1415,26 +1429,6 @@ onUnmounted(() => {
 </style>
 
 <style>
-.plane-label {
-  display: flex;
-  align-items: center;
-  padding: 4px 9px;
-  font-size: 11px;
-  font-weight: 700;
-  letter-spacing: 0.08em;
-  text-transform: uppercase;
-  color: var(--sw-fg-0);
-  background: rgba(15, 19, 26, 0.93);
-  border: 1px solid var(--sw-line-2);
-  border-radius: 5px;
-  white-space: nowrap;
-  transform: translateX(-100%);
-  pointer-events: none;
-}
-.plane-label[data-plane='apps']  { border-color: rgba(249, 115, 22, 0.65); color: #fb923c; }
-.plane-label[data-plane='mesh']  { border-color: rgba(168, 85, 247, 0.65); color: #c084fc; }
-.plane-label[data-plane='infra'] { border-color: rgba(34, 197, 94, 0.65); color: #4ade80; }
-
 /* NOTE: pointer-events on cientos <Html> wrappers is set via the
    component's `pointer-events="none"` prop (NOT a CSS class). Cientos
    applies `pointerEvents` as an INLINE style on the wrapper, which
diff --git a/apps/ui/src/features/infra-3d/Infra3DView.vue b/apps/ui/src/features/infra-3d/Infra3DView.vue
index 6f199c3..9b0d39d 100644
--- a/apps/ui/src/features/infra-3d/Infra3DView.vue
+++ b/apps/ui/src/features/infra-3d/Infra3DView.vue
@@ -71,6 +71,10 @@ const focusTarget = ref<{ x: number; y: number; z: number } | null>(null);
 // bundled defaults are in hand.
 const { config: infraConfig, levelsOrdered, ensureLoaded, levelForLayer } = useInfra3dConfig();
 const ready = ref(false);
+// Set when the config fetch rejects (OAP/BFF offline, or a role without
+// `infra-3d:read`). Without this the page sat on "Loading…" forever —
+// the operator couldn't tell a slow load from a hard failure.
+const configError = ref<string | null>(null);
 
 // Resolver + plane order are bound to the loaded config; both are
 // passed into the Scene AND used to build the local placement copy
@@ -459,10 +463,16 @@ function onKeyDown(e: KeyboardEvent): void {
 // pan + arrow keys / WASD work without any chrome to fight.
 onMounted(async () => {
   window.addEventListener('keydown', onKeyDown);
-  // Fetch the admin config; gate scene mount on success. A failure
-  // (offline / 401) leaves `ready` false and the operator sees the
-  // load message — no broken-render between 3-plane and 4-plane.
-  await ensureLoaded();
+  // Fetch the admin config; gate scene mount on success. On failure
+  // (offline / 401 from a role without infra-3d:read) surface the
+  // reason instead of hanging on the load message — no broken-render
+  // between 3-plane and 4-plane.
+  try {
+    await ensureLoaded();
+  } catch (err) {
+    configError.value = err instanceof Error ? err.message : String(err);
+    return;
+  }
   ready.value = true;
   // Kick the loading pipeline once. Subsequent runs are operator-
   // initiated (timeline strip's refresh button).
@@ -518,7 +528,13 @@ const visibleServices = computed(() => {
     </header>
 
     <div class="canvas-shell">
-      <div v-if="!ready" class="cfg-loading">Loading 3D map configuration…</div>
+      <div v-if="configError" class="cfg-error">
+        <strong>Couldn’t load the 3D map.</strong>
+        <span class="cfg-error__detail">{{ configError }}</span>
+        <span class="cfg-error__hint">Check that OAP is reachable and your role has 3D Infra Map access (<code>infra-3d:read</code>).</span>
+        <router-link class="cfg-error__back" to="/">← Back</router-link>
+      </div>
+      <div v-else-if="!ready" class="cfg-loading">Loading 3D map configuration…</div>
       <Infra3DScene
         v-else
         ref="sceneRef"
@@ -757,6 +773,24 @@ const visibleServices = computed(() => {
   letter-spacing: 0.02em;
   color: var(--sw-fg-3);
 }
+.cfg-error {
+  position: absolute;
+  inset: 0;
+  display: flex;
+  flex-direction: column;
+  align-items: center;
+  justify-content: center;
+  gap: 8px;
+  padding: 24px;
+  text-align: center;
+  font-size: 12px;
+  color: var(--sw-fg-1);
+}
+.cfg-error strong { color: var(--sw-err); font-size: 13px; }
+.cfg-error__detail { color: var(--sw-fg-2); font-family: var(--sw-font-mono, monospace); font-size: 11px; }
+.cfg-error__hint { color: var(--sw-fg-3); max-width: 420px; line-height: 1.5; }
+.cfg-error__back { margin-top: 6px; color: var(--sw-accent); text-decoration: none; }
+.cfg-error__back:hover { text-decoration: underline; }
 .pipeline-strip {
   position: absolute;
   left: 0;