-
Notifications
You must be signed in to change notification settings - Fork 4
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
Feature/improve image generation from color #42
Feature/improve image generation from color #42
Conversation
We should wait for #41 to be merged before merging this evolution |
return image | ||
size: CGSize, | ||
scale: CGFloat) -> UIImage? { | ||
let format = UIGraphicsImageRendererFormat() |
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.
UIGraphicsImageRendererFormat 😄
e6c7f9b
to
1bd857d
Compare
@@ -16,16 +16,19 @@ public extension UIImage { | |||
- parameter size: The size of the final image | |||
*/ | |||
static func ad_filled(with color: UIColor, | |||
size: CGSize = CGSize(width: 1, height: 1)) -> UIImage? { | |||
size: CGSize = CGSize(width: 1, height: 1), | |||
scale: CGFloat = 1.0) -> UIImage? { |
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.
par défaut, ça vaut pas le scale du device ?
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.
J'ai préféré laisser 1.0 par défaut pour ne pas changer le comportement de base de la méthode. Mais a l'utilisation oui on voudra utiliser le scale du device si on a besoin d'autre chose qu'un rectangle de couleur
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.
Justement, vu qu'on précise dorénavant un scale, je me demande si on n'a pas changé le comportement actuel
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.
Avant ce changement l'image était générée avec un scale de 1, et tous les anciens tests passent encore, donc je pense que l'ancien comportement a bien été conservé
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.
on peut faut juste prévenir les gens
du coup sans majeure, soit mineure
et dans tous les cas mettre une gros attention
meme si en théorie la scale en plus devrait juste améliorer l'existant partout
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.
Avant ce changement l'image était générée avec un scale de 1, et tous les anciens tests passent encore, donc je pense que l'ancien comportement a bien été conservé
y'a moyen que dans les tests y'avait pas la scale non?
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.
Avant ce changement l'image était générée avec un scale de 1, et tous les anciens tests passent encore, donc je pense que l'ancien comportement a bien été conservé
y'a moyen que dans les tests y'avait pas la scale non?
Le scale n'est pas fournis pour les anciens tests et pourtant ils sont sur un simu qui a plus de x1.
Le problème, c'est que UIScreen.main
est déprécié, et donc je ne suis pas trop fan de le mettre en valeur par défaut de la méthode (A moins qu'il y ait un autre moyen d'avoir l'info ?). Après je peux déprécier l'appel à ad_filled sans le scale, mais je pense pas qu'on ait besoin en général du scale sur des images de couleur
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.
si j'en crois la doc, on est bon, c'était un scale de 1.0 😌
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.
je parlais de vérification sur le scale pour les tests ;)
yes ils était justement tous à un par défaut
J’arrive après la bataille, mais le passage de UI…Context à UIGraphicsImageRenderer est important. You can use image renderers to accomplish drawing tasks, without having to handle configuration such as color depth and image scale, or manage Core Graphics contexts. You initialize an image renderer with parameters such as image output dimensions and format. You then use one or more of the drawing functions to render images that share these properties. Ça m’orienterait plutôt vers une majeure en laissant le param scale optional vu qu’il l’a déjà si on l’écrase pas. My 2 cents. |
J'ai justement fait en sorte qu'on laisse le scale a 1 par défaut, donc il ne doit pas y avoir de changement (d'où la branche de version mineure). Après on peux faire un bump majeur, mais au vu des tests qui sont resté identiques je ne suis pas sur que ca soit nécéssaire |
This PR add a scale parameter for ad_filled method on UIColor. With that we can use and modify the generated image ant it will be properly displayed at correct scale.
A situation where this is needed is when we generate an image from a color, then make corners rounded and then displaying it on screen as navigation bar background. Without the scale the image is not sharp at all.