47 improve inspected point visualization #58
Conversation
pruned the table with all the values related to the point selected, in order to delete all unused and useless attributes while keeping origianl behaviour
fixed a graphic bug that made not possible to see x,y and z coordinates on other measurement tools sections in the sidebar, and that showed Latitude, Longitude and Elevation in those sections
…into 47-improve-inspected-point-visualization
…ud on the globe add a simple label with the name of the measured object, taken from the sidebar measurement list. It automatically gets removed once the measured object is removed, and it moves with the moving of the points.
made labels come up faster after a movement, not requiring a camera update anymore
add background to make the labels visible even while displaying black or white datasets
WalkthroughAdds scoped table zebra-striping rules and two new CSS classes for on-canvas labels. Extends the measurements panel to create/manage per-measurement overlay labels, project 3D positions to screen space, update positions on camera/mouse/resize events, and manage overlay lifecycle and mounted state. Changes
Sequence DiagramsequenceDiagram
participant Viewer as Viewer/Renderer
participant Panel as Measurements Panel
participant Overlay as Overlay DOM
participant Camera as Camera
rect rgb(240,248,255)
note over Panel: Initialization & mount
Viewer->>Panel: initMeasurementsPanel(viewer)
Panel->>Overlay: create overlay div, attach to render area
Panel->>Panel: set data-mp-mounted flag
end
rect rgb(235,255,235)
note over Panel: Measurement added/updated
Panel->>Panel: rebuild measurement list
Panel->>Overlay: createOrUpdateMeasurementCanvasLabel(measurement)
Overlay->>Overlay: store label & metadata
end
rect rgb(255,250,240)
note over Camera: Interaction/viewport updates
Camera->>Panel: camera_changed / render / resize / marker_moved
Panel->>Overlay: updateOverlayPositions()
Overlay->>Overlay: getRepresentativePosition() -> projectToScreen()
Overlay->>Overlay: update DOM position & visibility
end
rect rgb(255,240,245)
note over Panel: Removal / cleanup
Panel->>Overlay: remove overlay label for deleted measurement
Overlay->>Panel: prune internal map
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
now it is not possible to see both name labels and value labels of the measurements, and it's possible to switch between these visualization thanks to two new buttons. There is also the possibility to hide both with a third button, all in the same section of the sidebar
Thanks for the suggestion! Just done that! |
![coderabbit[bot]](https://avatars.git.ntnu.no/u/70329?s=60){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/MeasurementControl/measurementsPanel.css(3 hunks)src/MeasurementControl/measurementsPanel.js(17 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-29T13:42:18.000Z
Learnt from: kleinc
Repo: TDT4290-group-4/MolloyExplorer PR: 52
File: src/AnnotationControl/annotationPanel.css:30-31
Timestamp: 2025-10-29T13:42:18.000Z
Learning: In src/AnnotationControl/annotationPanel.css, the .annotation-edit-textarea element intentionally has both background and color set to #636262, making the text invisible. This is the intended design per kleinc's preference.
Applied to files:
src/MeasurementControl/measurementsPanel.css
🧬 Code graph analysis (1)
src/MeasurementControl/measurementsPanel.js (3)
src/potreeViewer.js (3)
viewer(23-25)e(41-41)el(223-223)src/CoordinateShowing/coordinateShowing.js (1)
lon(38-42)src/AcceptedFiltering/threePanels.js (1)
$(199-199)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/MeasurementControl/measurementsPanel.js (1)
709-723: Fix className to match stylesheet (unresolved from past review).The past review correctly identified that the className should be
'attrRow'(not'attr-row') to match the existing stylesheet selectors. This issue remains unresolved.Apply this diff:
const createRowNode = (id, label, value) => { const row = document.createElement('tr') row.id = id - row.className = 'attr-row' + row.className = 'attrRow' const tdLabel = document.createElement('td')
🧹 Nitpick comments (1)
src/MeasurementControl/measurementsPanel.js (1)
485-570: Extract duplicatedlooksLikeNumberhelper.The
looksLikeNumberhelper function is defined identically in bothroundCoordinates(lines 424-428) andpruneMeasurementRows(lines 512-518). Extract it to a shared helper at the function scope to reduce duplication.Define once at the top level of
initMeasurementsPanel:export function initMeasurementsPanel(viewer) { const lastSelection = { uuid: null, label: '', type: null } + // Helper to detect if a string represents a number + const looksLikeNumber = (s) => { + if (!s) return false + const cleaned = (s + '').replace(/[^0-9+\-.,eE]/g, '').replace(/,+/g, '') + return /[-+]?\d*\.?\d+(e[-+]?\d+)?/.test(cleaned) + }Then remove the duplicate definitions in both functions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/MeasurementControl/measurementsPanel.css(2 hunks)src/MeasurementControl/measurementsPanel.js(17 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/MeasurementControl/measurementsPanel.css
🧰 Additional context used
🧬 Code graph analysis (1)
src/MeasurementControl/measurementsPanel.js (3)
src/potreeViewer.js (3)
viewer(21-23)e(39-39)el(234-234)src/CoordinateShowing/coordinateShowing.js (1)
lon(38-42)src/Filter/filter.js (1)
$(206-206)
🔇 Additional comments (9)
src/MeasurementControl/measurementsPanel.js (9)
1-2: LGTM: Import statements for coordinate transformation.The ecef and wgs84 imports are appropriately used later for lat/lon/elevation calculations via proj4.
124-166: Verify canvas dimensions to prevent NaN coordinates.The screen projection function should guard against zero-width or zero-height canvases, which could produce NaN coordinates and cause layout issues.
Consider adding dimension guards:
const canvas = (viewer && viewer.renderer && viewer.renderer.domElement) || document.querySelector('#potree_render_area canvas') if (!canvas) return null const w = canvas.clientWidth || canvas.width const h = canvas.clientHeight || canvas.height + if (!w || !h || w <= 0 || h <= 0) return null const x = (vec3.x * 0.5 + 0.5) * w const y = (-vec3.y * 0.5 + 0.5) * h
411-483: Approve coordinate rounding with mounted flag guard.The function correctly gates processing to only run when the properties panel is mounted for single-point measurements, and handles multiple coordinate table formats.
777-795: LGTM: Type tracking for mounted flag gating.Storing the measurement type in
lastSelectionenables the mounted flag logic to correctly gate post-processing to single-point measurements only.
888-896: LGTM: Overlay label creation integrated with list rebuild.The overlay labels are created/updated in sync with the measurement list, ensuring consistency between the sidebar and on-canvas labels.
924-936: LGTM: Proper cleanup of removed measurement overlays.The cleanup logic correctly removes overlay labels for measurements that no longer exist and updates positions for remaining labels.
964-980: LGTM: Movement event listeners update overlay positions.The event listeners correctly update overlay label positions when markers/points are moved, using
requestAnimationFramefor efficient rendering.
1076-1086: LGTM: Camera and resize listeners maintain overlay positions.The event listeners correctly update overlay label positions when the camera moves or the window resizes, ensuring labels stay aligned with their measurements.
1291-1337: LGTM: Display mode toggling with dual event handling.The display mode logic correctly toggles between VALUES (numeric labels), NAMES (measurement names), and HIDE (no labels) modes. The dual event handling approach (jQuery events + direct click listener) provides good compatibility.
fixed behaviour after deleting measurements. Now UI is immediately refreshed, not needing anymore to move the camera position to see the labels of deleted points disappear
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/MeasurementControl/measurementsPanel.js (1)
704-716: Keep using the existing.attrRowstyling hook.The new Latitude/Longitude/Elevation rows still use
className = 'attr-row', so they miss the.attrRowrules that style the rest of the measurement rows. Please switch back to the original class name so the rows inherit the expected look.- row.className = 'attr-row' + row.className = 'attrRow'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/MeasurementControl/measurementsPanel.css(2 hunks)src/MeasurementControl/measurementsPanel.js(18 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/MeasurementControl/measurementsPanel.css
🧰 Additional context used
🧬 Code graph analysis (1)
src/MeasurementControl/measurementsPanel.js (2)
src/potreeViewer.js (3)
viewer(21-23)e(39-39)el(234-234)src/CoordinateShowing/coordinateShowing.js (1)
lon(38-42)
Issue number
Closes #47
Description
Testing steps
To test the changes, try to add some points on the renderer area and check the values int the Measurement section of the sidebar while moving them, and also check the displaying of the labels in the potree render area, they should match the names in the sidebar. Try also to test different tools, as they all have their personalized label name, following the names present in the Measurement section of the sidebar, and to change the color of the datasets where the points sit, to test the visibility of the labels.
Screenshots (optional)
Summary by CodeRabbit
New Features
Bug Fixes
Style