linebender / color

Color in Rust.
Apache License 2.0
39 stars 5 forks source link

Serialize color names by packing them into `DynamicColor` #39

Open tomcur opened 1 week ago

tomcur commented 1 week ago

To perform correct CSS Color 4 serialization of colors, some details of how a color was declared have to be preserved. Specifically, CSS Color Module Level 4 § 15.2 and the two sections on Lab and Oklab following it specify that

This implementation uses the padding currently available on DynamicColor to pack in the color name the DynamicColor was created from, or whether a named color space function was used; an implementation of the ideas in https://github.com/linebender/color/issues/26#issuecomment-2467997421.

DynamicColor now contains Flags, and the existing Missing color components are moved into Flags. In this implementation, only parse_color sets the color name or color space function name.

tomcur commented 1 week ago

What's needed to move this out of draft?

I think when https://github.com/linebender/color/pull/39#discussion_r1844964057 is resolved, we can move this out of draft.

raphlinus commented 14 hours ago

I'm having trouble making a go/no-go decision on this. I like the idea of better CSS compatibility, and the implementation seems fine (though I didn't dig deep), but I also like the idea of the code being minimal, and there are applications that will not need this functionality. I know we talked about this in office hours but don't feel consensus emerged.

Does anyone have a strong feeling either way? My gut feeling is that it's easier to add later if there's a strong need than remove, but if it is going in anyway, now is a good time to do it.