fix(cmake): build GPU spectrum without Qt6GuiPrivate; drop duplicate qt_add_shaders#3561
Conversation
Two defects block the QRhi GPU spectrum path on Qt installs lacking the
Qt6GuiPrivate CMake package (e.g. Windows/macOS aqtinstall builds):
1. Detection probed only Debian multi-arch paths, so installs with the
private headers on disk but no Qt6GuiPrivate config fell back to CPU
QPainter. Now also derive the Qt include root from Qt6::Gui and probe
<root>/QtGui/<ver>/QtGui/private/, reusing the DEBIAN_GPU_FIX_REQUIRED
include plumbing.
2. Two identical qt_add_shaders(AetherSDR "aether_shaders" ...) blocks
crashed configure with a duplicate-target error whenever the GPU path
was actually enabled. Removed the duplicate and folded its corrected
macOS rhi-header fallback (was "${_pd}/.." -> "${_pd}") into the
single remaining block.
CI never hit either defect: AETHER_GPU_SPECTRUM auto-disables on the
Qt 6.4 CI image, so the GPU code path is not exercised there.
Demonstrated: configures and builds clean on Windows (Qt 6.8.3
msvc2022_64, VS2022); Help -> About reports a GPU QRhi renderer and all
six spectrum shaders compile to .qsb.
Fixes aethersdr#3560
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Thanks for this, @svabi79 — it's a well-diagnosed, well-documented fix, and the two defects are real.
I traced both changes against the current tree:
Defect 2 (duplicate qt_add_shaders) — confirmed. There were genuinely two top-level if(AETHER_GPU_SPECTRUM) blocks each calling qt_add_shaders(AetherSDR "aether_shaders" ...). The second call reuses the same resource name, so once the GPU path is actually on it tries to create the aether_shaders custom target twice and aborts configure. Folding into one block is the right call.
The off-by-one include fix — confirmed correct. In the surviving fallback, _pd is ${_dir}/${Qt6_VERSION}/QtGui and the header is found at ${_pd}/rhi/qrhi.h. To resolve #include <rhi/qrhi.h> the include dir must be the directory containing rhi/, i.e. _pd, not _pd/... The old "${_pd}/.." would have put rhi/ one level too deep on the search path. Good catch — this path was never exercised by CI so the bug was latent.
Defect 1 (detection) — looks sound and non-regressing. Deriving the include root from Qt6::Gui's INTERFACE_INCLUDE_DIRECTORIES and prepending it to the find_path PATHS is additive: on Debian, _qt_gui_inc_root resolves to the same /usr/include/<multiarch>/qt6 the existing probe already used, so find_path returns the same result and the green Linux path is unchanged. The DEBIAN_GPU_FIX_REQUIRED apply block then wires both …/QtGui/${ver} (for <QtGui/private/...>) and …/QtGui/${ver}/QtGui (for <rhi/qrhi.h>), so both header families resolve on the aqt layout. The dangling elseif(NOT DEBIAN_GPU_FIX_REQUIRED) rhi-GLOB branch is now a harmless safety net for the rare "GuiPrivate package found but no target" case.
One minor robustness note (non-blocking): detection keys solely on private/qhighdpiscaling_p.h, but the apply step also assumes rhi/qrhi.h sits beside it under …/QtGui/${ver}/QtGui/. That holds for every standard Qt SDK layout, so it's fine in practice — just flagging that an exotic install shipping private headers without the rhi tree would configure clean and then fail at compile on <rhi/qrhi.h>. Not worth complicating the probe for; mentioning only for completeness.
CI is green on all six checks (build / macOS / Windows / CodeQL / accessibility), though as you note the GPU path itself stays auto-disabled on the Qt 6.4 image, so the real proof is your local Qt 6.8.3 Windows configure+build — which is exactly the right verification for a build-system change.
CMakeLists.txt is a maintainer-only CODEOWNERS path (@ten9876), so this needs maintainer sign-off to merge — flagging per your note. LGTM from my read.
🤖 aethersdr-agent · cost: $3.8120 · model: claude-opus-4-8
…qt_add_shaders (aethersdr#3561) Unblocks the QRhi GPU spectrum/waterfall path on Qt installs lacking the Qt6GuiPrivate CMake package (Windows/macOS aqtinstall): derive the private-include root from Qt6::Gui and wire it up manually, and remove a duplicate qt_add_shaders block that aborted configure when the GPU path was actually on (fixing an off-by-one rhi/ include path in the folded fallback). Fixes aethersdr#3560. CI can't exercise this (AETHER_GPU_SPECTRUM auto-disables on the Qt 6.4 image); verified by @svabi79 on Windows/Qt 6.8.3/MSVC 2022. Co-authored-by: svabi79 <svabi79@users.noreply.github.com>
Summary
Fixes #3560. Unblocks the QRhi GPU spectrum/waterfall path (
AETHER_GPU_SPECTRUM) on Qt installs that lack theQt6GuiPrivateCMake package — notably standard Windows/macOSaqtinstalldeployments, where the private headers are on disk but the CMake config that exportsQt6::GuiPrivateis not. Two defects combined to either silently fall back to CPUQPainterrendering or hard-crashcmakeconfigure when the GPU path was enabled.Defect 1 — detection only handled Debian. The
if(NOT Qt6GuiPrivate_FOUND)probe searched only/usr/include/<multiarch>/qt6. Now it also derives the Qt include root fromQt6::Gui'sINTERFACE_INCLUDE_DIRECTORIESand probes<root>/QtGui/<ver>/QtGui/private/, so Windows/macOS SDK layouts are detected and wired up via the existingDEBIAN_GPU_FIX_REQUIRED→target_include_directoriespath. (Same class as #3157, one layer out.)Defect 2 — duplicate
qt_add_shaders. Two identical top-levelif(AETHER_GPU_SPECTRUM)blocks each calledqt_add_shaders(AetherSDR "aether_shaders" ...). With the GPU path actually on, the second call aborts configure with a duplicate-target error — before any compilation. Removed the duplicate and folded its macOS rhi-header fallback into the single remaining block, correcting an off-by-one include path ("${_pd}/.."→"${_pd}", otherwise#include <rhi/qrhi.h>would not resolve).CI never caught either defect because
AETHER_GPU_SPECTRUMauto-disables on the Qt 6.4 CI image, so the GPU code path is never exercised there.Constitution principle honored
Principle XI — Fixes Are Demonstrated. Verified by a clean configure + full build on the affected platform (see Test plan), not just a code-reading hypothesis. The squash-merge CI re-run is the independent check.
Test plan
msvc2022_64, VS 2022; fullAetherSDR.exelink, no errors. Configure now logsGPU spectrum rendering enabled (QRhi, Qt 6.8.3)andApplying private QtGui include paths to AetherSDR; all six spectrum shaders compile to.qsb.Help → Aboutreports aGPU QRhi (...)renderer instead ofCPU QPainter.No-functional-regression note: on the Qt 6.4 / Linux CI path the new detection branch is a strict superset of the old one (Debian probe unchanged, new Qt6::Gui-root probe is additive), and the GPU blocks were textually identical, so removing the duplicate cannot change a green build.
Checklist
AppSettingscalls (N/A — CMake only)MeterSmoother(N/A — CMake only)