diff --git a/CMakeLists.txt b/CMakeLists.txt index 12e158d..6ba4c52 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -19,6 +19,7 @@ find_package(nav2_msgs REQUIRED) find_package(pluginlib REQUIRED) find_package(marine_autonomy REQUIRED) find_package(marine_colormap REQUIRED) +find_package(marine_colormap_widgets REQUIRED) find_package(marine_interfaces REQUIRED) find_package(marine_tiled_raster_store REQUIRED) find_package(marine_nav_interfaces REQUIRED) @@ -260,6 +261,7 @@ set(CAMP_MAP_SOURCES src/camp_map/map_tree_view/map_tree_view.cpp src/camp_map/map_view/map_view.cpp src/camp_map/map_view/web_mercator.cpp + src/camp_map/raster/colormap_range_dialog.cpp src/camp_map/raster/raster_gl_renderer.cpp src/camp_map/raster/raster_layer.cpp src/camp_map/raster/gggs_tile.cpp @@ -301,6 +303,10 @@ target_link_libraries(camp_map # consumer of those headers (camp_map_ros, the camp executable, the tests). marine_colormap::marine_colormap PRIVATE + # [camp#142 PR2] The interactive colorbar dialog (colormap_range_dialog.cpp) + # is the only user of the widget, and only in the .cpp — the public header + # forward-declares QWidget and exposes no widget type — so link PRIVATE. + marine_colormap_widgets::marine_colormap_widgets ${GDAL_LIBRARY} ) diff --git a/docs/decisions/0009-colormap-range-colorbar-placement.md b/docs/decisions/0009-colormap-range-colorbar-placement.md new file mode 100644 index 0000000..c97b050 --- /dev/null +++ b/docs/decisions/0009-colormap-range-colorbar-placement.md @@ -0,0 +1,84 @@ +# ADR-0009: Place the interactive colorbar in a context-menu dialog + +## Status + +Accepted + +Implements CAMP issue [#142](https://github.com/rolker/camp/issues/142) — give the +operator an interactive way to override a scalar layer's colormap range when an +outlier skews the auto-range (the motivating case: backscatter band 2 with +Max ≈ 925 / Mean ≈ 0.16 collapses the auto colorbar). Builds on the per-layer +`marine_colormap::RangeModel` and numeric "Set range…" prompts landed in PR1 +(also #142), and on [ADR-0008](0008-adopt-marine-colormap-lut-bake.md) (the shared +`marine_colormap` palettes) and [ADR-0007](0007-raster-field-source-interface.md) +(the unified raster render path the three scalar layers share). + +> **Numbering.** This is **camp ADR-0009**, a *project* ADR in this repo's +> `docs/decisions/` series — independent of the workspace agent-framework ADR +> series, where the numbers collide by coincidence (see ADR-0007/0008). + +## Context + +PR1 shipped the range-override backend (`RangeModel` Auto/Manual on +`GggsTileLayer`, `RasterLayer`, `SonarLiveCacheLayer`; `setRangeOverride` / +`resetRangeToAuto`; per-layer persistence) with a minimal UI: two sequential +`QInputDialog::getDouble` prompts. That works but is blind — the operator types +numbers without seeing the palette, the data extents, or where the handles sit +relative to the outlier. + +The shared `marine_colormap_widgets::ColormapLegendWidget` (marine_colormap +ADR-0002) paints the value→colour ramp and lets the operator **drag** min/max +handles to pin a `Manual` range. PR2 embeds it. A draggable widget cannot live +*inside* a transient `QMenu` (the menu closes on click), so "context menu" means a +small window opened *from* the menu. + +Three placements were considered: + +1. **Modal dialog opened from the layer context menu** — one entry, "Colormap + range…", per scalar layer. No permanent screen real estate; discoverable from + the same place as the colormap/band actions. +2. **Docked panel in the Layers tab** showing the selected layer's colorbar — + always visible, better for continuous live tweaking, but needs panel layout + work and a selection-follows-colorbar wiring. +3. **Map overlay** near the active layer (scale-bar style) — closest to a chart + legend, but the most placement/occlusion code. + +## Decision + +Use **option 1**: a modal `QDialog` opened from each scalar layer's context menu +("Colormap range…"), hosting the `ColormapLegendWidget` plus min/max +`QDoubleSpinBox`es. Implemented once in `raster/colormap_range_dialog.{h,cpp}` +(`camp::raster::showColormapRangeDialog`) and called from all three layers, so the +wiring is not duplicated. This replaces PR1's two numeric prompts; the spin boxes +preserve precise numeric entry, now kept in sync with the bar. + +Wiring contract: + +- On open, the dialog seeds the widget from a `ColormapRangeState` snapshot + (palette index, data extents as the domain, current mode/lo/hi). An existing + `Manual` override is shown via `ColormapLegendWidget::setManual` (the slot added + in marine_colormap#10 precisely so a consumer can display a persisted override); + otherwise the bar tracks Auto. +- The widget's `rangeChanged` fires for both a manual pin and an auto fold; the + dialog disambiguates on `mode()` — `Manual` → `on_range(lo, hi)` (the layer's + `setRangeOverride`), otherwise `on_reset()` (the layer's `resetRangeToAuto`). +- Edits fire **live** while the dialog is open, so the map behind it re-renders as + the operator drags or types. + +## Consequences + +- The colorbar is a per-layer, on-demand tool — no always-on UI, no layout churn. +- The shared dialog keeps the three layers' menus identical and DRY; a future + fourth scalar layer reuses it. +- Modal `exec()` blocks the rest of the UI while open (the map still repaints + underneath). Acceptable for a brief range adjustment; a future modeless or docked + variant (option 2) can reuse the same `ColormapRangeState` seam if continuous + multi-layer tweaking becomes a need. +- The dialog is thin Qt glue over already-tested components — the widget's + `setManual`/drag/reset/`rangeChanged` (marine_colormap gtest) and the layers' + `setRangeOverride`/`resetRangeToAuto`/persistence (PR1 `test_range_persist`). The + modal event loop makes the glue itself impractical to unit-test headlessly, so it + is validated by manual exercise in the deployed app (consistent with how the #108 + band picker was verified). +- The uncertainty-band default (`quality` palette + `[0, threshold]` on open) is a + separate follow-up (needs #104 band semantics), not part of this dialog. diff --git a/package.xml b/package.xml index 1fb0270..d3a709b 100644 --- a/package.xml +++ b/package.xml @@ -24,6 +24,7 @@ pluginlib marine_autonomy marine_colormap + marine_colormap_widgets marine_interfaces marine_tiled_raster_store marine_nav_interfaces diff --git a/src/camp_map/raster/colormap_range_dialog.cpp b/src/camp_map/raster/colormap_range_dialog.cpp new file mode 100644 index 0000000..4e4e489 --- /dev/null +++ b/src/camp_map/raster/colormap_range_dialog.cpp @@ -0,0 +1,136 @@ +// Copyright 2026 Roland Arsenault +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "colormap_range_dialog.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "marine_colormap_widgets/colormap_legend_widget.hpp" + +namespace camp +{ +namespace raster +{ + +void showColormapRangeDialog( + QWidget * parent, const QString & title, const ColormapRangeState & state, + std::function on_range, std::function on_reset) +{ + QDialog dialog(parent); + dialog.setWindowTitle(title); + + auto * layout = new QVBoxLayout(&dialog); + layout->addWidget(new QLabel( + "Drag the handles or edit the bounds to pin a manual range; " + "reset to track the data automatically.", &dialog)); + + auto * legend = new marine_colormap_widgets::ColormapLegendWidget(&dialog); + legend->setPalette(state.palette_index); + + // The colorbar domain is the data extent the handles slide within. When no data + // has been folded yet (data_min > data_max, the layers' crossed sentinel), fall + // back to the resolved range; widen a degenerate point so the handles have room. + float domain_min = state.data_min; + float domain_max = state.data_max; + if (domain_min > domain_max) { + domain_min = state.lo; + domain_max = state.hi; + } + if (domain_min >= domain_max) { + domain_min -= 0.5f; + domain_max += 0.5f; + } + legend->setDomain(domain_min, domain_max); + layout->addWidget(legend); + + // Min/max spin boxes give precise entry alongside the drag (the numeric path + // PR1 offered, now folded into the same dialog and kept in sync with the bar). + auto * form = new QFormLayout(); + auto * min_spin = new QDoubleSpinBox(&dialog); + auto * max_spin = new QDoubleSpinBox(&dialog); + for (QDoubleSpinBox * spin : {min_spin, max_spin}) { + spin->setRange(-1.0e9, 1.0e9); + spin->setDecimals(6); + } + form->addRow("Minimum:", min_spin); + form->addRow("Maximum:", max_spin); + layout->addLayout(form); + + // Seed the widget to the layer's current state BEFORE wiring callbacks, so the + // seeding emissions don't echo back to the layer or the spin boxes. + if (state.mode == marine_colormap::RangeMode::Manual) { + legend->setManual(state.lo, state.hi); + } else { + legend->updateAuto(domain_min, domain_max); + } + // Initialise the spin boxes from the resolved bar state so the two always agree. + min_spin->setValue(legend->lo()); + max_spin->setValue(legend->hi()); + + // The widget's rangeChanged fires for both a manual pin and a reset/auto fold; + // mode() disambiguates which callback to run. The spin boxes mirror the bar. + QObject::connect( + legend, &marine_colormap_widgets::ColormapLegendWidget::rangeChanged, + [legend, min_spin, max_spin, on_range, on_reset](float lo, float hi) { + { + const QSignalBlocker block_min(min_spin); + const QSignalBlocker block_max(max_spin); + min_spin->setValue(lo); + max_spin->setValue(hi); + } + if (legend->mode() == marine_colormap::RangeMode::Manual) { + if (on_range) { + on_range(lo, hi); + } + } else if (on_reset) { + on_reset(); + } + }); + + // Editing either spin box pins a manual range on the bar (which then echoes + // back through rangeChanged to update the layer and re-sync the boxes). + auto pin_from_spins = [legend, min_spin, max_spin]() { + legend->setManual( + static_cast(min_spin->value()), static_cast(max_spin->value())); + }; + QObject::connect(min_spin, &QDoubleSpinBox::editingFinished, legend, pin_from_spins); + QObject::connect(max_spin, &QDoubleSpinBox::editingFinished, legend, pin_from_spins); + + auto * buttons = new QHBoxLayout(); + auto * reset_btn = new QPushButton("Reset to auto", &dialog); + QObject::connect(reset_btn, &QPushButton::clicked, legend, [legend, domain_min, domain_max]() { + legend->reset(); // model -> Auto (emits rangeChanged -> on_reset) + legend->updateAuto(domain_min, domain_max); // move handles back across the domain + }); + auto * close_btn = new QPushButton("Close", &dialog); + close_btn->setDefault(true); + QObject::connect(close_btn, &QPushButton::clicked, &dialog, &QDialog::accept); + buttons->addWidget(reset_btn); + buttons->addStretch(); + buttons->addWidget(close_btn); + layout->addLayout(buttons); + + dialog.exec(); +} + +} // namespace raster +} // namespace camp diff --git a/src/camp_map/raster/colormap_range_dialog.h b/src/camp_map/raster/colormap_range_dialog.h new file mode 100644 index 0000000..de38918 --- /dev/null +++ b/src/camp_map/raster/colormap_range_dialog.h @@ -0,0 +1,62 @@ +// Copyright 2026 Roland Arsenault +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef CAMP_MAP__RASTER__COLORMAP_RANGE_DIALOG_H_ +#define CAMP_MAP__RASTER__COLORMAP_RANGE_DIALOG_H_ + +#include + +#include + +#include "marine_colormap/transfer.hpp" + +class QWidget; + +namespace camp +{ +namespace raster +{ + +/// A layer's current colormap-range state, passed into showColormapRangeDialog() +/// to seed the interactive colorbar (camp#142). The three scalar raster layers +/// (GggsTileLayer, RasterLayer, SonarLiveCacheLayer) each own a +/// marine_colormap::RangeModel; this is the snapshot the dialog needs to open +/// reflecting that model. +struct ColormapRangeState +{ + int palette_index = 0; ///< marine_colormap registry index for the painted ramp + float data_min = 0.0f; ///< data extent low (colorbar domain); data_min > data_max == no data + float data_max = 1.0f; ///< data extent high + marine_colormap::RangeMode mode = marine_colormap::RangeMode::Auto; + float lo = 0.0f; ///< current resolved low bound (the active window) + float hi = 1.0f; ///< current resolved high bound +}; + +/// Open a modal dialog hosting marine_colormap_widgets::ColormapLegendWidget plus +/// min/max spin boxes for one layer's colormap range (camp#142 PR2, the +/// interactive successor to PR1's numeric prompts). +/// +/// Dragging a handle (or editing a spin box) pins a Manual override and fires +/// `on_range(lo, hi)`; the "Reset to auto" button returns to the data-driven +/// Auto range and fires `on_reset()`. Both fire **live** while the dialog is open +/// so the map re-renders as the operator adjusts. The colorbar and spin boxes +/// stay in sync with each other. +void showColormapRangeDialog( + QWidget * parent, const QString & title, const ColormapRangeState & state, + std::function on_range, std::function on_reset); + +} // namespace raster +} // namespace camp + +#endif // CAMP_MAP__RASTER__COLORMAP_RANGE_DIALOG_H_ diff --git a/src/camp_map/raster/gggs_tile_layer.cpp b/src/camp_map/raster/gggs_tile_layer.cpp index 05e8307..667fcc4 100644 --- a/src/camp_map/raster/gggs_tile_layer.cpp +++ b/src/camp_map/raster/gggs_tile_layer.cpp @@ -2,6 +2,7 @@ #include "gggs_tile.h" #include "gggs_tile_util.h" +#include "colormap_range_dialog.h" #include "../map_view/web_mercator.h" #include @@ -10,7 +11,6 @@ #include #include #include -#include #include #include #include @@ -644,29 +644,27 @@ void GggsTileLayer::contextMenu(QMenu* menu) connect(action, &QAction::triggered, this, [this, name]() { setColormap(name); }); } - // [camp#142] Colormap range override. GGGS tiles are always scalar values, so the - // submenu is offered unconditionally (same gating as the Colormap submenu above). - // "Set range…" prompts for lo then hi (pre-filled with the current resolved range) - // and pins a Manual override; "Reset to auto" returns to the data-driven extents. - QMenu* range_menu = menu->addMenu("Colormap range"); - QAction* set_range = range_menu->addAction("Set range…"); - connect(set_range, &QAction::triggered, this, [this]() + // [camp#142 PR2] Colormap range override. GGGS tiles are always scalar, so the + // action is offered unconditionally (same gating as the Colormap submenu above). + // Opens the interactive colorbar — drag the handles or edit the bounds to pin a + // Manual override, reset to track the data extents — the successor to PR1's + // sequential numeric prompts. + QAction* range_action = menu->addAction("Colormap range…"); + connect(range_action, &QAction::triggered, this, [this]() { - bool ok = false; - const double lo = QInputDialog::getDouble( - nullptr, "Colormap range", "Minimum:", range_model_.lo(), - -1.0e9, 1.0e9, 6, &ok); - if(!ok) - return; - const double hi = QInputDialog::getDouble( - nullptr, "Colormap range", "Maximum:", range_model_.hi(), - -1.0e9, 1.0e9, 6, &ok); - if(!ok) - return; - setRangeOverride(float(lo), float(hi)); + ColormapRangeState state; + const auto idx = marine_colormap::palette_index(renderer_.colormap()); + state.palette_index = idx ? static_cast(*idx) : 0; + state.data_min = static_cast(data_min_); + state.data_max = static_cast(data_max_); + state.mode = range_model_.mode(); + state.lo = range_model_.lo(); + state.hi = range_model_.hi(); + showColormapRangeDialog( + nullptr, "Colormap range", state, + [this](float lo, float hi) { setRangeOverride(lo, hi); }, + [this]() { resetRangeToAuto(); }); }); - QAction* reset_range = range_menu->addAction("Reset to auto"); - connect(reset_range, &QAction::triggered, this, [this]() { resetRangeToAuto(); }); // [camp#108] Band picker — only for multi-band tile-sets (bathy depth + // uncertainty, backscatter intensity + quality). Single-band stores (the diff --git a/src/camp_map/raster/raster_layer.cpp b/src/camp_map/raster/raster_layer.cpp index 498ff50..c6d814b 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 "colormap_range_dialog.h" #include #include #include @@ -13,7 +14,6 @@ #include #include #include -#include #include #include @@ -521,29 +521,27 @@ void RasterLayer::contextMenu(QMenu* menu) connect(action, &QAction::triggered, this, [this, name]() { setColormap(name); }); } - // [camp#142] Colormap range override (scalar only — gated by the is_scalar_ early - // return above, the same condition as the Colormap submenu). "Set range…" prompts - // for lo then hi (pre-filled with the current resolved range) and pins a Manual - // override; "Reset to auto" returns to the data-driven extents. - QMenu* range_menu = menu->addMenu("Colormap range"); - QAction* set_range = range_menu->addAction("Set range…"); - connect(set_range, &QAction::triggered, this, [this]() + // [camp#142 PR2] Colormap range override (scalar only — gated by the is_scalar_ + // early return above, the same condition as the Colormap submenu). Opens the + // interactive colorbar — drag the handles or edit the bounds to pin a Manual + // override, reset to track the data extents — the successor to PR1's sequential + // numeric prompts. + QAction* range_action = menu->addAction("Colormap range…"); + connect(range_action, &QAction::triggered, this, [this]() { - bool ok = false; - const double lo = QInputDialog::getDouble( - nullptr, "Colormap range", "Minimum:", range_model_.lo(), - -1.0e9, 1.0e9, 6, &ok); - if(!ok) - return; - const double hi = QInputDialog::getDouble( - nullptr, "Colormap range", "Maximum:", range_model_.hi(), - -1.0e9, 1.0e9, 6, &ok); - if(!ok) - return; - setRangeOverride(float(lo), float(hi)); + ColormapRangeState state; + const auto idx = marine_colormap::palette_index(renderer_.colormap()); + state.palette_index = idx ? static_cast(*idx) : 0; + state.data_min = static_cast(data_min_); + state.data_max = static_cast(data_max_); + state.mode = range_model_.mode(); + state.lo = range_model_.lo(); + state.hi = range_model_.hi(); + showColormapRangeDialog( + nullptr, "Colormap range", state, + [this](float lo, float hi) { setRangeOverride(lo, hi); }, + [this]() { resetRangeToAuto(); }); }); - QAction* reset_range = range_menu->addAction("Reset to auto"); - connect(reset_range, &QAction::triggered, this, [this]() { resetRangeToAuto(); }); } void RasterLayer::readSettings() 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 ffc0abf..df5c01e 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 @@ -2,6 +2,7 @@ #include "../node.h" #include "../../map_view/web_mercator.h" +#include "../../raster/colormap_range_dialog.h" #include @@ -9,7 +10,6 @@ #include #include #include -#include #include #include #include @@ -729,29 +729,27 @@ void SonarLiveCacheLayer::contextMenu(QMenu* menu) connect(action, &QAction::triggered, this, [this, name]() { setColormap(name); }); } - // [camp#142] Colormap range override. Live bands are scalar values (same gating as - // the Colormap submenu above), so the submenu is offered unconditionally. "Set - // range…" prompts for lo then hi (pre-filled with the current resolved range) and - // pins a Manual override; "Reset to auto" returns to the data-driven extents. - QMenu* range_menu = menu->addMenu("Colormap range"); - QAction* set_range = range_menu->addAction("Set range…"); - connect(set_range, &QAction::triggered, this, [this]() + // [camp#142 PR2] Colormap range override. Live bands are scalar (same gating as + // the Colormap submenu above), so the action is offered unconditionally. Opens + // the interactive colorbar — drag the handles or edit the bounds to pin a Manual + // override, reset to track the data extents — the successor to PR1's sequential + // numeric prompts. + QAction* range_action = menu->addAction("Colormap range…"); + connect(range_action, &QAction::triggered, this, [this]() { - bool ok = false; - const double lo = QInputDialog::getDouble( - nullptr, "Colormap range", "Minimum:", range_model_.lo(), - -1.0e9, 1.0e9, 6, &ok); - if(!ok) - return; - const double hi = QInputDialog::getDouble( - nullptr, "Colormap range", "Maximum:", range_model_.hi(), - -1.0e9, 1.0e9, 6, &ok); - if(!ok) - return; - setRangeOverride(float(lo), float(hi)); + raster::ColormapRangeState state; + const auto idx = marine_colormap::palette_index(renderer_.colormap()); + state.palette_index = idx ? static_cast(*idx) : 0; + state.data_min = static_cast(data_min_); + state.data_max = static_cast(data_max_); + state.mode = range_model_.mode(); + state.lo = range_model_.lo(); + state.hi = range_model_.hi(); + raster::showColormapRangeDialog( + nullptr, "Colormap range", state, + [this](float lo, float hi) { setRangeOverride(lo, hi); }, + [this]() { resetRangeToAuto(); }); }); - QAction* reset_range = range_menu->addAction("Reset to auto"); - connect(reset_range, &QAction::triggered, this, [this]() { resetRangeToAuto(); }); // Band picker over the union of band names across held tiles. Only shown when // there is more than one band to choose from.