GGGS store layers: depth-at-cursor readout via the depth-provider walk - #184
Merged
Conversation
Add GggsTile::sampleAt(), GggsTileLayer::getElevation(), AVP::getStoreElevation() dynamic walk, and ProjectView dual-label display
Sync the work plan to the operator-approved decision to fold all four Plan Review findings before implementing: - #1 (must-fix): sampleAt() reads NoData only through the pixelsLoaded() acquire gate, so the deferred nodata_/has_nodata_ members are never read unset. - #2: add tileLevel() basename-parse + headless getElevation() tests; getStoreElevation walk covered by a manual protocol note. - #3: getStoreElevation() honors the Layers-tab enable state (isVisible()). - #4: sampleAt() indexes the resident CPU buffer (texture() no longer frees data_) — a cheap in-memory lookup, no synchronous GDAL open per cursor move. Records the two open questions as decided (both labels shown; hidden store does not report elevation). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Hovering over an open GGGS bathymetry store now reports its value in the status bar. Previously getDepth() walked only m_depthRasters (chart depth bands), so store layers silently returned NaN. New query path, kept separate from getDepth() because store values are ellipsoidal up-positive heights, not chart-datum depths (distinct labels until the datum service, #288): - GggsTile::sampleAt(lon,lat): invert the north-up geotransform to a pixel index and read the RESIDENT CPU buffer. Gated on pixelsLoaded() (acquire), which is set by loadPixels() after the deferred NoData members are populated — so the NoData mask is never read while unset (Plan Review #1). - GggsTile::texture() no longer frees data_ after the GPU upload: sampleAt() indexes that resident buffer, so a point query is a cheap in-memory lookup with no synchronous GDAL re-open per cursor move (Plan Review #4). Trade-off: a painted tile now holds both a CPU and a GPU copy. - tileLevel() (gggs_tile_util.h): parse LEVEL from a <level>_<row>_<col> basename. - GggsTileLayer::getElevation(): sample the finest (highest-level) covering tile first, regardless of the rendered LOD (camp#103 follow-on). - AutonomousVehicleProject::getStoreElevation(): dynamic topLevelLayers() walk (no owned list — can't dangle) consulting only ENABLED (isVisible()) stores, per ADR-0003 §2's enabled-layer contract (Plan Review #3). - ProjectView::mouseMoveEvent(): query stores first (precedence) and append " Elev: X (ellipsoid)"; the chart " Depth: X" can still appear alongside. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- test_gggs_tile.cpp: SampleAtResidentBuffer asserts sampleAt() returns NaN BEFORE loadPixels() (the pixelsLoaded() gate that stops it reading the unset NoData members — Plan Review #1), then valid/NoData/out-of-extent results after load. TileLevelParse pins the <level>_<row>_<col> basename parse and its rejection of companions / malformed names. - test_gggs_elevation.cpp (new, headless / GL-free like test_gggs_rescan): getElevation() returns the finest (highest-level) covering tile's value despite the coarser tile loading first (descending-level sort), NaN out of extent (extent filter), and NaN over an all-NoData covering tile. getStoreElevation()'s enabled-layer walk is left to a manual protocol (building a full AutonomousVehicleProject in a unit test is disproportionate for a thin dynamic_cast + isVisible() filter). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Record the implementation outcome, commits, and 194-pass test result, plus the manual verification protocol for the getStoreElevation enabled-walk. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
getElevation() ran QFileInfo + QRegularExpression (tileLevel) on every covering tile per cursor move. Parse the level once in the GggsTile ctor and expose it via level(); getElevation() reads the cached value, keeping the hot GUI-thread query a plain extent test + small sort.
The pixel-index inversion assumes a north-up geotransform (geo[2] == geo[4] == 0), which every GGGS tile satisfies by construction. Return NaN (a miss) rather than a mis-indexed sample if a rotated/sheared tile ever reaches sampleAt().
The retained data_ buffer is held by every LOADED tile (filled in loadPixels(), never freed), not just painted ones, so the resident footprint scales with loaded tiles. Only the GL texture is painted-tile only. Reword the comment so it no longer understates the scope.
getDepth() filters on depthValid() only (membership, not the Layers-tab checkbox), so an unchecked chart still reports Depth:, whereas the sibling getStoreElevation() honors isVisible() per ADR-0003 §2. Document the intentional divergence at getDepth; aligning it is an out-of-scope follow-up on that path.
There was a problem hiding this comment.
Pull request overview
This PR extends CAMP’s status-bar cursor readout to include GGGS bathymetry “store” layers (via GggsTileLayer) by adding an in-memory point-sampling path over resident tile buffers, then wiring that into ProjectView alongside the existing chart-raster depth readout.
Changes:
- Add
GggsTile::sampleAt()and retain CPU tile buffers after GPU upload to enable fast per-cursor-move sampling without GDAL I/O. - Implement
GggsTileLayer::getElevation()(finest-covering-tile-wins) andAutonomousVehicleProject::getStoreElevation()(enabled/visible store layers only) and surface it inProjectViewasElev: … (ellipsoid). - Add headless unit tests for sampling semantics, tile level parsing, and elevation selection logic; register the new test target.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/camp_map/raster/gggs_tile.h |
Adds cached tile LOD level accessor and sampleAt() API for in-memory point sampling. |
src/camp_map/raster/gggs_tile.cpp |
Implements sampleAt(), caches tile level on construction, and retains CPU buffers after texture upload to support cursor queries. |
src/camp_map/raster/gggs_tile_util.h |
Adds tileLevel() helper for parsing <level>_<row>_<col> basenames. |
src/camp_map/raster/gggs_tile_layer.h |
Declares getElevation() API for querying store-layer elevation at a point. |
src/camp_map/raster/gggs_tile_layer.cpp |
Implements getElevation() with extent filtering and finest-level precedence using cached tile levels. |
src/camp/autonomousvehicleproject.h |
Declares getStoreElevation() as a separate ellipsoidal-elevation query path. |
src/camp/autonomousvehicleproject.cpp |
Implements getStoreElevation() by walking visible top-level GGGS store layers and returning the first non-NaN value. |
src/camp/projectview.cpp |
Adds status-bar “Elev: … (ellipsoid)” readout (stores first) alongside existing “Depth:” readout. |
test/test_gggs_tile.cpp |
Adds unit tests for sampleAt() resident-buffer behavior and tileLevel() parsing. |
test/test_gggs_elevation.cpp |
New headless tests for GggsTileLayer::getElevation() (extent filtering, finest-wins, all-NoData). |
CMakeLists.txt |
Registers and links the new test_gggs_elevation gtest target. |
.agent/work-plans/issue-180/plan.md |
Work plan documenting approach/constraints and how review findings were incorporated. |
.agent/work-plans/issue-180/progress.md |
Progress log capturing review/verification steps and outcomes for issue #180. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Wires GGGS store layers into the status-bar depth-at-cursor readout: hovering over an open bathy store (e.g. the S-102 reference store from rolker/unh_marine_autonomy#278) now reports the store's value, where previously only legacy background chart rasters contributed.
Closes #180
What's here
GggsTile::sampleAt()— resident-buffer point sample (no GDAL open on the cursor path); returns NaN beforeloadPixels()and applies the tile's NoData mask; guards sheared geotransforms (returns NaN rather than a wrong value); cached LOD level replaces per-move basename parsing.GggsTileLayer::getElevation()— extent filter → finest-covering-tile-wins walk over loaded tiles.getStoreElevation()joins the depth-provider walk, consulting only Layers-tab-enabled layers (ADR-0003 §2); precedence: stores first, then chart rasters, load order within. Deregistration on layer remove and shutdown covered.getDepthvisibility divergence and the actual CPU-buffer retention behavior intexture().Test plan
sampleAt()resident-buffer semantics (NaN-before-load, NoData mask),tileLevel()parse,getElevation()extent filter + finest-wins, provider-walk visibility filtering. Full camp suite 194 tests, 0 failures (12 pre-existing GL skips in-container).Review history (local-first)
Issue review → plan → plan review (approve-with-suggestions, all 4 findings folded pre-implementation) → implement → pre-push R1 approved (0 must-fix, Ship: recommended) → operator-requested suggestion pass (4 commits) → R2 approved (0 must-fix, Ship: recommended). Timeline in
.agent/work-plans/issue-180/progress.md.Authored-By:
Claude Code AgentModel:
Claude Fable 5