search

microsoft / vscode-css-languageservice

CSS, LESS & SCSS language service extracted from VSCode to be reused, e.g in the Monaco editor.
MIT License
317 stars 176 forks source link

lab() and lch() color previews added #306

Open GauravB159 opened 1 year ago

GauravB159 commented 1 year ago

Issue #305 Main Repo Issue microsoft/vscode#165207

aeschli commented 1 year ago

Thanks for the PR, it looks promising. I added comments.

GauravB159 commented 1 year ago

Done! Didn't know about the formatter, I'll keep that in mind going forward. And I've replaced const when possible. Is the functionality required all covered by this? I'll add more test cases and the other two functions if so. Thank you.

romainmenke commented 1 year ago

Thank you for this @GauravB159!

I've gone over these changes and the conversions don't seem to be accurate. Can you share the source of the transforms?

Good reference code can be found here : https://github.com/w3c/csswg-drafts/tree/main/css-color-4 WPT has a bunch of tests : https://wpt.fyi/results/css/css-color?label=master&label=experimental&aligned&view=subtest

We also have a bunch of tests : https://github.com/csstools/postcss-plugins/blob/main/plugins/postcss-lab-function/test/basic.display-p3-false.preserve-true.expect.css And also code that you can take : https://github.com/csstools/postcss-plugins/tree/main/plugins/postcss-lab-function/src/css-color-4

But best of all would be to use https://github.com/LeaVerou/color.js, if @aeschli is open to adding this as a dependency? The authors of this package are the specification editors for most of the things related to color in CSS.

One important aspect that doesn't seem to be covered here is out of gamut color handling. It is not possible to round trip lab -> rgb -> lab for all colors without changing the channel values. This might be confusing for users if they are not aware of this and just switch to a different color function out of habit.

GauravB159 commented 1 year ago

Hi @romainmenke

I used this webpage for a reference for the formulae. Is there somewhere I could see the correct values?

I was using this online converter to verify my values. It does give values in decimals, but rgb requires whole numbers I guess, so I have rounded the values at the end. Maybe that is causing an issue?

lab -> rgb -> lab is probably inaccurate because of the rounding up right? Is there a workaround to that that I could implement?

Thanks for your help! I'll check out the code you linked, maybe there's some issue with the formula implementation.

aeschli commented 1 year ago

Thanks for the great info @romainmenke ! No added dependencies please. It would mean quite some administrative overhead as I have to get legal approvals. Also for maintenance it's easier if we own all the code.

romainmenke commented 1 year ago

lab -> rgb -> lab is probably inaccurate because of the rounding up right? Is there a workaround to that that I could implement?

Rounding errors might also cause this, but those would be very small deviations. lab, lch,... however can all express a lot more colors than srgb (even some colors that do not exist). So a valid value in lab might not have a corresponding value in srgb.

I think there are three options :

  1. allow channel values in srgb to go under 0 or above 255. (this options is lossless)
  2. clamp channel values in srgb to 0 - 255. (lossy and gives poor results)
  3. apply a gamut mapping so that colors from lab that are out of gamut for srgb are visually similar. (lossy and gives ok results)

The "right" option depends on the function of this feature in VSCode :

No added dependencies please. It would mean quite some administrative overhead as I have to get legal approvals. Also for maintenance it's easier if we own all the code.

Got it! Yes avoiding all that is better.

The code linked from the CSSTools repo is a port to TypeScript from the reference implementation that exists in the csswg-drafts repo.

It has the same W3C license because the only thing I did was add typings.

GauravB159 commented 1 year ago

@romainmenke Yeah, I understand what you're saying. Now that you mention it, I remember there were negative and valuss greater than 255 after converting to sRGB. I clamped those values to 0-255 which might be causing the loss.

@aeschli Which option do you recommend we go with? We could store the negative values and clamp them just while displaying the color possibly. Although this might still cause issues in a lab->rgb->lab conversion through the color picker since it will pick up the clamped string from the new rule and use that, and not the stored negative values.