wu-sheng opened a new pull request, #28:
URL: https://github.com/apache/skywalking-horizon-ui/pull/28

   ## Summary
   
   Addresses the actionable findings from the 3D Infra Map review. Four fixes 
here; three findings deferred (see below) because they need live-canvas 
validation or conflict with documented design — flagging rather than 
blind-fixing.
   
   ### Fixed
   - **HIGH — TubeGeometry GPU 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). Now disposes the previous batch when 
each set changes (`flush:'post'`, after the new geometries have rendered and 
old meshes detached).
   - **MED — `metricChunkSize` cap mismatch.** Config validation + the admin 
input allowed up to 50, but `/api/infra-3d/metrics` rejects >12 services 
(`MAX_SERVICES`, OAP GraphQL complexity ceiling) — so any oversized chunk 
5xx'd. Capped both at 12.
   - **MED — config-load failure state.** `/3d/map`'s `onMounted` `await 
ensureLoaded()` had no `try/catch`, so a reject (OAP offline, or a role without 
`infra-3d:read`) hung on *"Loading 3D map configuration…"* forever. Now caught 
→ shows the reason + an access hint (`infra-3d:read`) + a Back link.
   - **LOW — dead code.** Removed orphaned `.plane-label` CSS and a stale 
"label rendering" prop comment (no plane-label render path exists).
   
   ### Deferred (need live-canvas validation / design decision — not in this PR)
   - **Empty-space deselect.** The *obvious* fix (a deselect handler on the 
tier/zone planes) would **break** documented behavior: operators orbit-drag 
*from empty plane space* specifically to rotate **without** changing selection 
(`Infra3DScene` CLAUDE.md). The design-consistent fix is a temporal canvas 
*click* (down+up, no drag) handler, but current deselect is pmndrs 
`pointermissed`; whether that already fires on plane clicks must be confirmed 
in a browser before changing it.
   - **Admin edge styling.** Only `intraCall` arcs are rendered 
(cross-level/hierarchy edges are intentionally **not** drawn in the static 
view), and those arcs are solid `TubeGeometry` — `dashed`/`arrow` can't apply 
without reworking the rendering. Only `color` is trivially wireable; doing it 
partially would silently ignore the other two controls. Needs a rendering 
decision + visual check.
   - **Theme support.** Canvas clear-color + the neutral plane are 
dark-hardcoded; the cube/zone palette already comes from tokens. A real 
light-theme pass needs reactive re-read on theme switch (not a one-time read) 
plus a visual review of emissive/contrast — a focused design task.
   
   ## Validation
   - UI + BFF `type-check`, UI `build`, and `eslint` on touched files all clean 
(the 3 remaining eslint warnings are pre-existing `any` casts in 
`getControls()`).
   - ⚠ Not yet validated against the live 3D canvas — recommend a browser pass 
on the geometry-leak disposal (toggle layers / select repeatedly) and the new 
config-error state before merge.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to