Mirrors/PowerToys
1
0
Fork 0
mirror of https://github.com/microsoft/PowerToys.git synced 2025-12-15 03:07:56 +01:00
Code Issues Packages Projects Releases Wiki Activity

[ImageResizer] Fix Fill mode not cropping image when Shrink Only was engaged and scale was 1 (#43855)

Browse Source 
## Summary of the Pull Request
This PR fixes an Image Resizer issue where **Fill** mode operations were
silently aborted when **Shrink Only** was enabled (the default) and
scale was 1.0 on one dimension, resulting in files that were renamed
according to the intended target size but which actually contained the
original, unmodified image.

This also fixes a latent bug regarding square images and the **Ignore
Orientation** setting, and improves the readability of the core
`Transform` method.

<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist

- [x] Closes: #43772
<!-- - [ ] Closes: #yyy (add separate lines for additional resolved
issues) -->
- [ ] **Communication:** I've discussed this with core contributors
already. If the work hasn't been agreed, this work might be rejected
- [ ] **Tests:** Added/updated and all pass
- [ ] **Localization:** All end-user-facing strings can be localized
- [ ] **Dev docs:** Added/updated
- [ ] **New binaries:** Added on the required places
- [ ] [JSON for
signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json)
for new binaries
- [ ] [WXS for
installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs)
for new binaries and localization folder
- [ ] [YML for CI
pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml)
for new test projects
- [ ] [YML for signed
pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml)
- [ ] **Documentation updated:** If checked, please file a pull request
on [our docs
repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys)
and link it here: #xxx

<!-- Provide a more detailed description of the PR, other things fixed,
or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
### Fix **Shrink Only** logic preventing the correct cropping of images
**Issue:**
When using **Fill** mode, the scaling factor is calculated based on the
larger dimension to ensure the image fills the target box. In scenarios
where one dimension matches the target and the other overflows (e.g.
shrinking a 100x100 pixel image to 50x100), the calculated scale factor
is `1.0`.

The previous `ShrinkOnly` logic included this:

```csharp
if (_settings.ShrinkOnly
        && _settings.SelectedSize.Unit != ResizeUnit.Percent
        && (scaleX >= 1 || scaleY >= 1))
    {
        return source;
    }
```

This correctly prevents `ShrinkOnly` operations from returning upscaled
result images, but it also exits too early for cases where the user is
cropping the image across one dimension only, leaving the other at scale
1. Effectively, the later cropping code is never run and instead of
returning the cropped image, the original is returned. The _intended_
target dimensions are correct, which results in the filename parts not
matching the resulting image size.

**Fix:**
The logic has been split between upscaling and cropping, so:

1. If the scale on either dimension is > `1.0`, return the source
(explicitly preventing upscaling for **Shrink Only** mode).
2. If the scale is <= `1.0` then check if the original dimensions exceed
the target dimensions. If a crop is required, proceed with it even if
the scale is exactly `1.0`.

### Fix for square images triggering orientation swap
**Issue:**
The "Ignore Orientation" check in the original code used a compound
boolean check:

```csharp
(originalWidth < originalHeight != width < height)
```

This clever but less than readable statement detects orientation
mismatches. The section also includes a logic issue. When the original
image was square, `originalWidth < originalHeight` evaluated to `false`,
treating it as Landscape. If the target dimensions were Portrait, the
logic detected a mismatch and swapped the target dimensions incorrectly,
which would crop the height instead of the width.

'Fortunately' this bug was masked by the first bug, as the crop code
would never be reached anyway.

**Fix:**
The orientation detection routine was refactored to explicitly check for
Landscape vs. Portrait states. Square images are now naturally excluded,
as they have neither Landscape nor Portrait orientations. This now
prevents the dimensions from being swapped.

### Refactoring/readability
The main `Transform` method has been cleaned up:

- Replaced widespread use of `var` with `double` and `int` for dimension
and scale calculations.
- Replaced the non-obvious XOR orientation check (`a < b != c < d`) with
named booleans (`isInputLandscape`, `isTargetPortrait` etc.) to make the
intent more self-documenting.
- New and expanded comments throughout.

<!-- Describe how you validated the behavior. Add automated tests
wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Three new unit tests have been added to `ResizeOperationTests.cs` to
cover the **Fill** mode edge cases:

1. `TransformHonorsFillWithShrinkOnlyWhenCropRequired`: Verifies than an
image requiring a crop but no scaling is processed correctly (tests that
the original bug report is resolved).
2. `TransformHonorsFillWithShrinkOnlyWhenUpscaleAttempted`: Confirms
that when `ShrinkOnly` is set, any upscaling operations are still
blocked.
3. `TransformHonorsFillWithShrinkOnlyWhenNoChangeRequired`: Verifies
that the system returns the source if neither scaling nor cropping is
required.

I also manually verified the bug fix with a test 4000 x 6000 pixel
source file with 1920 x `Auto` **Fill** mode and **Shrink Only**
settings, mirroring the original user's settings, and their source and
target dimensions.
This commit is contained in:
Dave Rayment
2025-12-08 01:45:46 +00:00
committed by GitHub
parent 2b0ecc2979
commit 7e791f2815
2 changed files with 146 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

87
src/modules/imageresizer/tests/Models/ResizeOperationTests.cs
View File

@@ -394,6 +394,93 @@ namespace ImageResizer.Models
});
}
[TestMethod]
public void TransformHonorsFillWithShrinkOnlyWhenCropRequired()
{
// Testing original 96x96 pixel Test.jpg cropped to 48x96 (Fill mode).
//
// ScaleX = 48/96 = 0.5
// ScaleY = 96/96 = 1.0
// Fill mode takes the max of these = 1.0.
//
// Previously, the transform logic saw the scale of 1.0 and returned the
// original dimensions. The corrected logic recognizes that a crop is
// required on one dimension and proceeds with the operation.
var operation = new ResizeOperation(
"Test.jpg",
_directory,
Settings(x =>
{
x.ShrinkOnly = true;
x.SelectedSize.Fit = ResizeFit.Fill;
x.SelectedSize.Width = 48;
x.SelectedSize.Height = 96;
}));
operation.Execute();
AssertEx.Image(
_directory.File(),
image =>
{
Assert.AreEqual(48, image.Frames[0].PixelWidth);
Assert.AreEqual(96, image.Frames[0].PixelHeight);
});
}
[TestMethod]
public void TransformHonorsFillWithShrinkOnlyWhenUpscaleAttempted()
{
// Confirm that attempting to upscale the original image will return the
// original dimensions when Shrink Only is enabled.
var operation = new ResizeOperation(
"Test.jpg",
_directory,
Settings(x =>
{
x.ShrinkOnly = true;
x.SelectedSize.Fit = ResizeFit.Fill;
x.SelectedSize.Width = 192;
x.SelectedSize.Height = 192;
}));
operation.Execute();
AssertEx.Image(
_directory.File(),
image =>
{
Assert.AreEqual(96, image.Frames[0].PixelWidth);
Assert.AreEqual(96, image.Frames[0].PixelHeight);
});
}
[TestMethod]
public void TransformHonorsFillWithShrinkOnlyWhenNoChangeRequired()
{
// With a scale of 1.0 on both axes, the original should be returned.
var operation = new ResizeOperation(
"Test.jpg",
_directory,
Settings(x =>
{
x.ShrinkOnly = true;
x.SelectedSize.Fit = ResizeFit.Fill;
x.SelectedSize.Width = 96;
x.SelectedSize.Height = 96;
}));
operation.Execute();
AssertEx.Image(
_directory.File(),
image =>
{
Assert.AreEqual(96, image.Frames[0].PixelWidth);
Assert.AreEqual(96, image.Frames[0].PixelHeight);
});
}
[TestMethod]
public void GetDestinationPathUniquifiesOutputFilename()
{

78
src/modules/imageresizer/ui/Models/ResizeOperation.cs
View File

@@ -167,49 +167,89 @@ namespace ImageResizer.Models
private BitmapSource Transform(BitmapSource source)
{
var originalWidth = source.PixelWidth;
var originalHeight = source.PixelHeight;
var width = _settings.SelectedSize.GetPixelWidth(originalWidth, source.DpiX);
var height = _settings.SelectedSize.GetPixelHeight(originalHeight, source.DpiY);
int originalWidth = source.PixelWidth;
int originalHeight = source.PixelHeight;
if (_settings.IgnoreOrientation
&& !_settings.SelectedSize.HasAuto
&& _settings.SelectedSize.Unit != ResizeUnit.Percent
&& originalWidth < originalHeight != (width < height))
// Convert from the chosen size unit to pixels, if necessary.
double width = _settings.SelectedSize.GetPixelWidth(originalWidth, source.DpiX);
double height = _settings.SelectedSize.GetPixelHeight(originalHeight, source.DpiY);
// Swap target width/height dimensions if orientation correction is required.
// Ensures that we don't try to fit a landscape image into a portrait box by
// distorting it, unless specific Auto/Percent rules are applied.
bool canSwapDimensions = _settings.IgnoreOrientation &&
!_settings.SelectedSize.HasAuto &&
_settings.SelectedSize.Unit != ResizeUnit.Percent;
if (canSwapDimensions)
{
var temp = width;
width = height;
height = temp;
bool isInputLandscape = originalWidth > originalHeight;
bool isInputPortrait = originalHeight > originalWidth;
bool isTargetLandscape = width > height;
bool isTargetPortrait = height > width;
// Swap dimensions if there is a mismatch between input and target.
if ((isInputLandscape && isTargetPortrait) ||
(isInputPortrait && isTargetLandscape))
{
(width, height) = (height, width);
}
}
var scaleX = width / originalWidth;
var scaleY = height / originalHeight;
double scaleX = width / originalWidth;
double scaleY = height / originalHeight;
// Normalize scales based on the chosen Fit/Fill mode.
if (_settings.SelectedSize.Fit == ResizeFit.Fit)
{
// Fit: use the smaller scale to ensure the image fits within the target.
scaleX = Math.Min(scaleX, scaleY);
scaleY = scaleX;
}
else if (_settings.SelectedSize.Fit == ResizeFit.Fill)
{
// Fill: use the larger scale to ensure the target area is fully covered.
// This often results in one dimension overflowing, which is handled by
// cropping later.
scaleX = Math.Max(scaleX, scaleY);
scaleY = scaleX;
}
if (_settings.ShrinkOnly
&& _settings.SelectedSize.Unit != ResizeUnit.Percent
&& (scaleX >= 1 || scaleY >= 1))
// Handle Shrink Only mode.
if (_settings.ShrinkOnly && _settings.SelectedSize.Unit != ResizeUnit.Percent)
{
return source;
// Shrink Only mode should never return an image larger than the original.
if (scaleX > 1 || scaleY > 1)
{
return source;
}
// Allow for crop-only when in Fill mode.
// At this point, the scale is <= 1.0. In Fill mode, it is possible for
// the scale to be 1.0 (no resize needed) while the target dimensions are
// smaller than the originals, requiring a crop.
bool isFillCropRequired = _settings.SelectedSize.Fit == ResizeFit.Fill &&
(originalWidth > width || originalHeight > height);
// If the scale is exactly 1.0 and a crop isn't required, we return the
// original image to prevent a re-encode.
if (scaleX == 1 && scaleY == 1 && !isFillCropRequired)
{
return source;
}
}
// Apply the scaling.
var scaledBitmap = new TransformedBitmap(source, new ScaleTransform(scaleX, scaleY));
// Apply the centered crop for Fill mode, if necessary. Applies when Fill
// mode caused the scaled image to exceed the target dimensions.
if (_settings.SelectedSize.Fit == ResizeFit.Fill
&& (scaledBitmap.PixelWidth > width
|| scaledBitmap.PixelHeight > height))
{
var x = (int)(((originalWidth * scaleX) - width) / 2);
var y = (int)(((originalHeight * scaleY) - height) / 2);
int x = (int)(((originalWidth * scaleX) - width) / 2);
int y = (int)(((originalHeight * scaleY) - height) / 2);
return new CroppedBitmap(scaledBitmap, new Int32Rect(x, y, (int)width, (int)height));
}