Skip to content

made FilterQuality configurable for Resizetizer#25686

Closed
thisisthekap wants to merge 2 commits intodotnet:mainfrom
thisisthekap:br_25683_Resizetizer_make_FilterQuality_configurable
Closed

made FilterQuality configurable for Resizetizer#25686
thisisthekap wants to merge 2 commits intodotnet:mainfrom
thisisthekap:br_25683_Resizetizer_make_FilterQuality_configurable

Conversation

@thisisthekap
Copy link
Copy Markdown
Contributor

@thisisthekap thisisthekap commented Nov 5, 2024

Description of Change

Made FilterQuality configurable for Resizetizer. Default value is SKFilterQuality.High (like it was hard coded before).

Issues Fixed

Fixes #25750

Documentation

Adapted documentation in dotnet/docs-maui#2604

@thisisthekap thisisthekap requested a review from a team as a code owner November 5, 2024 11:00
@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Nov 5, 2024
@jsuarezruiz
Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 3 pipeline(s).

@thisisthekap
Copy link
Copy Markdown
Contributor Author

@jsuarezruiz I think that the failing CI is unrelated to my changes.

@rmarinho
Copy link
Copy Markdown
Member

rmarinho commented Nov 8, 2024

/rebase

@rmarinho rmarinho added this to the .NET 9 SR1.1 milestone Nov 8, 2024
@github-actions github-actions bot force-pushed the br_25683_Resizetizer_make_FilterQuality_configurable branch from 24ba86c to 7cfbdb5 Compare November 8, 2024 14:25
@rmarinho
Copy link
Copy Markdown
Member

rmarinho commented Nov 8, 2024

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 3 pipeline(s).

@mattleibow
Copy link
Copy Markdown
Member

How does this fix the issue? The filter quality should just be configuring the antialiasing and similar. The image should be the same size.

Are the generated images smaller after this PR?

@thisisthekap
Copy link
Copy Markdown
Contributor Author

This PR enables us to set the quality to 'None' which was the old default behavior. For our case, this reduces our app size tremendously. Later on we will try to fix our image resolutions to have a working setup with better antialiasing. But for the sake of keeping the old behavior, we need this PR.

@rmarinho
Copy link
Copy Markdown
Member

/rebase

@github-actions github-actions bot force-pushed the br_25683_Resizetizer_make_FilterQuality_configurable branch from 7cfbdb5 to 69b1556 Compare November 14, 2024 17:59
@rmarinho
Copy link
Copy Markdown
Member

/azp run maui-public maui-uitests-public

@azure-pipelines
Copy link
Copy Markdown

No pipelines are associated with this pull request.

@rmarinho
Copy link
Copy Markdown
Member

/azp run maui-public

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Copy Markdown
Member

@rmarinho rmarinho left a comment

Choose a reason for hiding this comment

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

Build seems to be failing ...

D:\a\_work\1\s\eng\ILRepack.targets(126,3): error MSB3073: The command ""D:\a\_work\1\s\eng\ILRepack.exe" /out:"D:\a\_work\1\s\artifacts\bin\Resizetizer\Release\netstandard2.0\ILRepacked\Microsoft.Maui.Resizetizer.dll" "D:\a\_work\1\s\artifacts\bin\Resizetizer\Release\netstandard2.0\Microsoft.Maui.Resizetizer.dll" /ver:42.42.42.42424 /internalize /keyfile:"D:\a\_work\1\s\eng/microsoft.maui.controls.snk" /lib:"C:\Users\cloudtest\.nuget\packages\netstandard.library\2.0.3\build\netstandard2.0\ref"" exited with code 1. [D:\a\_work\1\s\src\SingleProject\Resizetizer\src\Resizetizer.csproj]
    22 Warning(s)
    1 Error(s)

@thisisthekap
Copy link
Copy Markdown
Contributor Author

@rmarinho As I already included this PR in our custom build, I am not sure if my changes caused these build issues. Could you provide more detail on the build failure?

@mattleibow mattleibow added the do-not-merge Don't merge this PR label Nov 18, 2024
@mattleibow
Copy link
Copy Markdown
Member

This PR has some value in and of itself - the attribute will allow people to decided to render images at a lower quality. But, the only reason today is because higher quality is a large png that was upscaled.

See more: #25750 (comment)

I am thinking that maybe we first evaluate whether we should be upscaling (buth this app and resizetizer as a whole).

@mattleibow
Copy link
Copy Markdown
Member

Makes sense. Interesting tho that skia upscaling is better than ios default. I would have thought that it was the other way around.

So yeah, let me review.

@thisisthekap
Copy link
Copy Markdown
Contributor Author

thisisthekap commented Nov 19, 2024

@mattleibow Maybe an additional bool property AllowUpscale would be benefitial?

Edit: What I am thinking of is to have good default values, but to give the developers out there a highly customizable experience if they need to do so.

@albilaga
Copy link
Copy Markdown

@mattleibow I think this is also because we use the base highest image as the image. So want to clarify
If I have image with resolution 1000x1000 and we set BaseSize="250,250" what will happen? Is the image will be use our image as the highest resolution and then downscale for lower density or it is will be our image downscaled to 250x250 and then upscaled again to 1000x1000?

@dotnet dotnet deleted a comment from azure-pipelines bot Nov 27, 2024
@thisisthekap
Copy link
Copy Markdown
Contributor Author

@mattleibow @jsuarezruiz I would like to make another property configurable. The Encode call when saving a resized image is always using quality 100. I want to make that EncodingQuality configurable. Would it be feasible to add that in this PR, or should I create a separate one?

@MartyIX
Copy link
Copy Markdown
Contributor

MartyIX commented Apr 15, 2025

I would like to make another property configurable. The Encode call when saving a resized image is always using quality 100. I want to make that EncodingQuality configurable. Would it be feasible to add that in this PR, or should I create a separate one?

Given that it takes a long time to merge a PR, I think it makes sense to add it in this PR. However, could you comment a bit on why you need it?

@thisisthekap
Copy link
Copy Markdown
Contributor Author

As we replaced maui image processing with our own custom solution, we no longer need this or any other additional property. I am leaving this PR as is for the community (and Microsoft) to continue working on it.

@jfversluis jfversluis force-pushed the br_25683_Resizetizer_make_FilterQuality_configurable branch from d7f0b34 to 6edc7dc Compare August 28, 2025 09:40
@jfversluis
Copy link
Copy Markdown
Member

/azp run MAUI-public

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@jfversluis jfversluis self-assigned this Sep 9, 2025
@PureWeen
Copy link
Copy Markdown
Member

/rebase

@github-actions github-actions bot force-pushed the br_25683_Resizetizer_make_FilterQuality_configurable branch from 6edc7dc to a3e3521 Compare September 11, 2025 17:39
Copy link
Copy Markdown
Member

@mattleibow mattleibow left a comment

Choose a reason for hiding this comment

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

This is great I think. Filter quality is obsolete as it was not specific enough. The new way is to use SKSamplingOptions wich is a lot more powerful - but at the same time not a nice enum. It is a struct of things.

I am working on getting the docs live so i can link to things, but I wonder if we can have a look at a way to represent this:

	public enum SKFilterMode {
		Nearest = 0,
		Linear = 1,
	}
	public enum SKMipmapMode {
		None = 0,
		Nearest = 1,
		Linear = 2,
	}
	public partial struct SKCubicResampler
	{
		public static readonly SKCubicResampler Mitchell = new (1 / 3.0f, 1 / 3.0f);
		public static readonly SKCubicResampler CatmullRom = new (0.0f, 1 / 2.0f);
		public SKCubicResampler (float b, float c);
	}
	public partial struct SKSamplingOptions
	{
		public static readonly SKSamplingOptions Default;
		public SKSamplingOptions (SKFilterMode filter, SKMipmapMode mipmap);
		public SKSamplingOptions (SKFilterMode filter);
		public SKSamplingOptions (SKCubicResampler resampler);
		public SKSamplingOptions (int maxAniso);
	}

Or maybe it is overkill and we should rather just "come up with our own enum.

I sort of felt this one out:

		public static SKSamplingOptions ToSamplingOptions (this SKFilterQuality quality) =>
			quality switch {
				SKFilterQuality.None => new SKSamplingOptions (SKFilterMode.Nearest, SKMipmapMode.None),
				SKFilterQuality.Low => new SKSamplingOptions (SKFilterMode.Linear, SKMipmapMode.None),
				SKFilterQuality.Medium => new SKSamplingOptions (SKFilterMode.Linear, SKMipmapMode.Linear),
				SKFilterQuality.High => new SKSamplingOptions (SKCubicResampler.Mitchell),
				_ => throw new ArgumentOutOfRangeException (nameof (quality), $"Unknown filter quality: '{quality}'"),
			};

This will also solve this issue we had where "high" was not always high depending on the scaling direction:

// Typically the Mitchell cubic resampler is for upsampling
var sampling = new SKSamplingOptions(SKCubicResampler.Mitchell);

// and the bilinear with mipmaps is for downsampling.
var sampling = new SKSamplingOptions(SKFilterMode.Linear, SKMipmapMode.Linear);

Maybe we can have an enum that hides all this so we can try a few things and see. Maybe an enum with:

enum ResizeQuality {
    Auto,
    Best,
    Fastest,
    Smallest
}

or something...

@PureWeen PureWeen modified the milestones: .NET 10 Servicing, Backlog Mar 3, 2026
jfversluis added a commit that referenced this pull request Mar 19, 2026
Per mattleibow's review on PR #25686, replace the obsolete SKFilterQuality
enum with a clean MAUI-specific ResizeQuality enum that hides SkiaSharp
internals:

- Auto: Default behavior (bilinear + mipmaps, preserves backward compat)
- Best: Mitchell cubic resampler (highest quality)
- Fastest: Nearest neighbor (smallest file size, fastest processing)

MSBuild metadata name changed from FilterQuality to ResizeQuality.
All 25 new tests pass. 593 total passed (568 baseline + 25 new).
Same 20 pre-existing failures. Zero regressions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jfversluis
Copy link
Copy Markdown
Member

Stale by now, sorry about that. I opened #34559 to see if we can still get it done for .NET 11 :) thanks for all your efforts and input on this!

@jfversluis jfversluis closed this Mar 19, 2026
jfversluis added a commit that referenced this pull request Apr 7, 2026
Per mattleibow's review on PR #25686, replace the obsolete SKFilterQuality
enum with a clean MAUI-specific ResizeQuality enum that hides SkiaSharp
internals:

- Auto: Default behavior (bilinear + mipmaps, preserves backward compat)
- Best: Mitchell cubic resampler (highest quality)
- Fastest: Nearest neighbor (smallest file size, fastest processing)

MSBuild metadata name changed from FilterQuality to ResizeQuality.
All 25 new tests pass. 593 total passed (568 baseline + 25 new).
Same 20 pre-existing failures. Zero regressions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jfversluis added a commit that referenced this pull request Apr 9, 2026
Per mattleibow's review on PR #25686, replace the obsolete SKFilterQuality
enum with a clean MAUI-specific ResizeQuality enum that hides SkiaSharp
internals:

- Auto: Default behavior (bilinear + mipmaps, preserves backward compat)
- Best: Mitchell cubic resampler (highest quality)
- Fastest: Nearest neighbor (smallest file size, fastest processing)

MSBuild metadata name changed from FilterQuality to ResizeQuality.
All 25 new tests pass. 593 total passed (568 baseline + 25 new).
Same 20 pre-existing failures. Zero regressions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-single-project Splash Screen, Multi-Targeting, MauiFont, MauiImage, MauiAsset, Resizetizer community ✨ Community Contribution do-not-merge Don't merge this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8.0.92 make app size bigger

9 participants