Reviewed against the surrounding SpectrumWidget.cpp render code — this is correct and well-scoped. Thanks for the careful writeup, @svabi79.

Verified the fix does what it claims:

• Composite order. In renderGpuFrame() the draw sequence is waterfall → m_bgSrb/m_overlayBg quad (drawn below FFT, ~line 7384) → FFT fill/line → m_ovSrb/m_overlayStatic quad (drawn above FFT, ~line 7424). Moving drawGrid into m_overlayBg therefore lands the grid behind the trace, matching the software path's drawGrid → drawSpectrum ordering. Parity confirmed.
• Zero-cost claim holds. m_overlayBg and m_overlayStatic are both rebuilt under the same m_overlayStaticDirty gate and uploaded on the same cadence — same paint, just a different cached layer. No per-frame cost added, and the grid now sits correctly above the bg fill/image but below the FFT within that layer.
• drawGrid is self-contained (line 7933): sets its own pen, draws relative to the passed specRect, and reads only state stable within the dirty gate. It doesn't depend on painter state from the rest of the static-overlay block, so removing it from there is side-effect-free.

No convention concerns (no QSettings/flat-key AppSettings, no null/leak risks), and the change stays within its stated single-file scope. The inline comments at both move sites are a nice touch for the next reader. LGTM. 👍

One optional follow-up, not blocking: as you note in the test plan, a Metal/Vulkan screenshot would be good confirmation — but since this is the same QRhi code path on all backends, I don't see a platform-specific risk here.

_{🤖 aethersdr-agent · cost: $2.8614 · model: claude-opus-4-8}

Human-directed independent review (Claude / @NF0T) | Request Changes

This review was conducted by Claude under direction from @NF0T, independently of the triage bot's prior pass. Every structural claim in the PR and the bot's review was verified against the current codebase before forming a conclusion — the bot review on file is treated as an unverified starting point, not corroboration. What follows documents the verification trail in full.

What I verified and how

Render order — confirmed from the command buffer, not from comments.

I read renderGpuFrame() directly to establish the actual GPU composite order. The command buffer submission sequence in the current codebase is:

1. Waterfall quad
2. m_bgSrb / m_overlayBg quad — submitted at line 7384, explicitly before the FFT passes
3. FFT fill pass + line pass — lines ~7395–7425
4. m_ovSrb / m_overlayStatic quad — submitted at line 7427, after the FFT

The PR's description of this ordering is accurate. I did not take the author's summary at face value — I traced the actual cb->setShaderResources and cb->draw calls in sequence to confirm.

The bug — confirmed from the current file, not from the issue.

With the above ordering established, I searched for every drawGrid call site in SpectrumWidget.cpp. There are exactly two: one in the GPU path (line 6997) and one in the software path (line 7627). The GPU path call at line 6997 is inside the if (m_overlayStaticDirty) block that paints m_overlayStatic using painter p — confirmed by the QPainter declaration at line 6994 (QPainter p(&m_overlayStatic)). That layer is the one submitted at step 4 above, above the FFT. The bug is real.

The software path at lines 7627–7628 is drawGrid(p, specRect) immediately followed by drawSpectrum(p, specRect) — grid behind trace. The two paths disagree. The parity gap is confirmed.

Dirty gate and zero-cost claim — verified.

Both the m_overlayBg block (line 6971) and the m_overlayStatic block (line 6992) open with if (m_overlayStaticDirty). Same flag, same rebuild cadence. Moving drawGrid between the two blocks does not change when it runs or how often. The zero-cost claim holds.

Static overlay removal side effects — verified clean.

I confirmed that after removing drawGrid from m_overlayStatic, the next call in that block becomes drawBandPlan. I read drawBandPlan() (line 8127) and confirmed it calls p.setPen(...) early in its own body before any drawing. It does not inherit or depend on whatever pen state drawGrid previously left on the painter. Removing drawGrid from before it introduces no side effects in the static overlay block.

Blocking issue found: painter opacity not reset before drawGrid in the new location

This is what the bot review missed, and how I found it.

After confirming the fix concept was structurally correct, I read the full m_overlayBg paint block — not just the lines adjacent to the diff hunk — to understand the complete painter state at the point where drawGrid(bp, specRect) is inserted. The block, in its entirety, is:

if (m_overlayStaticDirty) {
    m_overlayBg.fill(Qt::transparent);
    QPainter bp(&m_overlayBg);
    bp.setRenderHint(QPainter::Antialiasing, false);
    bp.fillRect(specRect, m_bgFillColor);
    if (!m_leanMode && !m_bgImage.isNull()) {
        if (m_bgScaledSize != specRect.size()) {
            // ... scaling ...
        }
        bp.setOpacity(1.0 - m_bgOpacity / 100.0);   // <── opacity set here
        bp.drawImage(specRect.topLeft(), m_bgScaled);
    }
    // PR inserts drawGrid(bp, specRect) here
    m_overlayBgNeedsUpload = true;
}

QPainter::setOpacity is a persistent state mutation — the painter does not auto-restore opacity when an if block exits. If the conditional branch runs (i.e., the user has configured a background image), bp exits that block with its opacity set to 1.0 - m_bgOpacity / 100.0, and that is the opacity in effect when drawGrid(bp, specRect) executes.

I then read drawGrid() in full (line 7933). It calls p.setPen(...) twice — once for horizontal lines, once for vertical — so it is correctly independent of the caller's pen state. However, it makes no call to setOpacity. It unconditionally inherits whatever opacity the painter carries at call time.

I then checked SpectrumWidget.h for the default value of m_bgOpacity:

int m_bgOpacity{80};  // 0=full image, 100=solid dark (default 80%)

With the default, bp.setOpacity(1.0 - 80/100.0) = bp.setOpacity(0.20). Any user who has configured a background image will have drawGrid execute at 20% opacity — the grid lines become nearly invisible, a direct visual regression introduced by this change.

The unaffected path: when no background image is configured, m_bgImage.isNull() returns true, the conditional branch is skipped entirely, bp retains its default opacity of 1.0, and drawGrid runs correctly. This is why the issue would not appear in the author's screenshot (likely taken without a background image) and why CI or casual testing may not catch it — it is a minority configuration.

The bot's review stated that drawGrid "doesn't depend on painter state from the rest of the static-overlay block, so removing it from there is side-effect-free." That observation is correct about the source location (the static overlay block keeps its opacity at 1.0 throughout). But the bot did not examine the painter state at the destination location, which is where the problem is. The self-containedness of drawGrid with respect to pen state does not extend to opacity state — and the destination context is materially different from the source context in precisely that dimension.

Required fix — one line:

            }  // end bg image block
+           bp.setOpacity(1.0);
            drawGrid(bp, specRect);
            m_overlayBgNeedsUpload = true;

Everything else

With the above issue corrected, the remainder of the PR is sound:

• The fix concept is correct — m_overlayBg is the right destination layer
• Both overlay layers rebuild on the same dirty gate — no extra invalidation wiring needed
• The static overlay removal leaves no painter-state side effects for drawBandPlan or any subsequent call
• The PR template is fully completed; Principle XI is cited and appropriate
• Commits are signed per the checklist
• Scope is one file, one function relocated, eight lines changed — blast radius is minimal

Once the opacity reset is in place, I will approve on the next revision.

Human-directed independent review (Claude / @NF0T)

Follow-up to the REQUEST_CHANGES posted earlier today — the one blocking issue is resolved.

Blocking issue from prior review — fixed:
bp.setOpacity(1.0) is now present immediately after the bg-image conditional block and before drawGrid(bp, specRect) in m_overlayBg. The opacity reset is correctly placed so the grid renders at full opacity regardless of whether a background image is configured and what m_bgOpacity is set to. The comment at the insertion site accurately documents the invariant and cites the review. ✓

Everything else — carried forward from prior verification:

• Render order (waterfall → bg → grid → FFT → static overlay) confirmed correct ✓
• Zero-cost claim holds (same m_overlayStaticDirty gate for both layers) ✓
• Removal of drawGrid from m_overlayStatic leaves no painter-state side effects for drawBandPlan ✓
• Single-file Tier 3 change ✓

Approving.