diff --git a/.agent/work-plans/issue-141/plan.md b/.agent/work-plans/issue-141/plan.md new file mode 100644 index 0000000..3a6651d --- /dev/null +++ b/.agent/work-plans/issue-141/plan.md @@ -0,0 +1,122 @@ +# Plan: Adopt marine_colormap in camp (replace internal ColorMap) + +## Issue + +https://github.com/rolker/camp/issues/141 + +## Context + +camp's internal `camp::map::ColorMap` (Grayscale/Viridis/Turbo) is used across 7 files +(`color_map.{h,cpp}`, `raster_gl_renderer`, `gggs_tile_layer`, `sonar_live_cache_layer`, +`raster_layer`, `grid_map`). `marine_colormap` is now merged (jazzy) and offers 6 palettes +(grayscale/bronze/thermal/viridis/turbo/quality) plus `bake_lut()` for LUT upload. rviz and rqt +already adopt it. camp#134 unified the render path into `RasterGlRenderer` + `RasterFieldSource`; +this issue swaps the colormap source, exposing the full 6-palette registry to camp operators. + +**LUT integration decision (record in ADR-0008):** bake marine_colormap's ramps into the existing +RasterGlRenderer 256×1 LUT-texture path via `bake_lut(palette, TransferParams{}, 256)` — do NOT +swap in marine_colormap's GPU shader. The #134 render contract (NaN + finite-NoData discard, +Nearest, per-band range uniforms) is preserved unchanged in the fragment shader. + +**Persisted-name migration:** existing settings use lowercase names (the current `ColorMap::name()` +already returns lowercase). shared names grayscale/viridis/turbo are identical in marine_colormap. +Add a case-insensitive fallback in `readSettings()` for any deployments that saved capitalized +names. + +## Approach + +1. **Write parity-baseline test** — add `test/test_color_map_parity.cpp`: hardcode expected + LUT samples from the old ColorMap ramps for grayscale/viridis/turbo at t={0,0.5,1} (and a + few interior points); compare against `marine_colormap::bake_lut()` output (≤2 per channel + tolerance for rounding). Commit alongside old `color_map.cpp` so CI confirms parity before + the old code is removed. + +2. **Add marine_colormap dependency** — `package.xml`: add `marine_colormap`. + `CMakeLists.txt`: add `find_package(marine_colormap REQUIRED)` and + `target_link_libraries(camp_map PUBLIC marine_colormap)`. + +3. **Migrate RasterGlRenderer** — replace `map::ColorMap colormap_` with + `std::string colormap_name_{"grayscale"}`. Update `setColormap(const std::string& name)` / + `colormap() const → const std::string&`. Replace `ensureLut()` body: call + `marine_colormap::find_palette(colormap_name_)` → `bake_lut(pal, TransferParams{}, 256)`; + copy the `Rgba8` vector to the existing 256×1 RGBA texture. If `find_palette` returns nullptr, + fall back to "grayscale". + +4. **Migrate RasterLayer, GggsTileLayer, SonarLiveCacheLayer** — update `setColormap` signature + to `const std::string& name`. Rebuild context menus from + `marine_colormap::palette_names()`. In `readSettings()`: read stored string; apply + case-insensitive lowering before passing to `setColormap`; unknown names default to current + ramp. In `writeSettings()`: write palette name string as-is. + +5. **Migrate GridMap (CPU path)** — replace `map::ColorMap colormap_` with + `std::string colormap_name_{"grayscale"}`. In `processGridMap` / `renderToData`, replace + `colormap.colorNormalized(t)` with: `auto& pal = *marine_colormap::find_palette(colormap_name_); + auto c = pal.sample(float(t)); QColor(int(c.r*255+0.5f), …, 255)`. Menu, settings, fallback + same as step 4. + +6. **Delete color_map.{h,cpp}** — remove from `CAMP_MAP_SOURCES` in CMakeLists.txt, delete files. + Update `test_color_map.cpp` to cover: all 6 palettes opaque in-range, NaN transparent, + endpoint values, `palette_names()` round-trip, case-insensitive settings fallback. Remove + old parity test once it passes (its job is done at step 1). + +7. **Write ADR-0008** — `docs/decisions/0008-adopt-marine-colormap-lut-bake.md`. Records: chose + bake-into-existing-LUT over swapping GPU shader; rationale (preserves #134 NaN/NoData/Nearest + contract, no shader rewrite risk); consequences (TransferParams must be identity at bake time; + `marine_colormap_response` is NOT used in the shader). + +## Files to Change + +| File | Change | +|------|--------| +| `package.xml` | Add `marine_colormap` | +| `CMakeLists.txt` | find_package + link camp_map; remove `color_map.cpp`; update test | +| `src/camp_map/map/color_map.h` | Delete | +| `src/camp_map/map/color_map.cpp` | Delete | +| `src/camp_map/raster/raster_gl_renderer.h` | `std::string colormap_name_`; update sig | +| `src/camp_map/raster/raster_gl_renderer.cpp` | `ensureLut()` via `bake_lut()` | +| `src/camp_map/raster/gggs_tile_layer.h` | Update `setColormap` sig | +| `src/camp_map/raster/gggs_tile_layer.cpp` | Menu, settings, impl | +| `src/camp_map/ros/live_coverage/sonar_live_cache_layer.h` | Update `setColormap` sig | +| `src/camp_map/ros/live_coverage/sonar_live_cache_layer.cpp` | Menu, settings, impl | +| `src/camp_map/raster/raster_layer.h` | Update `setColormap` sig | +| `src/camp_map/raster/raster_layer.cpp` | Menu, settings, impl, default init | +| `src/camp_map/ros/grids/grid_map.h` | `std::string colormap_name_`; update sig | +| `src/camp_map/ros/grids/grid_map.cpp` | CPU sample path via `find_palette` | +| `test/test_color_map.cpp` | Replace with marine_colormap behavior + settings tests | +| `test/test_color_map_parity.cpp` | New: parity baseline (step 1, then retired) | +| `docs/decisions/0008-adopt-marine-colormap-lut-bake.md` | New ADR | + +## Principles Self-Check + +| Principle | Consideration | +|---|---| +| Human control and transparency | Full 6-palette menu exposed; per-band selection and defaults preserved; settings-key migration is backward-compatible | +| Capture decisions, not just implementations | LUT integration choice recorded in ADR-0008 | +| A change includes its consequences | Parity test, settings migration, and #63 wording reconciliation all in-scope | +| Only what's needed | Widget/range UI deferred to #142; no marine_colormap_response use in shader | +| Test what breaks | Parity test at step 1 locks in render equivalence before deletion | + +## ADR Compliance + +| ADR | Triggered | How addressed | +|---|---|---| +| ADR-0007 (RasterFieldSource) | Yes — modifies RasterGlRenderer's LUT path | bake_lut() replaces hand-coded LUT loop; shader contract unchanged | +| ADR-0001 (Adopt ADRs) | Yes — LUT integration is a design decision | New ADR-0008 records choice and rationale | +| ROS 2 conventions (ament) | Yes — new dependency | `` + `find_package` + `target_link_libraries(camp_map PUBLIC marine_colormap)` | + +## Consequences + +| If we change... | Also update... | Included? | +|---|---|---| +| `ColorMap::setColormap(Type)` API | All 4 call sites (gggs, sonar, raster_layer, grid_map) | Yes | +| Palette names in settings | Case-insensitive read fallback | Yes | +| camp#63 "camp-internal ColorMap" wording | Reconcile in ADR-0008 + issue comment | Yes (ADR reference) | +| `bake_lut` identity-params contract | Do NOT also apply `marine_colormap_response` in shader | Yes (ADR note) | + +## Open Questions + +- [ ] No open questions — scope decisions are explicit in the issue; plan is review-plan-ready. + +## Estimated Scope + +Single PR. ~17 files, all in the `camp` repo. Build + test on host only (container cannot build camp). diff --git a/.agent/work-plans/issue-141/progress.md b/.agent/work-plans/issue-141/progress.md new file mode 100644 index 0000000..259fc7f --- /dev/null +++ b/.agent/work-plans/issue-141/progress.md @@ -0,0 +1,243 @@ +--- +issue: 141 +--- + +# Issue #141 — Adopt marine_colormap in camp (replace internal ColorMap) + +## Issue Review +**Status**: complete +**When**: 2026-06-29 00:00 +00:00 +**By**: Claude Code Agent (Claude Sonnet) + +**Issue**: #141 +**Comment**: (best-effort post follows this entry; not recorded inline) +**Scope verdict**: well-scoped + +### Summary + +Replace camp's internal `camp::map::ColorMap` (3 named ramps: Grayscale, Viridis, Turbo) with +`marine_colormap` palettes + `bake_lut()` / GPU shader across the unified render path +(`RasterGlRenderer` + ~7 consumer files). Explicitly defers the colorbar widget and range UI to +camp#142. `marine_colormap` is already merged (jazzy); camp#134 (render path unification) is +merged (#140). The scope is moderate but coherent: one PR, one dependency addition, one +architectural decision to record. + +### Principle Alignment + +| Principle | Status | Notes | +|---|---|---| +| Human control and transparency | OK | Colormap menu + per-band selection preserved; settingsKey() persistence maintained; defaults are user-overridable | +| Capture decisions, not just implementations | Watch | LUT integration decision (use marine_colormap's GPU shader vs. bake its ramps into the existing LUT-texture path) is architectural — should be recorded in a project ADR or the plan's decision block, not left implicit | +| A change includes its consequences | Watch | camp#63 "camp-internal ColorMap" wording reconciliation is in scope; render-parity verification must be part of the PR, not a follow-up | +| Only what's needed | OK | Widget and range UI correctly deferred to #142; no premature abstraction | +| Improve incrementally | OK | ~7 consumer files; staging guidance (RasterGlRenderer/GGGS first, then grids/raster_layer) already noted; fits one PR | +| Test what breaks | Action needed | Issue names behavior parity as the key risk but does not specify a test mechanism; plan must define how render equivalence is verified | +| Workspace vs. project separation | OK | Project-repo change; marine_colormap is a project-level library | + +### ADR Applicability + +| ADR | Triggered | Notes | +|---|---|---| +| ADR-0001 (Adopt ADRs) | Watch | LUT path decision is architectural; capture in a camp project ADR or plan decision block | +| ADR-0002 (Worktree isolation) | OK | feature/issue-141 branch already exists in camp repo | +| ADR-0008 (ROS 2 Conventions) | OK | Adding marine_colormap via package.xml + CMakeLists.txt; standard ament pattern | + +### Consequences + +- **package.xml + CMakeLists.txt**: Must add `marine_colormap` and ament find/target_link. +- **Settings persistence**: Existing persisted colormap names (e.g., "Grayscale", "Viridis", "Turbo") must map to marine_colormap palette names; a name-migration fallback may be needed if names differ. +- **camp#63 wording**: Reconcile "camp-internal ColorMap" issue description once this ships. +- **All ~7 consumers must be migrated together**: a partial migration risks a mixed CPU/GPU LUT path for the same data. + +### Actions +- [ ] Define render-parity test approach in the plan: specify how equivalence between the old camp LUT and marine_colormap `bake_lut()` output is verified (e.g., unit test comparing LUT entries at sampled points, host-side visual test, or `./ui_ws/test.sh camp` coverage). +- [ ] Resolve "bathy/turbo" and "sequential" palette name ambiguity: marine_colormap's registry is grayscale/bronze/thermal/viridis/turbo/quality — map each per-band default to a named entry or add a new palette to marine_colormap before this issue can ship. +- [ ] Record the LUT integration decision (marine_colormap shader vs. bake ramps into existing LUT-texture path) in a camp project ADR or the plan's decision block. +- [ ] Verify settings-key persistence: ensure that persisted colormap names survive the rename (add migration/fallback if marine_colormap's ramp names differ from current `ColorMap::name()` strings). + +## Plan Authored +**Status**: complete +**When**: 2026-06-29 00:00 +00:00 +**By**: Claude Code Agent (Claude Sonnet) + +**Plan**: `.agent/work-plans/issue-141/plan.md` at `d14ca9b` +**Branch**: feature/issue-141 at `d14ca9b` +**Phases**: single + +### Open questions +- [ ] No open questions — scope decisions are explicit in the issue; plan is review-plan-ready. + +## Plan Review +**Status**: complete +**When**: 2026-06-29 09:52 +00:00 +**By**: Claude Code Agent (Claude Opus) + + +**Plan**: `.agent/work-plans/issue-141/plan.md` at `d14ca9b` +**PR**: PR-less (`--issue` mode; `gh` unauthenticated, issue context taken from the +`## Issue Review` entry above) +**Verdict**: changes-requested + +### Findings +- [ ] (must-fix) Parity test rests on false equivalence — camp viridis/turbo are sparse 7/8-stop + approximations; marine_colormap's are canonical 256-entry tables. At t=0.5 camp viridis (34,168,132) + vs canonical (33,145,140) = green off by 23, >> the ≤2/channel tolerance. Grayscale parity holds; + viridis/turbo do not. Reframe step 1 (grayscale exact; viridis/turbo = intended change, snapshot new + values) and fix the "locks in render equivalence" Principles claim — `plan.md:28`, `plan.md:97` +- [ ] (suggestion) ADR table mislabels camp ADR-0001 as "Adopt ADRs" — that is a *workspace* ADR + (camp ADR-0001 is TopicBridge); ADR-0007 already corrected this exact error — `plan.md:104` +- [ ] (suggestion) Link `marine_colormap` `PRIVATE` not `PUBLIC` — no marine_colormap type appears in + camp_map's installed headers post-migration (Only what's needed) — `plan.md:36`, `plan.md:105` +- [ ] (suggestion) ~17 files exceeds the single-PR rubric; record a GPU/CPU split fallback + (renderer+gggs+sonar, then raster_layer+grid_map) per the Issue Review staging note — `plan.md:120` +- [ ] (suggestion) Scope the new ADR explicitly as "camp ADR-0008" to avoid collision with workspace + ADR-0008 (ROS 2 conventions) referenced in the Issue Review — `plan.md:62`, `plan.md:87` + +## Implementation +**Status**: complete (host-verify pending — container cannot build camp) +**When**: 2026-06-29 10:30 +00:00 +**By**: Claude Opus + +**Branch**: feature/issue-141 +**Commits** (atomic, ordered; agent identity stamped): +- `166bdc1` feat: add marine_colormap dependency + ADR-0008 +- `3c35cfe` refactor: migrate GPU LUT path (renderer + gggs + sonar + raster_layer) +- `a61764e` refactor: migrate GridMap CPU path +- `9d0bf33` refactor: delete internal ColorMap; characterize marine_colormap +- (this entry) progress + +### What changed +Replaced camp's internal `camp::map::ColorMap` (3 sparse ramps) with `marine_colormap` +(6-palette registry) across all 7 consumers, migrated together (no mixed path): + +- **GPU path** (`RasterGlRenderer::ensureLut`, used by gggs_tile_layer / + sonar_live_cache_layer / raster_layer): the LUT is now baked via + `bake_lut(find_palette(name), TransferParams{}, 256)` and copied into the existing + 256×1 RGBA LUT texture. The **#134 shader contract is preserved unchanged** — NaN + + finite-NoData `discard`, `Nearest`, per-band range normalize stay in the fragment + shader; marine_colormap's GPU shader / `marine_colormap_response` are **not** used. + Identity `TransferParams` at bake time (range/gain/contrast stay the shader's job). +- **CPU path** (`GridMap`): `colormap.colorNormalized(t)` → `find_palette(name)->sample(t)` + quantized to a `QColor` (`to_rgba8`); invalid (NaN) cells stay transparent via the + existing `!isnan` guard. The worker snapshot carries the name by value under the + same mutex as before. +- **API**: `setColormap(const std::string& name)`; layers store/persist + `std::string colormap_name_`; getter returns the name. Unknown name → grayscale + (renderer `ensureLut` + each layer's read both fall back). + +**Full-registry exposure**: every Colormap context menu is now built from +`marine_colormap::palette_names()` (grayscale/bronze/thermal/viridis/turbo/quality), +not the old three ramps. + +**Persisted-name migration**: each `readSettings()` lowercases the stored name and +validates it against the registry (`palette_index`), falling back to grayscale for an +unknown name — so existing lowercase names round-trip and any legacy capitalized +"Viridis"/"Turbo" still resolves. Persistence stays keyed via `settingsKey()` / +`itemID()` as before. RasterLayer keeps **viridis** as its scalar default. + +**Dependency**: `marine_colormap` + `find_package`; linked **PRIVATE** +to `camp_map` (no marine_colormap type is in any installed camp_map header — the +colormap is a `std::string` name) and added to `camp_map_ros`'s +`ament_target_dependencies` for the GridMap/SonarLiveCacheLayer consumers. + +**Deleted** `src/camp_map/map/color_map.{h,cpp}` + removed from CMake sources. + +### Plan-review findings folded in +- **(must-fix) Reframed the parity test — no false equivalence.** `test_color_map.cpp` + is replaced with a characterization suite: **grayscale** asserted exact; + **viridis/turbo** snapshot the NEW canonical 256-entry `bake_lut` output + (endpoints exact, midpoints within 1 LSB: viridis[128]≈(33,145,140), + turbo[128]≈(164,252,60)) and explicitly guard that the canonical (not the old + sparse ~168-green) table is in use. Also covers: all 6 palettes opaque in-range, + `palette_names()` registry pin, name↔index round-trip, and the case-insensitive + grayscale-fallback rule. No "locks in render equivalence" wording. NaN-discard + stays covered by the render-path test (`test_raster_gl_renderer.cpp`). +- **(suggestion) PRIVATE link** — done (see Dependency above). +- **(suggestion) camp ADR-0008 numbering** — ADR explicitly labels itself **camp + ADR-0008** with a note disambiguating it from the workspace ADR-0008, and its + cross-refs use the correct camp series (camp ADR-0001 = TopicBridge, ADR-0007 = + RasterFieldSource). The committed `plan.md`'s ADR-table mislabel was left as the + historical record; the authoritative ADR is correct. +- **(suggestion) GPU/CPU split fallback** — the migration was committed as that exact + split (GPU: renderer+gggs+sonar+raster_layer; CPU: grid_map; then delete), so a + bisect/partial-revert seam exists even though it shipped as one PR. + +### Build status +**Not built in-container** — `./ui_ws/build.sh camp` fails at colcon configure because +the underlay `marine_colormap` (and the other lower layers) are not installed in the +container (the known camp limitation): `Failed to find ... install/marine_colormap/ +share/marine_colormap/package.sh`. This is a missing-underlay error, not a compile +error in the change, and confirms the new dependency is now resolved by colcon. The +edits were made cleanly and committed; **host verifies** via +`source setup.bash; ./ui_ws/build.sh camp; ./ui_ws/test.sh camp`. + +### Next step +Host build + test. On green, this is ready for PR (Closes #141); reconcile camp#63's +"camp-internal ColorMap" wording per ADR-0008. + +## Local Review (Pre-Push) +**Status**: complete +**When**: 2026-06-29 14:48 +00:00 +**By**: Claude Code Agent (Claude Opus) +**Verdict**: approved + +**Branch**: feature/issue-141 at `eb483bd` +**Mode**: pre-push +**Depth**: Deep (reason: new project ADR + render-path/concurrency refactor + new dependency) +**Must-fix**: 0 | **Suggestions**: 2 +**Round**: 1 | **Ship**: recommended — no must-fix; both suggestions are pre-existing/parity-preserving niceties + +### Findings +- [ ] (suggestion) `GridMap::readSettings` reads `colormap_name_` as the QSettings default without holding `mutex_` (pre-existing pattern; now a std::string — read default under lock or use literal "grayscale") — `src/camp_map/ros/grids/grid_map.cpp:250` +- [ ] (suggestion) `QColor(c.r, c.g, c.b)` drops palette alpha; parity-preserving (all built-in palettes opaque) — optionally pass `c.a` — `src/camp_map/ros/grids/grid_map.cpp:170` +- [ ] (follow-up) Post the camp#63 "camp-internal ColorMap" wording reconciliation comment when `gh` is authenticated — `docs/decisions/0008-adopt-marine-colormap-lut-bake.md` + +**Notes**: API usage verified correct against live marine_colormap headers (find_palette/bake_lut/palette_index/palette_names/sample/to_rgba8 signatures + 6-palette registry order). #134 GPU shader contract preserved (identity TransferParams). Static analysis: repo has no lint/CI gate; cppcheck cannot parse Qt without project config. Adversarial: 2 disjoint-lens passes both independently surfaced the two suggestions above (cross-pass confirmed). + +## Implementation (Address Review Findings — Round 1) +**Status**: complete (host-verify pending) +**When**: 2026-06-29 +00:00 +**By**: Claude Opus +**Branch**: feature/issue-141 at `477136a` + +Applied both round-1 suggestions in `src/camp_map/ros/grids/grid_map.cpp`: + +1. **readSettings colormap default (~:250)** — replaced the unlocked + `QString::fromStdString(colormap_name_)` QSettings default with the string literal + `"grayscale"`, so no `colormap_name_` member is read without holding `mutex_`. The + existing registry-validation (`palette_index` → fallback `"grayscale"`) makes this a + no-op for valid stored values and identical fallback for unknowns; behavior unchanged. +2. **Palette alpha (~:170)** — `QColor(c.r, c.g, c.b)` → `QColor(c.r, c.g, c.b, c.a)`. + `marine_colormap::Rgba8` already carries an 8-bit `a` (to_rgba8 scales 0..1→0..255 for + all four channels), so `c.a` is passed directly — no extra scaling. Parity-preserving + for the opaque built-in palettes; correct if a translucent palette is ever added. + +### Build status +**Host verifies** — the container cannot build camp (missing underlays; known limitation). +Host runs `source setup.bash; ./ui_ws/build.sh camp; ./ui_ws/test.sh camp`. + +### Next step +Host build + test. On green, ready for PR (Closes #141). Not pushed. + +## Local Review (Pre-Push) +**Status**: complete +**When**: 2026-06-29 16:49 +00:00 +**By**: Claude Code Agent (Claude Opus) +**Verdict**: approved + +**Branch**: feature/issue-141 at `e9c68b2` +**Mode**: pre-push +**Depth**: Deep (reason: new project ADR-0008 + 1123 lines / 21 files + render-path/concurrency refactor + new dependency) +**Must-fix**: 0 | **Suggestions**: 0 +**Round**: 2 | **Ship**: recommended — round-1's two suggestions are both addressed; this round surfaced no must-fix and no new actionable findings + +### Findings +- [ ] No issues found. LGTM. + +**Notes**: Re-reviewed the diff after the round-1 address-findings pass (commits `477136a`, `e9c68b2`). Both round-1 suggestions verified fixed in `grid_map.cpp`: readSettings now uses the literal `"grayscale"` QSettings default (no unlocked `colormap_name_` read), and the CPU path passes palette alpha via `QColor(c.r, c.g, c.b, c.a)`. marine_colormap API usage re-verified against live headers — `find_palette`→`const Palette*`, `bake_lut(pal, TransferParams{}, 256)`→`vector`, `palette_index`→`optional` (correct `if(!...)` truthiness), `palette(idx).name()`, `Palette::sample(float)`→`Rgba`, `to_rgba8`→`Rgba8`. Registry confirmed 6 palettes (grayscale/bronze/thermal/viridis/turbo/quality) — `test_color_map.cpp`'s `RegistryIsTheFullSix` pin is accurate (the 5-entry docstring in marine_colormap's `palette.hpp` is stale upstream, not in this diff). Concurrency: GridMap snapshots `colormap_name_` by value under `mutex_` before each worker launch / menu build / settings write; the `find_palette` pointer targets a static (app-lifetime) registry and Meyer's-singleton lookups are read-only/reentrant — race-free. #134 GPU shader contract preserved (identity TransferParams; per-band range-normalize + NaN/NoData discard stay in the shader). Two disjoint-lens Claude Adversarial passes (Lens A logic / Lens B systemic) both returned no must-fix. Static analysis: repo has no lint/CI/pre-commit gate (per `.agents/README.md`); cppcheck cannot parse Qt without project config — build + gtest (host-side; container lacks underlays) is the only gate. Governance: camp ADR-0008 added and correctly disambiguated from the workspace ADR-0008; cross-refs use the camp series (ADR-0001=TopicBridge, ADR-0007=RasterFieldSource); dependency linked PRIVATE to camp_map + `ament_target_dependencies` on camp_map_ros. Plan drift: `test_color_map_parity.cpp` was not added as a separate file — its parity baseline was folded into the reframed `test_color_map.cpp` characterization suite (per the plan-review must-fix on false equivalence); a documented, sound deviation. + +### Follow-ups (non-blocking) +- [ ] (follow-up) Post the camp#63 "camp-internal ColorMap" wording-reconciliation comment when `gh` is authenticated — `docs/decisions/0008-adopt-marine-colormap-lut-bake.md` +- [ ] (follow-up) Host build + test (`source setup.bash; ./ui_ws/build.sh camp; ./ui_ws/test.sh camp`) before push — container cannot build camp (missing underlays; known limitation). diff --git a/CMakeLists.txt b/CMakeLists.txt index 5df1dea..31862ea 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -18,6 +18,7 @@ find_package(nav_msgs REQUIRED) find_package(nav2_msgs REQUIRED) find_package(pluginlib REQUIRED) find_package(marine_autonomy REQUIRED) +find_package(marine_colormap REQUIRED) find_package(marine_interfaces REQUIRED) find_package(marine_tiled_raster_store REQUIRED) find_package(marine_nav_interfaces REQUIRED) @@ -249,7 +250,6 @@ set(CAMP_MAP_SOURCES src/camp_map/map/map_item_mime_data.cpp src/camp_map/map/layer_list.cpp src/camp_map/map/layer.cpp - src/camp_map/map/color_map.cpp src/camp_map/map_tiles/cached_tile_loader.cpp src/camp_map/map_tiles/map_tiles.cpp src/camp_map/map_tiles/tile.cpp @@ -293,6 +293,11 @@ target_link_libraries(camp_map Qt5::Positioning PRIVATE ${GDAL_LIBRARY} + # [camp#141] marine_colormap is consumed only inside camp_map's .cpp files + # (the raster LUT bake / CPU sample); no marine_colormap type appears in any + # installed camp_map header (the colormap is a std::string name), so link it + # PRIVATE — it does not propagate into camp_map's public link/usage interface. + marine_colormap::marine_colormap ) install(TARGETS camp_map @@ -347,6 +352,7 @@ ament_target_dependencies(camp_map_ros grid_map_ros marine_ais_msgs marine_autonomy + marine_colormap marine_interfaces marine_tiled_raster_store rclcpp @@ -402,13 +408,14 @@ if(BUILD_TESTING) ) - # ColorMap facility unit tests (camp#63) + # Colormap characterization tests (camp#63 / camp#141): the marine_colormap + # palettes, the exposed registry, and the name-resolution rules the layers use. ament_add_gtest(test_color_map test/test_color_map.cpp ) target_link_libraries(test_color_map - camp_map - Qt5::Test + marine_colormap::marine_colormap + Qt5::Core ) # Collision-monitor polygon converter unit tests diff --git a/docs/decisions/0008-adopt-marine-colormap-lut-bake.md b/docs/decisions/0008-adopt-marine-colormap-lut-bake.md new file mode 100644 index 0000000..084f685 --- /dev/null +++ b/docs/decisions/0008-adopt-marine-colormap-lut-bake.md @@ -0,0 +1,114 @@ +# ADR-0008: Adopt `marine_colormap`; bake its palettes into the existing GL LUT + +## Status + +Accepted + +Implements CAMP issue [#141](https://github.com/rolker/camp/issues/141) — replace +camp's internal `camp::map::ColorMap` (three hand-sampled ramps: Grayscale, Viridis, +Turbo) with the shared `marine_colormap` package (six palettes: grayscale, bronze, +thermal, viridis, turbo, quality). Builds on +[ADR-0007](0007-raster-field-source-interface.md) (the unified +`RasterFieldSource` + `RasterGlRenderer` render path this migration retargets) and +[ADR-0002](0002-web-mercator-scene-and-layer-model.md) (the Web-Mercator scene/layer +model the raster layers stay within). + +> **Numbering.** This is **camp ADR-0008** — a *project* ADR in this repo's +> `docs/decisions/` series, where ADR-0001 is the TopicBridge/executor contract. +> It is **not** the workspace ADR-0008 ("ROS 2 conventions") referenced from the +> issue-review record; the two series are independent and the numbers collide by +> coincidence. (ADR-0007 had to make the same disambiguation.) + +## Context + +camp grew its own `ColorMap` (camp#63) so each scalar-field renderer would stop +reimplementing a value→colour ramp. It carried three ramps; viridis/turbo were +**sparse approximations** — viridis sampled at 7 stops, turbo at 8 — interpolated +piecewise-linearly. It served two render paths: the GPU LUT bake in +`RasterGlRenderer::ensureLut()` (GGGS tiles, live sonar coverage, single-chart +`RasterLayer`) and the CPU `colorNormalized()` sample in `GridMap`. + +Meanwhile `marine_colormap` landed (jazzy) as the project-wide colormap library — +already adopted by rviz and rqt. It ships a **name-keyed, append-only registry** of +six palettes, where viridis/turbo are the **canonical 256-entry matplotlib/Google +tables** (not sparse stops), plus `bake_lut(palette, TransferParams, n)` for GPU LUT +upload and `Palette::sample(t)` for CPU lookup. Keeping camp's private ramps means a +second, drifting colormap definition and a viridis/turbo that visibly disagree with +every other tool in the workspace. + +Two design questions had to be settled before adopting it: + +1. **How to integrate on the GPU.** `marine_colormap` also offers a full GPU shader + path (`marine_colormap::shader` + a `marine_colormap_response` uniform block) that + applies range-normalize, gain, contrast, alpha-ramp and below/no-data sentinels in + the fragment shader. camp#134 (ADR-0007) had *just* unified camp's own fragment + shader, whose contract is load-bearing: NaN **and** finite-sentinel NoData + `discard`, `Nearest` value-texture filtering (so the equality NoData test never + blends across a sentinel boundary — the camp#122 halo fix), and per-band range + normalize over the *true* data span. Swapping in marine_colormap's shader would put + that contract at risk for no functional gain. + +2. **Whether to accept a colour change.** Because the canonical viridis/turbo tables + differ from camp's sparse approximations (e.g. viridis at t=0.5 is ≈(33,145,140) + canonical vs ≈(34,168,132) in camp's 7-stop ramp — green off by ~23), adopting + marine_colormap is **not** a no-op recolour: existing depth/backscatter renders + shift slightly toward the published palettes. + +## Decision + +1. **Bake marine_colormap palettes into the existing 256×1 RGBA LUT; keep the + camp#134 shader.** `RasterGlRenderer::ensureLut()` now calls + `marine_colormap::bake_lut(find_palette(name), TransferParams{}, 256)` and copies + the returned `Rgba8` entries into the same LUT texture the fragment shader already + samples. The shader is **unchanged** — NaN/finite-NoData discard, `Nearest` + filtering and per-band range normalize are all preserved. marine_colormap's GPU + shader and `marine_colormap_response` block are **not** used. + +2. **Bake with identity `TransferParams{}`.** `min=0, max=1, gain=1, contrast=1`, + alpha-ramp off, no below/no-data colour. This is load-bearing: range-normalize + stays the shader's job (per-band `u_min`/`u_max` over the true span), and the + NoData sentinels stay the shader's `discard` branches. The LUT carries *only* the + palette colour ramp — exactly what camp's `colorNormalized(i/255)` loop used to + produce. (This is also why `bake_lut`'s own min/max/gain/contrast must stay + identity: applying them here would double-apply the range the shader already owns.) + +3. **CPU path uses `Palette::sample(t)`.** `GridMap` replaces + `colormap.colorNormalized(t)` with `find_palette(name)->sample(t)` quantized to a + `QColor`. The `marine_colormap` contract guarantees the CPU sample and the baked + LUT agree at the same normalized position, so GGGS/live/chart (GPU) and grid (CPU) + render the same palette. + +4. **The colormap selection is a `std::string` palette name.** `setColormap` takes a + name; layers store `std::string colormap_name_` and persist the name via + `settingsKey()` as before. An unknown name falls back to `"grayscale"`. Persisted + names are read **case-insensitively** (lowercased, then validated against the + registry) so any legacy capitalized `"Viridis"`/`"Turbo"` still resolves. The full + six-palette registry is exposed in every colormap context menu via + `marine_colormap::palette_names()`. + +5. **Accept the canonical colour shift.** The viridis/turbo change is intended: it + aligns camp with the published tables and with rviz/rqt. The parity test is + reframed accordingly (see Consequences). + +## Consequences + +- **`bake_lut` TransferParams MUST stay identity.** Any future use of gain, contrast, + alpha-ramp or below/no-data must go through the shader (or a deliberate shader + rewrite), not by baking them into this LUT — otherwise the range the shader applies + is double-counted. The colorbar/range UI (camp#142) must respect this split. +- **viridis/turbo renders shift.** Existing screenshots/goldens of camp depth or + backscatter renders change. The colormap test is reframed to match: **grayscale** + is asserted exact (it matches), while **viridis/turbo** are a **golden/ + characterization snapshot of the new canonical `bake_lut` output** — not a + comparison against the retired sparse ramp. The old "locks in render equivalence" + framing is dropped: there is no equivalence to lock, by design. +- **Dependency surface.** `marine_colormap` is a new ``; it links **PRIVATE** + to `camp_map` (no marine_colormap type appears in any installed camp_map header — + the colormap is a `std::string` name) and is added to `camp_map_ros`'s + `ament_target_dependencies` for the `GridMap`/`SonarLiveCacheLayer` consumers. +- **camp#63 reconciliation.** The "camp-internal `ColorMap`" facility described in + camp#63 is retired; this ADR is its successor record. `color_map.{h,cpp}` are + deleted. +- **Registry is name-keyed and append-only.** Persisting the palette *name* (not a + registry index) means new marine_colormap palettes can be added without remapping + camp's saved selections. diff --git a/package.xml b/package.xml index 0f02c2d..1fb0270 100644 --- a/package.xml +++ b/package.xml @@ -23,6 +23,7 @@ nav2_msgs pluginlib marine_autonomy + marine_colormap marine_interfaces marine_tiled_raster_store marine_nav_interfaces diff --git a/src/camp_map/map/color_map.cpp b/src/camp_map/map/color_map.cpp deleted file mode 100644 index acd12d3..0000000 --- a/src/camp_map/map/color_map.cpp +++ /dev/null @@ -1,151 +0,0 @@ -#include "color_map.h" - -#include // std::min / std::max in colorNormalized() -#include -#include -#include - -namespace camp -{ -namespace map -{ - -namespace -{ - -// A control point in a colour ramp: normalised position [0,1] and its RGB (0-255). -struct Stop -{ - double t; - int r, g, b; -}; - -// Viridis (matplotlib) sampled at 7 stops — perceptually uniform. -const std::vector& viridisStops() -{ - static const std::vector stops = { - {0.000, 68, 1, 84}, - {0.167, 65, 68, 135}, - {0.333, 42, 120, 142}, - {0.500, 34, 168, 132}, - {0.667, 122, 209, 81}, - {0.833, 189, 223, 38}, - {1.000, 253, 231, 37}, - }; - return stops; -} - -// Turbo (Google) sampled at 8 stops — high-contrast, improved jet. -const std::vector& turboStops() -{ - static const std::vector stops = { - {0.000, 48, 18, 59}, - {0.143, 64, 116, 240}, - {0.286, 30, 184, 199}, - {0.429, 76, 220, 99}, - {0.571, 191, 211, 47}, - {0.714, 251, 162, 44}, - {0.857, 230, 79, 21}, - {1.000, 122, 4, 3}, - }; - return stops; -} - -int lerp(int a, int b, double f) -{ - return static_cast(std::lround(a + (b - a) * f)); -} - -QColor sampleStops(const std::vector& stops, double t) -{ - if(t <= stops.front().t) - return QColor(stops.front().r, stops.front().g, stops.front().b); - if(t >= stops.back().t) - return QColor(stops.back().r, stops.back().g, stops.back().b); - for(size_t i = 1; i < stops.size(); ++i) - { - if(t <= stops[i].t) - { - const Stop& lo = stops[i - 1]; - const Stop& hi = stops[i]; - const double span = hi.t - lo.t; - const double f = span > 0.0 ? (t - lo.t) / span : 0.0; - return QColor(lerp(lo.r, hi.r, f), lerp(lo.g, hi.g, f), lerp(lo.b, hi.b, f)); - } - } - return QColor(stops.back().r, stops.back().g, stops.back().b); -} - -} // namespace - -ColorMap::ColorMap(Type type): type_(type) -{ -} - -ColorMap::Type ColorMap::type() const -{ - return type_; -} - -void ColorMap::setType(Type type) -{ - type_ = type; -} - -QColor ColorMap::color(double value, double min, double max) const -{ - if(std::isnan(value) || !(max > min)) - return QColor(0, 0, 0, 0); - return colorNormalized((value - min) / (max - min)); -} - -QColor ColorMap::colorNormalized(double t) const -{ - if(std::isnan(t)) - return QColor(0, 0, 0, 0); - t = std::min(1.0, std::max(0.0, t)); - switch(type_) - { - case Viridis: - return sampleStops(viridisStops(), t); - case Turbo: - return sampleStops(turboStops(), t); - case Grayscale: - default: - { - const int v = static_cast(std::lround(t * 255.0)); - return QColor(v, v, v); - } - } -} - -QString ColorMap::name(Type type) -{ - switch(type) - { - case Viridis: return QStringLiteral("viridis"); - case Turbo: return QStringLiteral("turbo"); - case Grayscale: - default: return QStringLiteral("grayscale"); - } -} - -ColorMap::Type ColorMap::typeFromName(const QString& name, bool* ok) -{ - if(ok) - *ok = true; - if(name == QStringLiteral("viridis")) return Viridis; - if(name == QStringLiteral("turbo")) return Turbo; - if(name == QStringLiteral("grayscale")) return Grayscale; - if(ok) - *ok = false; - return Grayscale; -} - -QList ColorMap::allTypes() -{ - return {Grayscale, Viridis, Turbo}; -} - -} // namespace map -} // namespace camp diff --git a/src/camp_map/map/color_map.h b/src/camp_map/map/color_map.h deleted file mode 100644 index 3c0006c..0000000 --- a/src/camp_map/map/color_map.h +++ /dev/null @@ -1,53 +0,0 @@ -#ifndef MAP_COLOR_MAP_H -#define MAP_COLOR_MAP_H - -#include -#include -#include - -namespace camp -{ -namespace map -{ - -/// Maps a scalar value to a QColor through a named, perceptually-reasonable -/// colour ramp. A single reusable facility for camp's scalar-field renderers -/// (grid_map, depth shading) so each subsystem stops reimplementing its own -/// value->colour ramp. See camp#63. -class ColorMap -{ -public: - enum Type - { - Grayscale, ///< linear black -> white - Viridis, ///< perceptually-uniform purple -> yellow - Turbo ///< high-contrast blue -> red rainbow (improved jet) - }; - - explicit ColorMap(Type type = Grayscale); - - Type type() const; - void setType(Type type); - - /// Map @p value within [@p min, @p max] to a colour. The value is clamped to - /// the range. A NaN value, or a degenerate range (max <= min), yields a fully - /// transparent colour so callers can treat "no data" uniformly. - QColor color(double value, double min, double max) const; - - /// Map an already-normalised value in [0,1] (clamped) to a colour. - /// NaN yields a fully transparent colour. - QColor colorNormalized(double t) const; - - /// Display name <-> Type, for UI population and settings persistence. - static QString name(Type type); - static Type typeFromName(const QString& name, bool* ok = nullptr); - static QList allTypes(); - -private: - Type type_; -}; - -} // namespace map -} // namespace camp - -#endif diff --git a/src/camp_map/raster/gggs_tile_layer.cpp b/src/camp_map/raster/gggs_tile_layer.cpp index 412afea..d5fc091 100644 --- a/src/camp_map/raster/gggs_tile_layer.cpp +++ b/src/camp_map/raster/gggs_tile_layer.cpp @@ -4,6 +4,8 @@ #include "gggs_tile_util.h" #include "../map_view/web_mercator.h" +#include + #include #include #include @@ -459,11 +461,11 @@ void GggsTileLayer::paint(QPainter* painter, const QStyleOptionGraphicsItem*, QW painter->restore(); } -void GggsTileLayer::setColormap(map::ColorMap::Type type) +void GggsTileLayer::setColormap(const std::string& name) { - if(type == renderer_.colormap()) + if(name == renderer_.colormap()) return; - renderer_.setColormap(type); // re-bakes the LUT on next render + renderer_.setColormap(name); // re-bakes the LUT on next render cached_image_ = QImage(); // force a re-render with the new ramp writeSettings(); update(boundingRect()); @@ -599,13 +601,15 @@ void GggsTileLayer::contextMenu(QMenu* menu) QAction* rescan_action = menu->addAction("Rescan"); connect(rescan_action, &QAction::triggered, this, [this]() { rescan(); }); + // [camp#141] Expose the FULL marine_colormap registry (grayscale/bronze/thermal/ + // viridis/turbo/quality), not just the three legacy ramps. QMenu* colormap_menu = menu->addMenu("Colormap"); - for(auto type : map::ColorMap::allTypes()) + for(const std::string& name : marine_colormap::palette_names()) { - QAction* action = colormap_menu->addAction(map::ColorMap::name(type)); + QAction* action = colormap_menu->addAction(QString::fromStdString(name)); action->setCheckable(true); - action->setChecked(type == renderer_.colormap()); - connect(action, &QAction::triggered, this, [this, type]() { setColormap(type); }); + action->setChecked(name == renderer_.colormap()); + connect(action, &QAction::triggered, this, [this, name]() { setColormap(name); }); } // [camp#108] Band picker — only for multi-band tile-sets (bathy depth + @@ -658,8 +662,13 @@ void GggsTileLayer::readSettings() // `visible` value still wins (the operator's on/off choice round-trips); only // the first-run default flips. setVisible(settings.value("visible", false).toBool()); - const map::ColorMap::Type type = map::ColorMap::typeFromName( - settings.value("colormap", map::ColorMap::name(renderer_.colormap())).toString()); + // [camp#141] Persisted palette name. Read case-insensitively (camp already stores + // lowercase, but tolerate a legacy capitalized "Viridis"/"Turbo") and validate + // against the marine_colormap registry; an unknown name falls back to grayscale. + std::string colormap = settings.value( + "colormap", QString::fromStdString(renderer_.colormap())).toString().toLower().toStdString(); + if(!marine_colormap::palette_index(colormap)) + colormap = "grayscale"; // [camp#108] Persisted band (default 1). Applied via applyBand() below — the // non-persisting band switch (texture release + reload + range reset) — so the // read path does NOT write the value straight back out (setBand() would). Only @@ -668,9 +677,9 @@ void GggsTileLayer::readSettings() const int band = settings.value("band", 1).toInt(); settings.endGroup(); settings.endGroup(); - if(type != renderer_.colormap()) + if(colormap != renderer_.colormap()) { - renderer_.setColormap(type); + renderer_.setColormap(colormap); cached_image_ = QImage(); } if(band != band_) @@ -683,7 +692,7 @@ void GggsTileLayer::writeSettings() QSettings settings; settings.beginGroup("MapItem"); settings.beginGroup(settingsKey()); - settings.setValue("colormap", map::ColorMap::name(renderer_.colormap())); + settings.setValue("colormap", QString::fromStdString(renderer_.colormap())); settings.setValue("band", band_); // [camp#108] selected band round-trips settings.endGroup(); settings.endGroup(); diff --git a/src/camp_map/raster/gggs_tile_layer.h b/src/camp_map/raster/gggs_tile_layer.h index c88fb83..9996092 100644 --- a/src/camp_map/raster/gggs_tile_layer.h +++ b/src/camp_map/raster/gggs_tile_layer.h @@ -2,7 +2,6 @@ #define RASTER_GGGS_TILE_LAYER_H #include "../map/layer.h" -#include "../map/color_map.h" #include "raster_field_source.h" #include "raster_gl_renderer.h" @@ -11,6 +10,7 @@ #include #include #include +#include #include namespace camp @@ -77,9 +77,9 @@ class GggsTileLayer: public map::Layer, public RasterFieldSource /// without a window. QImage renderImage(const QSize& size); - /// [camp#90] Select the colour ramp (the shared camp::map::ColorMap, baked to - /// a GPU LUT). Persists and re-renders. - void setColormap(map::ColorMap::Type type); + /// [camp#90 / camp#141] Select the colour ramp by marine_colormap palette name + /// (baked to a GPU LUT). Persists and re-renders. Unknown name -> grayscale. + void setColormap(const std::string& name); /// [camp#108] The 1-indexed band the tiles render (default 1). int band() const { return band_; } diff --git a/src/camp_map/raster/raster_gl_renderer.cpp b/src/camp_map/raster/raster_gl_renderer.cpp index 7466805..88ddd31 100644 --- a/src/camp_map/raster/raster_gl_renderer.cpp +++ b/src/camp_map/raster/raster_gl_renderer.cpp @@ -2,6 +2,10 @@ #include "../map_view/web_mercator.h" +#include +#include +#include + #include #include #include @@ -74,7 +78,8 @@ void main() discard; // NaN NoData (1.20-portable) if(u_has_nodata != 0 && v == u_nodata) discard; // finite sentinel - // Normalize over the TRUE data span (matches ColorMap::color), so sub-unit + // Normalize over the TRUE data span (the per-band range step, kept in the + // shader — the baked LUT uses identity TransferParams, see ADR-0008), so sub-unit // ranges still stretch across the colormap. The 1e-6 floor is ONLY a // divide-by-zero guard for a genuinely degenerate (zero-width) range — a true // span of 0 collapses to t=0 (a flat LUT value). The old 1.0 floor silently @@ -171,18 +176,27 @@ bool RasterGlRenderer::ensureProgram() QOpenGLTexture* RasterGlRenderer::ensureLut() { - // Bake camp::map::ColorMap into a 256x1 RGBA LUT (re-baked when the ramp - // changes). Sampled by the fragment shader as the colour transfer. + // [camp#141] Bake the selected marine_colormap palette into a 256x1 RGBA LUT + // (re-baked when the ramp changes), sampled by the fragment shader as the colour + // transfer. TransferParams stays IDENTITY: the shader still owns range-normalize + // (per-band u_min/u_max over the true span) and the NaN/finite-NoData discard, so + // the LUT carries only the palette ramp — exactly what the old colorNormalized + // loop produced (see ADR-0008). An unknown name falls back to grayscale. if(lut_texture_ && !lut_dirty_) return lut_texture_.get(); + const marine_colormap::Palette* palette = marine_colormap::find_palette(colormap_name_); + if(!palette) + palette = marine_colormap::find_palette("grayscale"); + const std::vector baked = + marine_colormap::bake_lut(*palette, marine_colormap::TransferParams{}, 256); std::vector lut(256 * 4); for(int i = 0; i < 256; ++i) { - const QColor c = colormap_.colorNormalized(i / 255.0); - lut[i * 4 + 0] = uchar(c.red()); - lut[i * 4 + 1] = uchar(c.green()); - lut[i * 4 + 2] = uchar(c.blue()); - lut[i * 4 + 3] = uchar(c.alpha()); + const marine_colormap::Rgba8& c = baked[static_cast(i)]; + lut[i * 4 + 0] = c.r; + lut[i * 4 + 1] = c.g; + lut[i * 4 + 2] = c.b; + lut[i * 4 + 3] = c.a; } if(!lut_texture_) { @@ -199,11 +213,11 @@ QOpenGLTexture* RasterGlRenderer::ensureLut() return lut_texture_.get(); } -void RasterGlRenderer::setColormap(map::ColorMap::Type type) +void RasterGlRenderer::setColormap(const std::string& name) { - if(type == colormap_.type()) + if(name == colormap_name_) return; - colormap_.setType(type); + colormap_name_ = name; lut_dirty_ = true; } diff --git a/src/camp_map/raster/raster_gl_renderer.h b/src/camp_map/raster/raster_gl_renderer.h index a5339d6..745f5a8 100644 --- a/src/camp_map/raster/raster_gl_renderer.h +++ b/src/camp_map/raster/raster_gl_renderer.h @@ -2,13 +2,13 @@ #define RASTER_RASTER_GL_RENDERER_H #include "raster_field_source.h" -#include "../map/color_map.h" #include #include #include #include #include +#include class QOpenGLContext; class QOffscreenSurface; @@ -67,9 +67,10 @@ class RasterGlRenderer const QRectF& scene_bounds, float data_min, float data_max, const QSize& size); - /// Select the colour ramp baked into the LUT (re-baked on next render). - void setColormap(map::ColorMap::Type type); - map::ColorMap::Type colormap() const { return colormap_.type(); } + /// [camp#141] Select the marine_colormap palette baked into the LUT, by name + /// (re-baked on next render). An unknown name renders as "grayscale". + void setColormap(const std::string& name); + const std::string& colormap() const { return colormap_name_; } /// Release the FBO / program / LUT. Safe to call with no context; the dtor /// makes the context current first and then destroys it. @@ -91,7 +92,7 @@ class RasterGlRenderer std::unique_ptr program_; std::unique_ptr lut_texture_; // colormap LUT (256x1 RGBA) - map::ColorMap colormap_{map::ColorMap::Grayscale}; + std::string colormap_name_{"grayscale"}; // [camp#141] marine_colormap palette bool lut_dirty_ = true; bool gl_failed_ = false; }; diff --git a/src/camp_map/raster/raster_layer.cpp b/src/camp_map/raster/raster_layer.cpp index 5dd97ad..adc504e 100644 --- a/src/camp_map/raster/raster_layer.cpp +++ b/src/camp_map/raster/raster_layer.cpp @@ -2,6 +2,7 @@ #include #include #include "../map_view/web_mercator.h" +#include #include #include #include @@ -29,8 +30,8 @@ RasterLayer::RasterLayer(map::MapItem* parentItem, const QString& filename): { if(GDALGetDriverCount() == 0) GDALAllRegister(); - // [camp#63] Default scalar ramp is Viridis (the renderer defaults to Grayscale). - renderer_.setColormap(map::ColorMap::Viridis); + // [camp#63] Default scalar ramp is viridis (the renderer defaults to grayscale). + renderer_.setColormap("viridis"); connect(&future_watcher_, &QFutureWatcher::finished, this, &RasterLayer::imageReady); // [#59 ADR-0003] Establish the scene extent + world transform synchronously, // before kicking off the async pixel load, so the layer knows where it is @@ -460,14 +461,14 @@ QList RasterLayer::items() return result; } -void RasterLayer::setColormap(map::ColorMap::Type type) +void RasterLayer::setColormap(const std::string& name) { - if(type == renderer_.colormap()) + if(name == renderer_.colormap()) return; // [camp#134] A colormap change is now just an LUT re-bake — no re-warp. The // scalar value texture already holds the raw data, so the shader recolours it on // the next render. - renderer_.setColormap(type); + renderer_.setColormap(name); writeSettings(); cached_image_ = QImage(); update(boundingRect()); @@ -478,13 +479,14 @@ void RasterLayer::contextMenu(QMenu* menu) map::Layer::contextMenu(menu); if(!is_scalar_) // colormap only applies to scalar (depth) rasters return; + // [camp#141] Expose the FULL marine_colormap registry, not just the legacy ramps. QMenu* colormap_menu = menu->addMenu("Colormap"); - for(auto type : map::ColorMap::allTypes()) + for(const std::string& name : marine_colormap::palette_names()) { - QAction* action = colormap_menu->addAction(map::ColorMap::name(type)); + QAction* action = colormap_menu->addAction(QString::fromStdString(name)); action->setCheckable(true); - action->setChecked(type == renderer_.colormap()); - connect(action, &QAction::triggered, this, [this, type]() { setColormap(type); }); + action->setChecked(name == renderer_.colormap()); + connect(action, &QAction::triggered, this, [this, name]() { setColormap(name); }); } } @@ -494,14 +496,18 @@ void RasterLayer::readSettings() QSettings settings; settings.beginGroup("MapItem"); settings.beginGroup(itemID()); - const map::ColorMap::Type type = map::ColorMap::typeFromName( - settings.value("colormap", map::ColorMap::name(renderer_.colormap())).toString()); + // [camp#141] Persisted palette name; case-insensitive read + registry-validated + // (unknown -> grayscale). + std::string colormap = settings.value( + "colormap", QString::fromStdString(renderer_.colormap())).toString().toLower().toStdString(); + if(!marine_colormap::palette_index(colormap)) + colormap = "grayscale"; settings.endGroup(); settings.endGroup(); // Apply the persisted ramp (re-bake + re-render if it differs); don't re-persist. - if(type != renderer_.colormap()) + if(colormap != renderer_.colormap()) { - renderer_.setColormap(type); + renderer_.setColormap(colormap); cached_image_ = QImage(); update(boundingRect()); } @@ -523,7 +529,7 @@ void RasterLayer::writeSettings() QSettings settings; settings.beginGroup("MapItem"); settings.beginGroup(itemID()); - settings.setValue("colormap", map::ColorMap::name(renderer_.colormap())); + settings.setValue("colormap", QString::fromStdString(renderer_.colormap())); settings.endGroup(); settings.endGroup(); } diff --git a/src/camp_map/raster/raster_layer.h b/src/camp_map/raster/raster_layer.h index 5685ac6..4224a5c 100644 --- a/src/camp_map/raster/raster_layer.h +++ b/src/camp_map/raster/raster_layer.h @@ -2,7 +2,6 @@ #define RASTER_RASTER_LAYER_H #include "../map/layer.h" -#include "../map/color_map.h" #include "raster_field_source.h" #include "raster_gl_renderer.h" @@ -11,6 +10,7 @@ #include #include #include +#include #include class QOpenGLTexture; @@ -46,10 +46,10 @@ class RasterLayer: public map::Layer, public RasterFieldSource QRectF boundingRect() const override; void paint(QPainter *painter, const QStyleOptionGraphicsItem *option, QWidget *widget) override; - /// [camp#63] Select the colour ramp for a scalar (single-band Float32) raster; - /// persists and re-renders (just re-bakes the LUT — no re-warp). No effect on - /// RGB rasters. - void setColormap(map::ColorMap::Type type); + /// [camp#63 / camp#141] Select the colour ramp for a scalar (single-band Float32) + /// raster by marine_colormap palette name; persists and re-renders (just re-bakes + /// the LUT — no re-warp). No effect on RGB rasters. Unknown name -> grayscale. + void setColormap(const std::string& name); /// [#59 ADR-0003] Source file this layer renders (kept for re-render, removal, /// and app-state persistence of the loaded-chart list). diff --git a/src/camp_map/ros/grids/grid_map.cpp b/src/camp_map/ros/grids/grid_map.cpp index 839c3fe..dbca3bf 100644 --- a/src/camp_map/ros/grids/grid_map.cpp +++ b/src/camp_map/ros/grids/grid_map.cpp @@ -6,6 +6,9 @@ #include "marine_autonomy/gz4d_geo.h" #include #include "grid_layer.h" +#include +#include +#include #include #include #include @@ -51,7 +54,7 @@ void GridMap::startRenderLocked() // Snapshot the inputs under the lock so the worker is fully isolated from // mutex_-guarded state. rendering_ = true; - process_future_ = QtConcurrent::run(this, &GridMap::processGridMap, last_msg_, colormap_); + process_future_ = QtConcurrent::run(this, &GridMap::processGridMap, last_msg_, colormap_name_); } void GridMap::requestRenderLocked() @@ -82,17 +85,21 @@ void GridMap::gridMapCallback(const grid_map_msgs::msg::GridMap &data) requestRenderLocked(); } -void GridMap::processGridMap(grid_map_msgs::msg::GridMap data, map::ColorMap colormap) +void GridMap::processGridMap(grid_map_msgs::msg::GridMap data, std::string colormap_name) { GridMapData grid_data; - if(renderToData(data, colormap, grid_data)) + if(renderToData(data, colormap_name, grid_data)) emit newGridData(grid_data); onProcessFinished(); } bool GridMap::renderToData(const grid_map_msgs::msg::GridMap &data, - const map::ColorMap &colormap, GridMapData &grid_data) + const std::string &colormap_name, GridMapData &grid_data) { + // [camp#141] Resolve the palette once for the whole message; unknown -> grayscale. + const marine_colormap::Palette* palette = marine_colormap::find_palette(colormap_name); + if(!palette) + palette = marine_colormap::find_palette("grayscale"); grid_map::GridMap grid_map; auto node = node_->node(); rclcpp::Clock clock; @@ -152,11 +159,15 @@ bool GridMap::renderToData(const grid_map_msgs::msg::GridMap &data, double value = grid_map.at(layer, *iterator); if(!std::isnan(value)) { - // [camp#63] Normalised value -> colour via the selectable ramp - // (colorNormalized clamps to [0,1]); default grayscale reproduces the - // prior output. + // [camp#63 / camp#141] Normalised value -> colour via the selected + // marine_colormap palette (Palette::sample clamps to [0,1]); invalid + // (NaN) cells are left transparent by the fill above. Default grayscale. value = (value - min_value) / (max_value - min_value); - grid_layer_data.grid_image.setPixelColor(QPoint(size.x()-1-iterator.getUnwrappedIndex().x(), iterator.getUnwrappedIndex().y()), colormap.colorNormalized(value)); + const marine_colormap::Rgba8 c = + marine_colormap::to_rgba8(palette->sample(static_cast(value))); + grid_layer_data.grid_image.setPixelColor( + QPoint(size.x()-1-iterator.getUnwrappedIndex().x(), iterator.getUnwrappedIndex().y()), + QColor(c.r, c.g, c.b, c.a)); } } } @@ -195,13 +206,13 @@ void GridMap::updateGridLayer(const GridMapLayerData& data) layer->updateGridLayer(data); } -void GridMap::setColormap(map::ColorMap::Type type) +void GridMap::setColormap(const std::string& name) { { QMutexLocker lock(&mutex_); - if(type == colormap_.type()) + if(name == colormap_name_) return; - colormap_.setType(type); + colormap_name_ = name; if(has_last_msg_) // re-render the cached grid with the new ramp requestRenderLocked(); // coalesced: not lost even if a render is in flight } @@ -211,13 +222,19 @@ void GridMap::setColormap(map::ColorMap::Type type) void GridMap::contextMenu(QMenu* menu) { Layer::contextMenu(menu); + // [camp#141] Expose the FULL marine_colormap registry, not just the legacy ramps. + std::string current; + { + QMutexLocker lock(&mutex_); + current = colormap_name_; + } QMenu* colormap_menu = menu->addMenu("Colormap"); - for(auto type : map::ColorMap::allTypes()) + for(const std::string& name : marine_colormap::palette_names()) { - QAction* action = colormap_menu->addAction(map::ColorMap::name(type)); + QAction* action = colormap_menu->addAction(QString::fromStdString(name)); action->setCheckable(true); - action->setChecked(type == colormap_.type()); - connect(action, &QAction::triggered, this, [this, type]() { setColormap(type); }); + action->setChecked(name == current); + connect(action, &QAction::triggered, this, [this, name]() { setColormap(name); }); } } @@ -227,12 +244,18 @@ void GridMap::readSettings() QSettings settings; settings.beginGroup("MapItem"); settings.beginGroup(itemID()); - auto type = map::ColorMap::typeFromName( - settings.value("colormap", map::ColorMap::name(colormap_.type())).toString()); + // [camp#141] Persisted palette name; case-insensitive read + registry-validated + // (unknown -> grayscale). + // Literal default avoids reading colormap_name_ without holding mutex_; the + // registry-validation below already falls back to "grayscale" for unknowns. + std::string name = settings.value( + "colormap", "grayscale").toString().toLower().toStdString(); + if(!marine_colormap::palette_index(name)) + name = "grayscale"; settings.endGroup(); settings.endGroup(); QMutexLocker lock(&mutex_); - colormap_.setType(type); + colormap_name_ = name; } void GridMap::writeSettings() @@ -241,7 +264,12 @@ void GridMap::writeSettings() QSettings settings; settings.beginGroup("MapItem"); settings.beginGroup(itemID()); - settings.setValue("colormap", map::ColorMap::name(colormap_.type())); + std::string name; + { + QMutexLocker lock(&mutex_); // snapshot the name; QSettings I/O stays unlocked + name = colormap_name_; + } + settings.setValue("colormap", QString::fromStdString(name)); settings.endGroup(); settings.endGroup(); } diff --git a/src/camp_map/ros/grids/grid_map.h b/src/camp_map/ros/grids/grid_map.h index 2217e03..b9eef43 100644 --- a/src/camp_map/ros/grids/grid_map.h +++ b/src/camp_map/ros/grids/grid_map.h @@ -2,7 +2,6 @@ #define CAMP_ROS_GRIDS_GRID_MAP_H #include "../layer.h" -#include "../../map/color_map.h" #include "grid_map_msgs/msg/grid_map.hpp" #include #include @@ -44,9 +43,10 @@ class GridMap: public Layer GridMap(MapItem* parent, Node* node, QString topic); ~GridMap() override; // joins the in-flight render worker before teardown - /// [camp#63] Select the colour ramp for this layer; persists and re-renders - /// the last received grid. - void setColormap(map::ColorMap::Type type); + /// [camp#63 / camp#141] Select the colour ramp for this layer by marine_colormap + /// palette name; persists and re-renders the last received grid. Unknown name -> + /// grayscale. + void setColormap(const std::string& name); protected: void contextMenu(QMenu* menu) override; @@ -60,12 +60,12 @@ class GridMap: public Layer void gridMapCallback(const grid_map_msgs::msg::GridMap &data); // Worker body (runs on a QtConcurrent thread). Takes its inputs by value so - // it never touches mutex_-guarded state: the message and colormap are + // it never touches mutex_-guarded state: the message and colormap name are // snapshotted under the lock at launch. - void processGridMap(grid_map_msgs::msg::GridMap data, map::ColorMap colormap); - // Renders `data` with `colormap` into `out`; returns false (no emit) when the - // message can't be converted, has no layers, or has no earth transform. - bool renderToData(const grid_map_msgs::msg::GridMap &data, const map::ColorMap &colormap, + void processGridMap(grid_map_msgs::msg::GridMap data, std::string colormap_name); + // Renders `data` with the `colormap_name` palette into `out`; returns false (no + // emit) when the message can't be converted, has no layers, or has no transform. + bool renderToData(const grid_map_msgs::msg::GridMap &data, const std::string &colormap_name, GridMapData &out); // Render-scheduling helpers. *Locked variants assume mutex_ is held. Only one @@ -95,9 +95,9 @@ private slots: bool render_pending_ = false; // a request arrived while rendering_; render once more bool shutdown_ = false; // set by the dtor so the worker stops relaunching - // [camp#63] Colour ramp applied to the normalised grid values. Default - // grayscale (the camp_map post-#59 default); selectable per layer. - map::ColorMap colormap_; + // [camp#63 / camp#141] marine_colormap palette applied to the normalised grid + // values. Default grayscale (the camp_map post-#59 default); selectable per layer. + std::string colormap_name_{"grayscale"}; // Last received message, cached so a colormap change can re-render without // waiting for the next publish (live costmaps would also pick it up). diff --git a/src/camp_map/ros/live_coverage/sonar_live_cache_layer.cpp b/src/camp_map/ros/live_coverage/sonar_live_cache_layer.cpp index d62ffbd..2a9dd9b 100644 --- a/src/camp_map/ros/live_coverage/sonar_live_cache_layer.cpp +++ b/src/camp_map/ros/live_coverage/sonar_live_cache_layer.cpp @@ -3,6 +3,8 @@ #include "../node.h" #include "../../map_view/web_mercator.h" +#include + #include #include #include @@ -638,11 +640,11 @@ void SonarLiveCacheLayer::paint(QPainter* painter, const QStyleOptionGraphicsIte // ------------------------------- band / colormap ----------------------------- -void SonarLiveCacheLayer::setColormap(map::ColorMap::Type type) +void SonarLiveCacheLayer::setColormap(const std::string& name) { - if(type == renderer_.colormap()) + if(name == renderer_.colormap()) return; - renderer_.setColormap(type); // re-bakes the LUT on next render + renderer_.setColormap(name); // re-bakes the LUT on next render cached_image_ = QImage(); writeSettings(); update(boundingRect()); @@ -686,13 +688,14 @@ void SonarLiveCacheLayer::contextMenu(QMenu* menu) &SonarLiveCacheLayer::enableLiveCoverage); } + // [camp#141] Expose the FULL marine_colormap registry, not just the legacy ramps. QMenu* colormap_menu = menu->addMenu("Colormap"); - for(auto type : map::ColorMap::allTypes()) + for(const std::string& name : marine_colormap::palette_names()) { - QAction* action = colormap_menu->addAction(map::ColorMap::name(type)); + QAction* action = colormap_menu->addAction(QString::fromStdString(name)); action->setCheckable(true); - action->setChecked(type == renderer_.colormap()); - connect(action, &QAction::triggered, this, [this, type]() { setColormap(type); }); + action->setChecked(name == renderer_.colormap()); + connect(action, &QAction::triggered, this, [this, name]() { setColormap(name); }); } // Band picker over the union of band names across held tiles. Only shown when @@ -726,17 +729,21 @@ void SonarLiveCacheLayer::readSettings() // Default discovered layers OFF in the tree (like GGGS tile-sets) — the operator // turns coverage on explicitly. setVisible(settings.value("visible", false).toBool()); - const map::ColorMap::Type type = map::ColorMap::typeFromName( - settings.value("colormap", map::ColorMap::name(renderer_.colormap())).toString()); + // [camp#141] Persisted palette name; case-insensitive read + registry-validated + // (unknown -> grayscale), mirroring GggsTileLayer. + std::string colormap = settings.value( + "colormap", QString::fromStdString(renderer_.colormap())).toString().toLower().toStdString(); + if(!marine_colormap::palette_index(colormap)) + colormap = "grayscale"; const std::string band = settings.value("band", QString::fromStdString(band_name_)) .toString().toStdString(); const bool was_enabled = settings.value("live_enabled", false).toBool(); settings.endGroup(); settings.endGroup(); - if(type != renderer_.colormap()) + if(colormap != renderer_.colormap()) { - renderer_.setColormap(type); + renderer_.setColormap(colormap); cached_image_ = QImage(); } if(!band.empty()) @@ -753,7 +760,7 @@ void SonarLiveCacheLayer::writeSettings() QSettings settings; settings.beginGroup("MapItem"); settings.beginGroup(settingsKey()); - settings.setValue("colormap", map::ColorMap::name(renderer_.colormap())); + settings.setValue("colormap", QString::fromStdString(renderer_.colormap())); settings.setValue("band", QString::fromStdString(band_name_)); settings.setValue("live_enabled", enabled_); settings.endGroup(); diff --git a/src/camp_map/ros/live_coverage/sonar_live_cache_layer.h b/src/camp_map/ros/live_coverage/sonar_live_cache_layer.h index 2684461..4ae9e90 100644 --- a/src/camp_map/ros/live_coverage/sonar_live_cache_layer.h +++ b/src/camp_map/ros/live_coverage/sonar_live_cache_layer.h @@ -2,7 +2,6 @@ #define CAMP_ROS_LIVE_COVERAGE_SONAR_LIVE_CACHE_LAYER_H #include "../layer.h" -#include "../../map/color_map.h" #include "../../raster/raster_field_source.h" #include "../../raster/raster_gl_renderer.h" #include "sonar_live_tile.h" @@ -150,7 +149,7 @@ private slots: // [camp#121] Band selection (by NAME, since live bands are named, unlike the // GggsTile 1-indexed bands). Default picks "depth" if present else the first. std::string defaultBand() const; - void setColormap(map::ColorMap::Type type); + void setColormap(const std::string& name); // [camp#141] marine_colormap palette void setBandName(const std::string& name); // [camp#134] (Re)upload the selected band of @p entry as an R32F value texture diff --git a/test/test_color_map.cpp b/test/test_color_map.cpp index a732487..33b4086 100644 --- a/test/test_color_map.cpp +++ b/test/test_color_map.cpp @@ -1,76 +1,143 @@ -// Unit tests for camp::map::ColorMap (camp#63). +// Characterization tests for the marine_colormap adoption (camp#141). +// +// camp's internal camp::map::ColorMap is deleted; the colormap is now a +// marine_colormap palette selected by name and baked into the GPU LUT (ADR-0008) +// or sampled on the CPU (GridMap). viridis/turbo are now the CANONICAL 256-entry +// matplotlib/Google tables — an INTENDED colour change from camp's old sparse +// 7/8-stop approximations — so these tests SNAPSHOT the new canonical bake_lut +// output rather than asserting equivalence to the retired ramp. grayscale still +// matches exactly (a plain black->white ramp). +// +// The render-path properties (NaN + finite-NoData discard, Nearest filtering, +// sub-unit range stretch) live with the GPU shader and are covered by +// test_raster_gl_renderer.cpp; here we pin the palette colours, the exposed +// registry, and the name-resolution rules the layers rely on. #include -#include -#include "map/color_map.h" -using camp::map::ColorMap; +#include -TEST(ColorMap, GrayscaleEndpointsAndMidpoint) +#include +#include + +#include +#include +#include + +using marine_colormap::Rgba8; + +namespace +{ + +// The camp colormap path bakes a 256-entry LUT with IDENTITY TransferParams (the +// per-band range stays in the shader; the LUT carries only the palette ramp — see +// RasterGlRenderer::ensureLut / ADR-0008). Reproduce that exact bake here. +std::vector bake(const std::string& name) { - ColorMap cm(ColorMap::Grayscale); - EXPECT_EQ(cm.colorNormalized(0.0), QColor(0, 0, 0)); - EXPECT_EQ(cm.colorNormalized(1.0), QColor(255, 255, 255)); - // Midpoint is a mid grey on the diagonal. - const QColor mid = cm.colorNormalized(0.5); - EXPECT_EQ(mid.red(), mid.green()); - EXPECT_EQ(mid.green(), mid.blue()); - EXPECT_NEAR(mid.red(), 128, 1); + const marine_colormap::Palette* pal = marine_colormap::find_palette(name); + EXPECT_NE(pal, nullptr) << "palette not found: " << name; + return marine_colormap::bake_lut(*pal, marine_colormap::TransferParams{}, 256); } -TEST(ColorMap, RangeMapsToNormalized) +// Mirrors the case-insensitive, registry-validated settings read every migrated +// layer performs: lowercase the stored string, validate against the registry, and +// fall back to grayscale for an unknown name. +std::string canonicalName(const QString& stored) { - ColorMap cm(ColorMap::Grayscale); - // value at min -> 0, at max -> 1, halfway -> 0.5. - EXPECT_EQ(cm.color(10.0, 10.0, 20.0), cm.colorNormalized(0.0)); - EXPECT_EQ(cm.color(20.0, 10.0, 20.0), cm.colorNormalized(1.0)); - EXPECT_EQ(cm.color(15.0, 10.0, 20.0), cm.colorNormalized(0.5)); + std::string name = stored.toLower().toStdString(); + if(!marine_colormap::palette_index(name)) + name = "grayscale"; + return name; } -TEST(ColorMap, ClampsOutOfRange) +} // namespace + +// The colormap context menus are built from palette_names(): pin the full exposed +// registry so a palette add/rename/reorder is a deliberate, visible change. +TEST(Colormap, RegistryIsTheFullSix) { - ColorMap cm(ColorMap::Turbo); - // Below min clamps to the t=0 colour; above max clamps to the t=1 colour. - EXPECT_EQ(cm.color(-5.0, 0.0, 10.0), cm.colorNormalized(0.0)); - EXPECT_EQ(cm.color(99.0, 0.0, 10.0), cm.colorNormalized(1.0)); + const std::vector expected = { + "grayscale", "bronze", "thermal", "viridis", "turbo", "quality"}; + EXPECT_EQ(marine_colormap::palette_names(), expected); } -TEST(ColorMap, NanAndDegenerateRangeAreTransparent) +// grayscale is a plain black->white ramp: it matches camp's old grayscale exactly. +TEST(Colormap, GrayscaleMatchesExactly) { - ColorMap cm(ColorMap::Viridis); - EXPECT_EQ(cm.colorNormalized(std::nan("")).alpha(), 0); - EXPECT_EQ(cm.color(std::nan(""), 0.0, 1.0).alpha(), 0); - // max <= min is degenerate -> transparent (no division-by-zero colour). - EXPECT_EQ(cm.color(5.0, 10.0, 10.0).alpha(), 0); - EXPECT_EQ(cm.color(5.0, 10.0, 1.0).alpha(), 0); + const std::vector lut = bake("grayscale"); + EXPECT_EQ(lut.front().r, 0); EXPECT_EQ(lut.front().g, 0); EXPECT_EQ(lut.front().b, 0); + EXPECT_EQ(lut.back().r, 255); EXPECT_EQ(lut.back().g, 255); EXPECT_EQ(lut.back().b, 255); + // Mid LUT entry is a neutral grey on the diagonal. + const Rgba8 mid = lut[128]; + EXPECT_EQ(mid.r, mid.g); + EXPECT_EQ(mid.g, mid.b); + EXPECT_NEAR(mid.r, 128, 1); } -TEST(ColorMap, OpaqueColoursInRange) +// viridis: snapshot the NEW canonical table (endpoints exact, midpoint ~1 LSB). +TEST(Colormap, ViridisIsCanonical) { - for(auto type : ColorMap::allTypes()) - { - ColorMap cm(type); - EXPECT_EQ(cm.colorNormalized(0.0).alpha(), 255); - EXPECT_EQ(cm.colorNormalized(0.5).alpha(), 255); - EXPECT_EQ(cm.colorNormalized(1.0).alpha(), 255); - } + const std::vector lut = bake("viridis"); + EXPECT_EQ(lut.front().r, 68); EXPECT_EQ(lut.front().g, 1); EXPECT_EQ(lut.front().b, 84); + EXPECT_EQ(lut.back().r, 253); EXPECT_EQ(lut.back().g, 231); EXPECT_EQ(lut.back().b, 37); + const Rgba8 mid = lut[128]; + EXPECT_NEAR(mid.r, 33, 1); + EXPECT_NEAR(mid.g, 145, 1); + EXPECT_NEAR(mid.b, 140, 1); + // The canonical midpoint is a teal green (~145), NOT camp's old sparse-ramp green + // (~168): this guards that the canonical table — not the retired approximation — + // is in use. An intended change, documented in ADR-0008. + EXPECT_LT(mid.g, 160); } -TEST(ColorMap, NamedRampsDifferFromGrayscale) +// turbo: snapshot the NEW canonical table. +TEST(Colormap, TurboIsCanonical) { - // A perceptual ramp's midpoint is not a neutral grey. - const QColor v = ColorMap(ColorMap::Viridis).colorNormalized(0.5); - EXPECT_FALSE(v.red() == v.green() && v.green() == v.blue()); + const std::vector lut = bake("turbo"); + EXPECT_EQ(lut.front().r, 48); EXPECT_EQ(lut.front().g, 18); EXPECT_EQ(lut.front().b, 59); + EXPECT_EQ(lut.back().r, 122); EXPECT_EQ(lut.back().g, 4); EXPECT_EQ(lut.back().b, 3); + const Rgba8 mid = lut[128]; + EXPECT_NEAR(mid.r, 164, 1); + EXPECT_NEAR(mid.g, 252, 1); + EXPECT_NEAR(mid.b, 60, 1); } -TEST(ColorMap, NameRoundTrip) +// Every palette is fully opaque across its in-range entries (no accidental +// transparency in the LUT; transparency is a render-path NoData concern, not a +// palette one). +TEST(Colormap, AllPalettesOpaqueInRange) { - for(auto type : ColorMap::allTypes()) + for(const std::string& name : marine_colormap::palette_names()) { - bool ok = false; - EXPECT_EQ(ColorMap::typeFromName(ColorMap::name(type), &ok), type); - EXPECT_TRUE(ok); + const std::vector lut = bake(name); + EXPECT_EQ(lut.front().a, 255) << name; + EXPECT_EQ(lut[128].a, 255) << name; + EXPECT_EQ(lut.back().a, 255) << name; } - bool ok = true; - EXPECT_EQ(ColorMap::typeFromName("nope", &ok), ColorMap::Grayscale); - EXPECT_FALSE(ok); +} + +// palette_names() round-trips through the name<->index<->palette registry, and an +// unknown name resolves to nullptr/nullopt (the layers fall back to grayscale). +TEST(Colormap, NameRoundTrip) +{ + for(const std::string& name : marine_colormap::palette_names()) + { + const auto index = marine_colormap::palette_index(name); + ASSERT_TRUE(index.has_value()) << name; + EXPECT_EQ(marine_colormap::palette(*index).name(), name); + EXPECT_NE(marine_colormap::find_palette(name), nullptr) << name; + } + EXPECT_FALSE(marine_colormap::palette_index("nope").has_value()); + EXPECT_EQ(marine_colormap::find_palette("nope"), nullptr); +} + +// The persisted-name migration rule the layers apply: camp stores lowercase, but a +// legacy capitalized "Viridis"/"Turbo" still resolves; an unknown name -> grayscale. +TEST(Colormap, CaseInsensitiveSettingsFallback) +{ + EXPECT_EQ(canonicalName("viridis"), "viridis"); + EXPECT_EQ(canonicalName("Viridis"), "viridis"); + EXPECT_EQ(canonicalName("TURBO"), "turbo"); + EXPECT_EQ(canonicalName("grayscale"), "grayscale"); + EXPECT_EQ(canonicalName("nope"), "grayscale"); + EXPECT_EQ(canonicalName(""), "grayscale"); } diff --git a/test/test_gggs_persistence.cpp b/test/test_gggs_persistence.cpp index f32238a..cfa42ec 100644 --- a/test/test_gggs_persistence.cpp +++ b/test/test_gggs_persistence.cpp @@ -322,7 +322,7 @@ TEST(GggsPersistence, SameDisplayNameDistinctPersistence) // Give A non-default visible/colormap/band; leave B at its defaults; persist. ASSERT_EQ(a->bandCount(), 2); a->setVisible(true); - a->setColormap(camp::map::ColorMap::Viridis); + a->setColormap("viridis"); a->setBand(2); a->writeSettings(); diff --git a/test/test_raster_gl_renderer.cpp b/test/test_raster_gl_renderer.cpp index bd21c3d..6deec55 100644 --- a/test/test_raster_gl_renderer.cpp +++ b/test/test_raster_gl_renderer.cpp @@ -128,7 +128,7 @@ TEST(RasterGlRendererTest, ScalarSubUnitRangeSpansColormap) RasterGlRenderer renderer; ASSERT_TRUE(renderer.makeCurrent()); - renderer.setColormap(camp::map::ColorMap::Grayscale); + renderer.setColormap("grayscale"); const int w = 2, h = 1; std::vector data = {0.0f, 0.3f}; // col0 = data_min, col1 = data_max @@ -231,10 +231,10 @@ TEST(RasterGlRendererTest, ColormapRebakesLut) auto tex = makeScalarTexture(n, n, data); const RasterFieldItem item = scalarItem(tex.get(), n, false, 0.0f); - renderer.setColormap(camp::map::ColorMap::Grayscale); + renderer.setColormap("grayscale"); const QColor gray = renderer.renderToImage({item}, QRectF(0, 0, n, n), 0.0f, 10.0f, QSize(n, n)).pixelColor(0, 0); - renderer.setColormap(camp::map::ColorMap::Viridis); + renderer.setColormap("viridis"); const QColor viridis = renderer.renderToImage({item}, QRectF(0, 0, n, n), 0.0f, 10.0f, QSize(n, n)).pixelColor(0, 0); tex.reset(); diff --git a/test/test_raster_layer_gdal_cleanup.cpp b/test/test_raster_layer_gdal_cleanup.cpp index b0a8c23..fa5db02 100644 --- a/test/test_raster_layer_gdal_cleanup.cpp +++ b/test/test_raster_layer_gdal_cleanup.cpp @@ -62,7 +62,7 @@ QString writeRaster(const QTemporaryDir& dir) ds->SetProjection(wkt); CPLFree(wkt); - // Varying values so the ColorMap spans a real (non-degenerate) range. + // Varying values so the colormap spans a real (non-degenerate) range. std::vector samples(static_cast(w) * h); for(size_t i = 0; i < samples.size(); ++i) samples[i] = static_cast(i); @@ -117,12 +117,12 @@ TEST(RasterLayerGdalCleanupTest, LoadAndColormapFlipsLeakNoDatasets) // ...and so must the async load that opens the handles under test. ASSERT_TRUE(waitForLoad(layer)) << "initial load did not reach the warp path"; - // Two additional ramp types (default is Viridis); each re-invokes + // Two additional ramp names (default is viridis); each re-invokes // loadAndReprojectFile, opening and — with the fix — closing both handles. - layer->setColormap(camp::map::ColorMap::Grayscale); - ASSERT_TRUE(waitForLoad(layer)) << "Grayscale re-render did not reach the warp path"; - layer->setColormap(camp::map::ColorMap::Turbo); - ASSERT_TRUE(waitForLoad(layer)) << "Turbo re-render did not reach the warp path"; + layer->setColormap("grayscale"); + ASSERT_TRUE(waitForLoad(layer)) << "grayscale re-render did not reach the warp path"; + layer->setColormap("turbo"); + ASSERT_TRUE(waitForLoad(layer)) << "turbo re-render did not reach the warp path"; // Dtor aborts + joins the in-flight load (waitForFinished), so every // loadAndReprojectFile invocation has returned and closed its handles.