Skip to content

Report content_rect from the CPU engine - #893

Merged
marcinz606 merged 1 commit into
mainfrom
fix/cpu-content-rect
Aug 18, 2026
Merged

Report content_rect from the CPU engine#893
marcinz606 merged 1 commit into
mainfrom
fix/cpu-content-rect

Conversation

@marcinz606

Copy link
Copy Markdown
Owner

The bug

Only the GPU engine writes content_rect. _on_render_finished does last_metrics.update(metrics) — a merge, not a replace — so the key kept the previous GPU render's value straight through a CPU render.

That is harmless while the two agree, and they usually do. With a crop active they do not: the GPU crops and then upscales the cropped region to the preview render size (gpu_engine.py:1673), reporting a rect for the upscaled buffer, while the CPU returns native cropped pixels. Measured on samples/20260619SP_EKTAR100_120_1_09_ME_4000PPI.tif:

gpu=True   buffer (1600,1184)   content_rect (0,0,1184,1600)
gpu=False  buffer (800,592)     content_rect (0,0,1184,1600)   <- inherited, 2x the buffer

main_window passes that rect to canvas.update_buffer whenever no border or paper aspect is set, which is the common case — the border branch is the only one that recomputes it. widget.py:362 get_pixel_rgb then maps normalized coords through the rect and clamps, so with the rect above every sample past 50% width pinned to the right edge: the white-balance picker and the colour readout sampled the wrong pixel. overlay.py:966,1425 map overlay coordinates through the same rect.

Reachable on any GPU-to-CPU switch with a crop active, including the automatic "GPU acceleration failed — using CPU" fallback, not only the manual toggle.

The fix

DarkroomEngine.process publishes content_rect too. No paper layout runs on that path — FinishProcessor/apply_carrier draw inside the frame rather than padding it — so the whole buffer is the picture and the honest report is None. Both engines now always write the key, so a render's metrics cannot inherit a stale one.

Verification

make all green.

tests/test_engine.py::test_the_engine_always_reports_content_rect seeds a stale rect into the context and asserts the engine clears it. Confirmed it fails without the one-line change and passes with it.

End to end on the real app, driving the exact switch that triggered it:

gpu buffer=(1600, 1184) content_rect=(0, 0, 1184, 1600)
PASS GPU still publishes its layout rect
cpu buffer=(800, 592) content_rect=None
PASS CPU render clears the stale GPU content_rect

Context

Found while chasing what looked like a CPU/GPU disagreement over crop_rect, flagged in #889. That part turned out to be a misreading on my side — both engines crop, they differ only in output resolution (correlation of the displayed picture +0.9995). This is the real defect that investigation turned up.

Only the GPU engine wrote content_rect, and the controller merges each
render's metrics into last_metrics rather than replacing them, so the key
kept the previous GPU render's value through a CPU render. With a crop
active the two disagree: the GPU upscales the cropped region to the preview
render size and reports that rect, while the CPU returns native cropped
pixels, so the inherited rect described an area twice the buffer.

The canvas maps picker and overlay coordinates through it (widget.get_pixel_rgb
clamps, overlay maps content-normalized coords), so after a GPU-to-CPU switch
the colour readout sampled the wrong pixel. The automatic GPU fallback reaches
this too, not just the manual toggle.

No paper layout runs on the CPU path, so it reports None. Both engines now
always publish the key, and a render's metrics no longer inherit a stale one.
@marcinz606
marcinz606 merged commit dbe2472 into main Aug 18, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant