PowerToys

mirror of https://github.com/microsoft/PowerToys.git synced 2026-04-03 09:46:54 +02:00

Files

Dave Rayment 0e922a4dcb [ImageResizer] Fix issue where settings could be changed during a batch resize (#42163 )

<!-- Enter a brief description/summary of your PR here. What does it
fix/what does it change/how was it tested (even manually, if necessary)?
-->
## Summary of the Pull Request
This fixes an issue where the Image Resizer settings were reloaded for
every resize operation in a multi-file batch. This could result in
inconsistent resize results if the Settings application or the user
interacted with the settings and the properties were changed, even
temporarily.

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

- [x] Closes: #42116, #35114
- [ ] **Communication:** I've discussed this with core contributors
already. If the work hasn't been agreed, this work might be rejected
- [x] **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
Corrects an issue in `ResizeBatch.Execute()` where `Settings.Default`
was used:

```csharp
    protected virtual void Execute(string file)
        => new ResizeOperation(file, DestinationDirectory, Settings.Default).Execute();
```

Unfortunately, `Settings.Default` is poorly named, and is not a constant
property. Instead it actually calls `Reload()`, which loads the settings
JSON file and re-processes it fully:

```csharp
        [JsonIgnore]
        public static Settings Default
        {
            get
            {
                defaultInstance.Reload();
                return defaultInstance;
            }
        }
```

If the settings are changed part-way through a resize batch operation
(even scrolling through the presets will set `selectedIndex` and save it
back to disk), the batch resize operation may switch to a different
resize behaviour, giving inconsistent results. The chances of this
occurring increase with the length of the batch operation.

### Solution
The solution is to set the `Settings` outside of the main loop and use
that for every resize operation, which is what this PR does.

`Process` is altered to load the `Settings` at the start and pass that
to `Execute` on each iteration of the batch loop:

```csharp
        public IEnumerable<ResizeError> Process(Action<int, double> reportProgress, CancellationToken cancellationToken)
        {
            double total = Files.Count;
            int completed = 0;
            var errors = new ConcurrentBag<ResizeError>();
            var settings = Settings.Default;

            ...

                        Execute(file, settings);
```

`Execute` is updated to accept a `Settings` object instead of using
`Settings.Default`:

```csharp
    protected virtual void Execute(string file, Settings settings)
        => new ResizeOperation(file, DestinationDirectory, settings).Execute();
```

### Additional changes
The batch-related unit tests failed after the above change. I updated
the Mock `Execute` to reflect the use of the `Settings` parameter. Also,
the `Settings` class was unfortunately bound to the WPF UI, and could
not be instantiated during unit testing. I've refactored this so it can
be instantiated with or without an `App.Current.Dispatcher`. Ther's a
new `ReloadCore()` method which just contains the extracted property
setting code.

None of the unit tests currently use the settings themselves, but at
least the capability is now there.

I also removed the setting of the `MaxDegreeOfParallelism` option in the
`Parallel.For()`. When left at its default, the runtime will use the
appropriate number of threads, and the heuristic it uses may not
necessarily equal the number of processor cores in the system.

<!-- Describe how you validated the behavior. Add automated tests
wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
- Ensured all existing unit tests passed.
- Tested with multiple runs of resizing 200 image files while browsing
and changing the settings. This was no longer able to alter the ongoing
resize operations.
- Checked that the settings could still be amended via the Settings app.

---------

Co-authored-by: Gordon Lam (SH) <yeelam@microsoft.com>

2025-11-06 19:55:47 +08:00