-
Notifications
You must be signed in to change notification settings - Fork 406
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
Adaptive Icon Scaling #181
base: master
Are you sure you want to change the base?
Conversation
remove unassigned icons children
Just tested it and seems to be working well for my use case too. @MarkOSullivan94 it'd be great if this could get merged, many people in the issue #96 would appreciate it :) Or if you could let us know what's stopping it from getting merged? |
Any updates on this? @MarkOSullivan94 |
few open issues mentioning this PR/adaptive icon bug (#96, #226), anyone able to take a look? @slightfoot |
@Aulig I do believe all icons (including adaptive) expect square input files. I would put your image on a square canvas before using it to generate the adaptive icon. |
Yea, that's what I'm doing now :) Thanks for the info - in that case it makes sense that this PR doesn't handle it separately. |
@Aulig Now, to be fair, there was a bug in my code for the scaling that caused non-square photos to be scaled incorrectly 😛 I am now enforcing a square canvas just to make everyone's life easier 😃 |
It's May 2021 and it's still not merged. :D |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works like a charm.
I merged master into this branch in my own fork if someone want's to use an up-to-date version or @MarkOSullivan94 want's to merge it without the conflicts
@knyghtryda 🎉 🎉 |
@knyghtryda could you please provide an image which has issues with the scaling? 🙂 It'll make it easy for me to test this and see the before vs after. Definitely should provide some kind of info on the README so if people are running into similar issues they can make use of the changes you have implemented here 🥳 |
@MarkOSullivan94 I'm getting the same result after adding the scale factor with the following image not scaling for me: Current configuration:
Result: |
Hey @MarkOSullivan94, I hope you are doing well. Is there a chance this PR gets merged in the near future? :) There are many open Issues which are related to this PR. |
I'll start taking a look at this. |
41b290f
to
b8ad042
Compare
@MarkOSullivan94 we may need to add this to our roadmap workaround #181 (comment) |
5c7a41a
to
1749b94
Compare
Since the adaptive icons seems to have extra padding when compared to their iOS counter parts I added a new scaling factor parameter so that the image for the adaptive icon can be scaled before finally scaling down to icon size. This scaling is done by enlarging the canvas, not scaling down the original image, so image quality should in theory be better. The two new yaml parameters are:
Adding background image scaling will be next.