Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

made FilterQuality configurable for Resizetizer #25686

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

thisisthekap
Copy link
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
@@ -124,6 +128,9 @@ public static ResizeImageInfo Parse(ITaskItem image)
info.ForegroundFilename = fgFileInfo.FullName;
}

if (Enum.TryParse<SKFilterQuality>(image.GetMetadata("FilterQuality"), out var filterQuality))
Copy link

Choose a reason for hiding this comment

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

we might have to add the docs here how to implement in the MauiImage and also available values for the FilterQuality

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@thisisthekap
Copy link
Contributor Author

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

@rmarinho
Copy link
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
Member

rmarinho commented Nov 8, 2024

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@mattleibow
Copy link
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
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
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
Member

/azp run maui-public maui-uitests-public

Copy link

No pipelines are associated with this pull request.

@rmarinho
Copy link
Member

/azp run maui-public

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
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
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
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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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
6 participants