Skip to content

Commit bd8923c

Browse files
jensenpatten9876claude
authored
fix(crash): Keep macOS GPU-spectrum PSK map on a raster viewport (#5623)
## Summary Split from [#5595](#5595): opening **PSK Reporter** on macOS in an `AETHER_GPU_SPECTRUM` build can crash when the flat map attaches a `QOpenGLWidget` viewport. The GUI thread re-enters `WaveformWidget::renderGpuFrame()` through a backing-store repaint while `QAbstractScrollArea::setViewport()` is attaching the map OpenGL viewport. The failure is in `QMetalGraphicsPipeline::makeActiveForCurrentRenderPassEncoder()`. This PR keeps the flat map on `QGraphicsView`'s **raster** viewport when both `Q_OS_MAC` and `AETHER_GPU_SPECTRUM` are set. Other platforms and non-GPU-spectrum builds still request `OpenGlIfAvailable`. ### What this trades `AETHER_GPU_SPECTRUM` defaults ON and `macos-dmg.yml` ships it ON, so this reaches every released macOS build. The OpenGL viewport was added for a stated reason — moving the weather-radar mesh's many transformed image samples off the GUI-thread raster paint engine (`MapView.cpp:151-155`) — and macOS gives that up here: **flat-map radar-playback smoothness is traded for not crashing.** Linux, Windows and CPU-spectrum macOS are unchanged and keep the OpenGL viewport. ## Review follow-ups A second commit addresses the review nits: - **`flatControllerLoopRetainsEveryPaint()` now `QSKIP`s under the same condition.** That case asserts the flat map owns a `QOpenGLWidget`; under `Q_OS_MAC && AETHER_GPU_SPECTRUM` it owns none. It passes today only because the registered target does not define `AETHER_GPU_SPECTRUM` — so it would have failed for anyone compiling the map sources the way the app is compiled, reading as a regression rather than as the intended raster path. - **`MapView`'s ctor now states why the raster path keeps Qt's default `MinimalViewportUpdate`** rather than the `SmartViewportUpdate` that `fallBackToRasterViewport()` installs: that path is undoing the `FullViewportUpdate` set for a failed OpenGL viewport, so Smart is a correction there rather than a tuned optimum, and a viewport that was never Full has nothing to correct. ## Constitution principle honored Principle XI — Fixes Are Demonstrated: the crash was observed on a GPU-spectrum macOS build while exercising PSK Reporter during #5595; this isolates the viewport change reviewers asked to keep out of the menu PR. ## Test plan - [x] Local RelWithDebInfo build with GPU spectrum enabled (`cmake --build build -j22 --target AetherSDR`) - [x] Open PSK Reporter on macOS GPU-spectrum build with a live panadapter/WAVE scope — must not crash — *@jensenpat, live retest on the GPU-spectrum test Mac: no crash, flat map came up and stayed usable* - [x] Pan/zoom the flat map; city lights / weather radar overlays still draw — *@K5PTB ran `weather_radar_loading_test` on macOS arm64 with `AETHER_GPU_SPECTRUM` added, i.e. the raster branch this PR selects: 32 passed / 5 skipped, covering radar playback, rolling history, export and progressive zoom* - [x] Linux/Windows (or macOS without `AETHER_GPU_SPECTRUM`) still use the OpenGL viewport path — *preprocessed with the real `AetherSDR` target flags and `AETHER_GPU_SPECTRUM` defined; the function reduces to `return MapView::ViewportMode::OpenGlIfAvailable;`, isolating `Q_OS_MAC` as the discriminator* - [ ] Toggle the PSK map to **Globe** projection on a macOS GPU-spectrum build with a live panadapter — `GlobeMapView` is a `QOpenGLWidget` and, after this change, the only GL surface the app can still realize in that configuration. Pre-existing behavior, not a regression from this PR. ## Checklist - [x] Commits are signed - [x] No new flat-key `AppSettings` calls - [x] Clean-room — Qt viewport selection only --------- Co-authored-by: Jeremy [KK7GWY] <kk7gwy@aethersdr.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 996b367 commit bd8923c

3 files changed

Lines changed: 38 additions & 2 deletions

File tree

src/gui/map/MapDisplayWidget.cpp

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,13 +52,28 @@ constexpr qint64 kFrameCacheLifetimeMs = 4 * 60 * 60 * 1000
5252
+ 15 * 60 * 1000;
5353
constexpr qint64 kMaximumFrameCacheBytes = 256 * 1024 * 1024;
5454

55+
// On macOS + GPU spectrum, attaching a QOpenGLWidget viewport can force a
56+
// backing-store repaint that re-enters a Metal-backed QRhiWidget (the
57+
// panadapter or WAVE scope) and blows up in
58+
// QMetalGraphicsPipeline::makeActiveForCurrentRenderPassEncoder. Keep the
59+
// flat PSK map on QGraphicsView's raster viewport in that configuration.
60+
// Discovered while opening Tools → PSK Reporter on a GPU-spectrum build
61+
// (PR #5595). Not a performance claim — compatibility only.
62+
MapView::ViewportMode flatMapViewportMode()
63+
{
64+
#if defined(Q_OS_MAC) && defined(AETHER_GPU_SPECTRUM)
65+
return MapView::ViewportMode::Raster;
66+
#else
67+
return MapView::ViewportMode::OpenGlIfAvailable;
68+
#endif
69+
}
70+
5571
}
5672

5773
MapDisplayWidget::MapDisplayWidget(QWidget* parent)
5874
: QWidget(parent)
5975
, m_stack(new QStackedLayout(this))
60-
, m_flatView(new MapView(
61-
this, MapView::ViewportMode::OpenGlIfAvailable))
76+
, m_flatView(new MapView(this, flatMapViewportMode()))
6277
{
6378
m_cityLightsSource = new CityLightsSource(this);
6479
connect(m_cityLightsSource, &CityLightsSource::imageChanged,

src/gui/map/MapView.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,16 @@ MapView::MapView(QWidget* parent, ViewportMode viewportMode)
164164
m_map->geoView()->setViewportUpdateMode(
165165
QGraphicsView::FullViewportUpdate);
166166
m_openGlViewport = openGlViewport;
167+
} else {
168+
// Raster keeps QGraphicsView's default MinimalViewportUpdate, and that
169+
// is deliberately NOT the SmartViewportUpdate that
170+
// fallBackToRasterViewport() installs. That path is undoing the
171+
// FullViewportUpdate set just above after an OpenGL viewport failed to
172+
// come up, so Smart is a correction there, not a tuned optimum. A
173+
// viewport that was never Full has nothing to correct, and Minimal is
174+
// what every other ViewportMode::Raster user — the GPS dialog, and
175+
// macOS GPU-spectrum builds via MapDisplayWidget's
176+
// flatMapViewportMode() — has always run with.
167177
}
168178
layout->addWidget(m_map);
169179

tests/weather_radar_loading_test.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -761,6 +761,17 @@ private slots:
761761

762762
void flatControllerLoopRetainsEveryPaint()
763763
{
764+
#if defined(Q_OS_MAC) && defined(AETHER_GPU_SPECTRUM)
765+
// MapDisplayWidget::flatMapViewportMode() deliberately keeps the flat
766+
// map on QGraphicsView's raster viewport in this configuration, so
767+
// m_flatView owns no QOpenGLWidget for this case to sample. The target
768+
// as registered does not define AETHER_GPU_SPECTRUM, so this only
769+
// fires for someone compiling the map sources the way the app is
770+
// compiled. Without it, that build reads as a regression rather than
771+
// as the raster path the app deliberately selects.
772+
QSKIP("macOS GPU-spectrum builds keep the flat map on the raster "
773+
"viewport; there is no QOpenGLWidget to read pixels from");
774+
#endif
764775
if (!qEnvironmentVariableIsSet("AETHERSDR_TEST_RADAR_GL")) {
765776
QSKIP("Opt in with AETHERSDR_TEST_RADAR_GL=1 and a native GUI platform");
766777
}

0 commit comments

Comments
 (0)