Skip to content

Conversation

@servantftransperfect
Copy link
Contributor

Remap was zeroing rgb values when alpha was zero. We want to keep the masked values but only mask it with alpha = 0.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a bug in the remapInter function where RGB values were being zeroed when the alpha channel was zero. The fix removes alpha-based weight adjustment during pixel interpolation, preserving RGB values while still allowing alpha masking.

Key Changes:

  • Removed the conditional alpha weight multiplication for RGBAfColor pixels during area interpolation
  • This ensures RGB values are preserved in the weighted average regardless of alpha values
  • Alpha channel itself will still be averaged, correctly indicating masking when all source pixels have alpha=0

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 163 to 168
const P & pix = source(by, bx);
double weight = yocc * xocc;

if constexpr (std::is_same<P, RGBAfColor>())
{
weight *= pix.a();
}

sum += pix * weight;
wsum += weight;
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This bug fix lacks test coverage. The image module has comprehensive test files (image_test.cpp, filtering_test.cpp, resampling_test.cpp), but there are no tests for the remap functionality. Consider adding a test case that verifies:

  1. RGB values are preserved when alpha=0 (the bug being fixed)
  2. The weighted averaging still works correctly for downsampling
  3. Alpha channel handling is correct for various input scenarios

Copilot uses AI. Check for mistakes.
@CedricThebault CedricThebault merged commit 83e219e into develop Dec 2, 2025
3 of 4 checks passed
@CedricThebault CedricThebault deleted the fix/remap branch December 2, 2025 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants