fix(colormap2): follow pan via GPU translation instead of freezing#36
Merged
Merged
Conversation
jeandet
force-pushed
the
fix/colormap2-pan-translation
branch
from
da44d56 to
e10d34c
Compare
jeandet
changed the title
There was a problem hiding this comment.
Pull request overview
Fixes QCPColorMap2 panning so viewport-dependent spectrograms no longer “freeze” during axis-synchronized pans, by dirtying the layer on viewport changes and enabling the existing skip-on-translate compositor path (GPU texture translation), including correct behavior on log axes.
Changes:
- Mark
QCPColorMap2’s layer dirty inonViewportChanged()sosetRange-driven pans repaint/compose during drags. - Implement
QCPColorMap2::stallPixelOffset()(with rendered-range tracking) to translate the last-rendered texture on pure pans instead of repainting/reuploading. - Add log-scale-aware zoom-vs-pan detection (
qcp::axisRangeSizeRatio) and newtest-paintbuffercoverage for pan dirtying and translation behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/auto/test-paintbuffer/test-paintbuffer.h | Adds new test declarations for colormap2 pan dirtying and stall offset behavior. |
| tests/auto/test-paintbuffer/test-paintbuffer.cpp | Adds regression tests validating layer dirtying on pan and GPU translation offsets (linear + log Y). |
| src/plottables/plottable-colormap2.h | Adds stallPixelOffset() override and rendered-range state used for translation. |
| src/plottables/plottable-colormap2.cpp | Dirts layer on viewport changes; implements translation offset logic; records rendered ranges after draw. |
| src/painting/viewport-offset.h | Adds axisRangeSizeRatio helper for scale-aware (log) pan/zoom discrimination. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1
to
+3
| #include <vector> | ||
| #include "test-paintbuffer.h" | ||
| #include <painting/viewport-offset.h> |
A spectrogram (QCPColorMap2) froze in place while the axes scrolled during a pan, snapping only when panning stopped — worst on large / log-Y energy spectrograms. QCPHistogram2D (ViewportIndependent) was unaffected. Two causes, two parts to the fix: 1. Engage the layer on pan. QCPColorMap2 is ViewportDependent: a range change kicks an async resample and the layer was only redrawn on the pipeline's finished signal. Interactive drags call markAffectedLayersDirty(), but a pan driven by set_range (axis synchronisation) does not — so the synced spectrogram was never marked dirty mid-drag and stayed frozen. Mark the layer dirty in onViewportChanged(). 2. Translate, don't repaint. Marking dirty alone would repaint the CPU staging buffer and re-upload the (viewport-sized) texture every frame — exactly what the skip-on-translate / Metal pan optimisation avoids for line graphs. Give QCPColorMap2 a stallPixelOffset() override so the compositor shifts the existing texture by the pan delta instead (no repaint, no re-upload), the same fast path QCPGraph2/QCPMultiGraph use. A fresh resample / data / gradient change forces a real redraw (guards on mapImageInvalidated). The zoom-rejection in stallPixelOffset is scale-aware (new qcp::axisRangeSizeRatio): a pure pan on a log axis changes the *linear* range size and would otherwise be misread as a zoom and rejected — which is why log-Y spectrograms were the worst affected. Tests (test-paintbuffer): pan dirties the colormap layer (fail before, pass after); stallPixelOffset is non-null on a pan incl. log-Y, and null on a zoom. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jeandet
force-pushed
the
fix/colormap2-pan-translation
branch
from
e10d34c to
c97003d
Compare
Member
Author
|
Addressed: reordered |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A spectrogram (
QCPColorMap2) stays stuck in place while the axes scroll during a pan, snapping to the right place only when panning stops. Worst on large / log-Y energy spectrograms.QCPHistogram2D(ViewportIndependent) is unaffected.Root cause (two parts)
QCPColorMap2isViewportDependent: each range change kicks an async resample, and the layer was only redrawn on the pipeline'sfinishedsignal.markAffectedLayersDirty(), but a pan driven byset_range(axis synchronisation across stacked plots) does not — so the synced spectrogram's layer was never marked dirty during the drag → frozen until the resample landed. (A tracer showed 0draw()calls during a fast drag vs 40 for the histogram.)Fix
onViewportChanged()so aset_range-driven pan engages it.QCPColorMap2::stallPixelOffset()so the compositor translates the existing texture by the pan delta (no repaint, no re-upload) — the same skip-on-translate fast pathQCPGraph2/QCPMultiGraphuse. A fresh resample/data/gradient change forces a real redraw (mapImageInvalidatedguard).qcp::axisRangeSizeRatio): a pure pan on a log axis changes the linear range size and would otherwise be misread as a zoom and rejected — which is why log-Y spectrograms were worst hit.Net: colormaps now pan as cheaply as line graphs (GPU texture shift), honoring the macOS Metal pan optimisation rather than re-uploading each frame.
Tests (
test-paintbuffer)colormap2_panDirtiesLayerBuffer{,LogY}— a pan dirties the colormap's layer (fail before this fix, pass after).colormap2_stallOffsetOnPan/…OnLogYPan/…NullOnZoom— offset is non-null on a pan (incl. log-Y) and null on a genuine zoom.No new regressions in the auto-test suite (the 4 pre-existing RHI-context failures are unrelated and present on
main).🤖 Generated with Claude Code