perf(gpu): make interactive geometry drags (fine rotation, keystone, crop tool) fast - #1081
Merged
marcinz606 merged 3 commits intoSep 12, 2026
Conversation
…ystone The GPU engine's shared meter grid built its buffer by cropping to the ROI, then running fine rotation (cv2.warpAffine) and keystone (cv2.warpPerspective) at full resolution, and only then downsampling for the meters that read it. Both warps are full-frame resamples whose cost scales with pixel count, so warping the full-res crop just to shrink it away spent the expensive part on pixels the analysis never sees -- most noticeably on every step of an interactive fine-rotation or keystone drag with Auto Exposure or Auto Normalize Contrast on, since each step invalidates this block's cache. Reordered to downsample first. Extracted into _build_analysis_source, a pure function, so the ordering is unit-tested directly rather than only reachable through a live GPU render. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
_analysis_cache_key and the shared prefilter's own key both folded in settings.geometry wholesale. Fine rotation, keystone and distortion reshuffle pixels within the analyzed region (_build_analysis_source applies them to the meter's own buffer) without changing what region it is, but every micro-step of dragging one of those sliders still counted as a changed key -- unlike a density or grade drag, which the same cache already treats as free. Every such step re-ran the full bounds/anchor/textural measurement from scratch, live-profiled at roughly 100ms per step regardless of Auto Exposure or Auto Normalize Contrast being on, since basic bounds analysis has no such toggle. Keying on only the fields that actually select the analyzed region -- rotation, flips, crop_rect, autocrop_offset -- drops that to about 1-3ms per step in the same profile. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
PabloRuizCuevas
changed the title
The crop tool shows the whole rotated frame, ignoring crop_rect, so every render while it's active was forced onto the CPU engine outright -- "sidestep the GPU engine's ROI-fused compute dispatch", per the comment this replaces. Fine rotation and keystone still change actual pixel content there (unlike the analysis-only case fixed earlier), so the CPU engine's own recompute is real work, just done without GPU parallelism: live-profiled at roughly 650ms per interactive step, worse than the GPU path even before its own fixes. Added full_frame to GPUEngine.process_to_texture: widens only the late-stage (toning/finish/layout) dispatch extent to the whole rotated frame, the same fallback every stage already takes with no crop_rect set. The meter, the contrast mask and the reported active_roi all stay on the real crop, so the crop tool's own overlay keeps tracking it and the print exposure matches what the CPU engine (and a plain crop, full_frame off) would compute -- parity- tested against the CPU engine's own output at the same tolerance the cropped GPU path already carries. _detect_invalidated_stage now also invalidates on a bare full_frame toggle: none of it shows up in a WorkspaceConfig diff, since it is a render parameter rather than a persisted field. Combined with the earlier analysis-cache-key fix, an interactive fine- rotation drag with the crop tool active drops from ~650ms/step to ~5ms. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
PabloRuizCuevas
changed the title
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
2 participants
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.
Rotation at least in my computer was relatively slow, this PR partially address the issue making it way quicker to react to user input, still not perfect.
from AI:
Three compounding costs in interactive geometry dragging, found by actually profiling a live drag rather than guessing.
Full-res warp before downsampling. The GPU engine's shared meter buffer cropped to the ROI, then ran fine rotation (
cv2.warpAffine) and keystone (cv2.warpPerspective) at full resolution, and only downsampled for the meters afterward. Reordered to downsample first; extracted into_build_analysis_source, a pure function, unit-tested directly.Analysis cache keyed on the whole geometry config.
_analysis_cache_key(and the shared prefilter's own key) folded insettings.geometrywholesale. Fine rotation, keystone and distortion reshuffle pixels within the analyzed region without changing what region it is, but every micro-step of dragging one of those sliders still counted as a changed key — unlike density/grade, which the same cache already treats as free. Live-profiled at roughly 100ms per step regardless of Auto Exposure or Auto Normalize Contrast being on. Keying on only the fields that actually select the analyzed region drops that to about 1-3ms per step.The crop tool forced the CPU engine outright. It shows the whole rotated frame ignoring
crop_rect, and the render was forced onto the CPU engine to get that — "sidestep the GPU engine's ROI-fused compute dispatch", per the comment this replaces. Live-profiled at ~650ms per interactive step, worse than the GPU path even before fixes 1-2. Addedfull_frametoGPUEngine.process_to_texture: widens only the late-stage (toning/finish/layout) dispatch extent to the whole frame, the same fallback every stage already takes with no crop set. The meter, the contrast mask and the reportedactive_roiall stay on the real crop, so the crop tool's overlay keeps tracking it and the exposure matches the CPU engine — parity-tested against it at the same tolerance the cropped GPU path already carries. Combined with fix 2, an interactive fine-rotation drag with the crop tool active drops from ~650ms/step to ~5ms.