bubble-dev / _

🍱 metarepo of many packages and various monorepos
52 stars 6 forks source link

@fantasy-color has sRGB and RGB completely backwards #507

Open thomcc opened 3 years ago

thomcc commented 3 years ago

Hi, I'm writing a blog post on how a color math bug in a spec ended up spreading through the web development ecosystem in some pretty baffling ways, and so I've looked through a bunch of JS packages that do color stuff, and yours were one of them. (Note: You don't seem to have the bug I'm writing the blog post on, that's not why I'm filing this)

I did notice that you have a serious enough bug that I can't leave it be without saying something though. You have a major confusion in all of your @fantasy-color packages:

You seem to have sRGB completely backwards. In particular, what you call the sRGB color space is actually usually called linear RGB β€” often just RGB (the terminology here is actually pretty complex and vague, unfortunately β€” this is also the wrong name, but it's not important here), and what you call RGB is actually sRGB.


In general, unless you've gone out of your way to convert it, or happen to get very surprising data, the right assumption is that a random RGB input is in the sRGB color space. In fact, this is specified for the web β€” if you don't know anything else, you are supposed to assume sRGB: https://www.w3.org/Graphics/Color/sRGB.html. (And even when it's not sRGB, it's likely something that has an sRGB-like gamma curve for its transfer function β€”Β for example P3, Adobe RGB, ...).

A couple rules of thumb here are:


Anyway, that's to say, most of your functions, documentation, etc have this completely backwards.

This is easy to verify too. For example, the wikipedia article for sRGB includes the step where you go from linear RGB into sRGB here:

Screen Shot 2021-06-06 at 8 50 46 PM

If you compare it with https://github.com/bubble-dev/_/blob/dd61b2067da87dc67e42392876c333f2f753321e/packages/fantasy-color/rgb-to-srgb/src/index.ts#L8-L10, you'll see that these are inverse operations. Ditto with the other direction and such.

xaviervia commented 3 years ago

Hi @thomcc ! Sorry for the slow response, I am not actively monitoring this repo and the notifications get buried in the notifications tab.

To be honest I got a bit lost when trying to port the operations from the sources I found into these packages, as they tended to be contradictory. The original motivation was to reproduce the contrast ratio checker to get WCAG accessibility scores, and the formula in there mentioned that they were calculated in sRGB, which at the time (and to some extent still) I had no idea what it was, so I figured it makes sense to convert to it. What you are saying is consistent with everything I learned since then, so point taken.

I am not 100% sure on how to proceed on fixing it though, since I don't fully understand. If sRGB is really the color space that we usually refer to just RGB in the context of web (if they are one and the same), what is the RGB color space that I'm converting from?

Put another way, will simply inverting all the names (replace all srgb with rgb and viceversa) fix the issue?

Thanks a lot for bringing this up, I really appreciate getting the eye of someone who is more familiar with this issue.