Add automatic per-camera color matching at the seam - #427
Open
RufanMelfor wants to merge 1 commit into
Open
Conversation
|
All contributors have signed the CLA. Thank you! |
…g at the seam Reimplemented from scratch against the current architecture, like the seam-positioning feature - color_match.rs, blur.wgsl, and multiband_composite.wgsl from the original design do not exist anywhere in this codebase currently, only the fisheye.wgsl uniform slots they used to drive (color_scale/color_offset_blend), which are wired but always hardcoded to identity. Periodically samples a coarse grid in the seam-adjacent band of each camera's raw frame and derives a small, EMA-smoothed, clamped per-camera YUV offset that nudges both cameras toward their shared mean - same idea as the original, reduced scope (offset only, no separate multiplicative scale; fixed internal constants for band width/grid/ interval/smoothing instead of individually GUI-tunable knobs). Sampling geometry note (see color_match.rs's module doc for the full rationale): deliberately reuses the same forward-KB4 primitive and normalized-intrinsics convention as stitch/geometry.rs's PlaneMap and fisheye.wgsl's fs_main - NOT lens::undistorted_to_distorted, which uses a different (halved-FOV, plane-fitted) intrinsics convention built for the single-camera undistort preview and would silently sample the wrong region here. Getting this exact mapping wrong is the class of bug this feature's own history already hit twice upstream in its prior form. Correctness note, found via this codebase's own agreement-oracle tests (not assumed): this codebase treats CPU/GPU render agreement as load-bearing and test-gated, but color matching is GPU-only (like the original's own "no CPU pixel access on BGRA/zero-copy paths" limitation) with no CPU-executor mirror. Defaulting it on (matching the original design) silently broke 7 existing agreement tests during development the moment a source had non-uniform seam-band content - caught by running the full suite, not shipped. Fixed by defaulting color_match_enabled to false (opt-in), unlike the original's "on by default" - a deliberate, documented difference from the upstream design, not an oversight. Wiring: StitchPipeline::set_color_match_enabled -> Executor (no-op on the CPU arm) -> StitchCore -> StitchSession -> StitchJob::color_match() builder -> `reco stitch --color-match` -> reco-gui "Auto color match (experimental)" checkbox (Stitching panel and export), mirroring show_seam_line's wiring exactly. Verified: cargo fmt --check clean; cargo test -p reco-core (156 passed, including 3 new ColorMatchState unit tests proving the correction moves in the right direction, is symmetric, and converges to the configured clamp; only the 2 pre-existing CUDA hardware-gap failures, unrelated); cargo check clean across the whole workspace (reco-obs excluded, pre-existing unrelated OBS-SDK build gap); clippy clean for this change's own code (blocked only by the same 4 pre-existing issues on upstream's own current main already fixed in the separate fix/d3d11-stage-frame-unsafe branch). Not included: the multi-band spatial seam blend from the original design (blur.wgsl/multiband_composite.wgsl, an alternate seam-blend algorithm) - independent enough from color matching to be its own follow-up PR rather than bundled here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
RufanMelfor
force-pushed
the
feat/color-matching-multiband
branch
from
July 16, 2026 10:37
c433ded to
aea0812
Compare
Contributor
Author
|
I have read the CLA Document and I hereby sign the CLA |
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.
Reimplemented from scratch against the current architecture, like the
seam-positioning PR —
color_match.rs,blur.wgsl, andmultiband_composite.wgslfrom the original design don't exist anywherein the current codebase, only the
fisheye.wgsluniform slots they usedto drive (
color_scale/color_offset_blend), which are wired but alwayshardcoded to identity today.
Periodically samples a coarse grid in the seam-adjacent band of each
camera's raw frame and derives a small, EMA-smoothed, clamped per-camera
YUV offset that nudges both cameras toward their shared mean — same idea
as the original design, reduced in scope (offset only, no separate
multiplicative scale; fixed internal constants for band width/grid/
interval/smoothing rather than individually GUI-tunable knobs).
Sampling-geometry note (full rationale in
color_match.rs's moduledoc): deliberately reuses the same forward-KB4 primitive and
normalized-intrinsics convention as
stitch/geometry.rs'sPlaneMapandfisheye.wgsl'sfs_main— notlens::undistorted_to_distorted, whichuses a different (halved-FOV, plane-fitted) intrinsics convention built
for the single-camera undistort preview and would silently sample the
wrong region here. Getting this mapping wrong is a bug class this
feature's own history already hit twice in its prior form.
Correctness note, found via this codebase's own agreement-oracle
tests, not assumed: this codebase treats CPU/GPU render agreement as
load-bearing and test-gated, but color matching is GPU-only (like the
original design's own "no CPU pixel access on BGRA/zero-copy paths"
limitation), with no CPU-executor mirror. Defaulting it on (matching the
original design) silently broke 7 existing agreement tests during
development the moment a source had non-uniform seam-band content —
caught by running the full suite, not shipped. Fixed by defaulting
color_match_enabledtofalse(opt-in), unlike the original's "on bydefault" — a deliberate, documented departure from the original design,
not an oversight.
Wiring:
StitchPipeline::set_color_match_enabled→Executor(no-op onthe CPU arm) →
StitchCore→StitchSession→StitchJob::color_match()builder →
reco stitch --color-match→ reco-gui "Auto color match(experimental)" checkbox, mirroring
show_seam_line's wiring exactly.Verified:
fmt --checkclean;cargo test -p reco-core(154 passed,including 3 new
ColorMatchStateunit tests proving the correction movesin the right direction, is symmetric, and converges to the configured
clamp; only the 2 pre-existing CUDA hardware-gap failures, unrelated);
full workspace build clean.
Not included: the multi-band spatial seam blend from the original
design (
blur.wgsl/multiband_composite.wgsl, an alternate seam-blendalgorithm) — independent enough from color matching to be its own
follow-up PR rather than bundled here.