Bug 214234

Experimenting with how to do this. My first attempt replaces the primitive type template argument to the color types (and the ColorComponents type) with a struct indicating type and clamped status. So you can have: SRGBA<UnclampedFloat> SRGBA<ClampedFloat> SRGBA<UnclampedByte> SRGBA<ClampedByte>

Created attachment 404097 [details] Experiment

Comment on attachment 404097 [details] Experiment Not fully fleshed out yet, but the basic idea is there. Conversion from SRGBA<UnclampedFloat> to SRGBA<ClampedFloat> (or the Byte variants) is done automatically here, with the idea being that users would essentially "tag" their input as either being "clamped" or "unclamped", and then functions would all actually only operate on the "clamped" types. So, if you tag your color as "unclamped" and then pass it to a function needing it "clamped", the clamping happens automatically. For example: void processColor(const SRGBA<ClampedFloat>&); auto color = SRGBA<UnclampedFloat> { untrustedRed, untrustedGreen, untrustedBlue, untrustedAlpha }; processColor(color); This works, and color is converted to SRGBA<ClampedFloat> implicitly. There are still a few rough edges with this automatic conversion (especially around Optionals), but I think we can probably make it work if its the direction we end up going.

Comment on attachment 404097 [details] Experiment View in context: https://bugs.webkit.org/attachment.cgi?id=404097&action=review I love the discipline of this; makes it hard to get things wrong. Not sure that the word ClampedFloat communicates the "between 0 and 1" property. ClampedByte, on the other hand, does seem unambiguous. Not sure there would be such a thing as UnclampedByte. Not loving the "normal language" feel of these type names. SRGBA<ClampedByte> is a far cry from Color. > Source/WebKit/UIProcess/API/wpe/WebKitColor.cpp:78 > + return WebCore::makeSimpleColor(WebCore::SRGBA<UnclampedFloat> { static_cast<float>(color->red), static_cast<float>(color->green), static_cast<float>(color->blue), static_cast<float>(color->alpha) }); EWS says this needs to be WebCore::UnclampedFloat. > Source/WebKit/UIProcess/API/wpe/WebKitColor.cpp:85 > + auto [r, g, b, a] = webCoreColor.toSRGBALossy<ClampedFloat>(); WebCore::ClampedFloat > Source/WebKit/UIProcess/gtk/ViewGestureControllerGtk.cpp:341 > + auto [red, green, blue, alpha] = color.toSRGBALossy<ClampedFloat>(); WebCore::ClampedFloat > Source/WebKit/WebProcess/WebPage/RemoteLayerTree/PlatformCAAnimationRemote.mm:696 > + auto [r, g, b, a] = keyframeValue.colorValue().toSRGBALossy<ClampedByte>(); WebCore::ClampedByte?

(In reply to Darin Adler from comment #4) > Comment on attachment 404097 [details] > Experiment > > View in context: > https://bugs.webkit.org/attachment.cgi?id=404097&action=review > > I love the discipline of this; makes it hard to get things wrong. Not sure > that the word ClampedFloat communicates the "between 0 and 1" property. Definitely struggling with this. We continue to need a word for this concept. > ClampedByte, on the other hand, does seem unambiguous. Not sure there would > be such a thing as UnclampedByte. Indeed. An UnclampedByte in this implementation is actually an int. So, if you got a bunch of untrusted int values, say from an API we don't necessarily know is going to return values in the 0-255 range like [NSBitmapImageRep getPixel:atX:y:], the idea would be you would something like: auto color = SRGBA<UnclampedByte> { untrustedRed, untrustedGreen, untrustedBlue, untrustedAlpha }; doSomthingWithClampedSRGBA(color); and get automatic clamping. To reiterate the goal (though not saying it a good idea or worthwhile, because I am not sure, or well implemented) with the design: Users *tag* values that come in. Functions *declare* what type of data they want to operate on. It could just be that we want only want the Clamped variants, and conversion from *untrusted* input to *clamped* input is explicit. auto color = rgbaFromUntrustedValues(untrustedRed, untrustedGreen, untrustedBlue, untrustedAlpha); doSomthingWithClampedSRGBA(color); > > Not loving the "normal language" feel of these type names. > SRGBA<ClampedByte> is a far cry from Color. Which aspect of it is "normal language" in your mind? I would agree that I don't love these names at all. Was hoping with iteration they would improve, but that has not panned out yet. Any ideas? using SimpleColor = SRGBA<ClampedByte>; is still on the table (though I find the word Simple a bit awkward here). Doing some bugzilla brainstorming (no bad ideas!). Maybe take some names from the GL world: - SRGBA<ClampedByte> -> ColorType<ColorSpace::SRGB, Format::RGBA8> / SRGBA<ClampedFloat> -> ColorType<ColorSpace::SRGB, Format::RGBAF32> or - SRGBA<ClampedByte> -> SRGBA8 / SRGBA<ClampedFloat> SRGBAF32 Those both seem worse. Will continue thinking.

Created attachment 404110 [details] Experiment

(In reply to Sam Weinig from comment #5) > An UnclampedByte in this implementation is actually an int. int32_t specifically? > It could just be that we want only want the Clamped variants, and conversion > from *untrusted* input to *clamped* input is explicit. I think that’s right. The only fly in the ointment is if you, the programmer, have an int, then convert to uint8_t before putting it into a ClampedByte, you have chopped it off without clamping and missed your chance to do the right thing. > > Not loving the "normal language" feel of these type names. > > SRGBA<ClampedByte> is a far cry from Color. > > Which aspect of it is "normal language" in your mind? I mean that "color" feels like "normal language" and "sRGBA made of clamped bytes" does not. > Any ideas? My favorite old technique was to talk to someone about the topic for a while and look for words they used when talking to another person and try to use those. > using SimpleColor = SRGBA<ClampedByte>; is still on the table (though I find > the word Simple a bit awkward here). As I mentioned before, I took the the name "simple color" from the HTML specification <https://html.spec.whatwg.org/multipage/common-microsyntaxes.html#simple-colour> but I added alpha so it’s not great. Sorry, no ideas to contribute right this second.

<rdar://problem/65852882>

Now that we have ExtendedSRGBA<> which is unclamped and SRGBA<> which is clamped (and variants of others as well, I think we are good here.