-
Notifications
You must be signed in to change notification settings - Fork 722
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
Colors: support P3 display #413
base: stable
Are you sure you want to change the base?
Conversation
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.
Thanks a lot for the PR!
I have to admit I'm not sure about forcing P3 colorspace on all contexts (well, as soon as it's available so in 10+).
That mean that now, every time I use a colors.txt
with #336699
I'll get a color with those components in P3 colorspace, instead of the standard colorspace. Which means this will now be a different color than before!
I see multiple solutions here:
- Allow users to opt-in to P3 colorspace with a template parameter. If people don't mention any extra param, use
init(red:green:blue:alpha:)
, but if they use--param P3
then use your new code - Or separate the template in two, one using P3 and one not. But In that specific case I prefer a template param rather than two separate templates, way easier to maintain in the long run
- Or come up with a new syntax to specify the colorspace on each input format we support, for example
#rrggbbaa@P3
incolors.txt
or similar. Which would have the advantage to allow us to specify the colorspace on each color's basis, but won't be scalable to all formats (*.clr
for example)
Thoughs on all this are welcome, I'd really love to have P3 support but we also need to be careful not to generate different colors than previously generated by SwiftGen previous templates, otherwise people will see their whole color palette change in their app which wouldn't be good.
|
||
* Add support for P3 display colorspace. | ||
[noppefoxwolf](https://github.com/noppefoxwolf) | ||
[#413](https://github.com/SwiftGen/SwiftGen/pull/413) | ||
### Internal Changes |
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.
Be sure to keep an empty newline before titles for correct markdown formatting on all readers 😉
Also note that according to the documentation:
Which confirms that changing the init will change the colorspace and thus the colors compared to what we previously generated and what users expected (P3 is ~25% larger than sRGB, so changing the colorspace/coordinate system scale will indeed return a different color for the same coordinates/values), but also tells us that we could still use the |
I think the floats go from somewhere below 0.0, to somewhere above 1.0. Don't |
Yeah, I'd have to double-check but I'm pretty sure |
Right, so I think that for the other formats (txt, json, xml) we need to define a way for the parser to support colorspaces and/or P3, convert these to sRGB if needed, and then simply pass these along to the templates. |
From a quick check of the Android docs, they don't seem to support wide colours in the |
Thank you there ❤️ In css
TODO:
|
This added support for p3 display colorspace.
UIColor instance created by
UIColor.init(displayP3Red:green:blue:alpha)
when iOS10+.TODO: