Bug 212871 - Extended Color: Unify rounding / clamping conversions between 0-1 float components and 0-255 byte components
Summary: Extended Color: Unify rounding / clamping conversions between 0-1 float compo...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sam Weinig
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-06-06 11:24 PDT by Sam Weinig
Modified: 2020-06-08 13:12 PDT (History)
21 users (show)

See Also:

Attachments
WIP (20.63 KB, patch)
2020-06-06 11:28 PDT, Sam Weinig 		no flags Details | Formatted Diff | Diff
WIP (23.01 KB, patch)
2020-06-06 11:48 PDT, Sam Weinig 		no flags Details | Formatted Diff | Diff
Patch (23.54 KB, patch)
2020-06-06 11:55 PDT, Sam Weinig 		no flags Details | Formatted Diff | Diff
Patch (25.80 KB, patch)
2020-06-06 12:03 PDT, Sam Weinig 		no flags Details | Formatted Diff | Diff
Patch (28.81 KB, patch)
2020-06-06 13:20 PDT, Sam Weinig 		no flags Details | Formatted Diff | Diff
Patch (31.57 KB, patch)
2020-06-06 13:30 PDT, Sam Weinig 		no flags Details | Formatted Diff | Diff
Patch (31.58 KB, patch)
2020-06-06 13:40 PDT, Sam Weinig 		no flags Details | Formatted Diff | Diff
Patch (32.00 KB, patch)
2020-06-06 14:23 PDT, Sam Weinig 		no flags Details | Formatted Diff | Diff
Patch (37.86 KB, patch)
2020-06-06 14:51 PDT, Sam Weinig 		no flags Details | Formatted Diff | Diff
Patch (41.79 KB, patch)
2020-06-06 16:09 PDT, Sam Weinig 		no flags Details | Formatted Diff | Diff
Show Obsolete (9) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Weinig 2020-06-06 11:24:02 PDT 
Extended Color: Unify rounding / clamping conversions between 0-1 float components and 0-255 byte components
Comment 1 Sam Weinig 2020-06-06 11:28:28 PDT Comment hidden (obsolete)
Comment 2 Sam Weinig 2020-06-06 11:48:33 PDT Comment hidden (obsolete)
Comment 3 Sam Weinig 2020-06-06 11:55:56 PDT Comment hidden (obsolete)
Comment 4 Sam Weinig 2020-06-06 12:03:12 PDT Comment hidden (obsolete)
Comment 5 Sam Weinig 2020-06-06 13:20:16 PDT Comment hidden (obsolete)
Comment 6 Sam Weinig 2020-06-06 13:30:21 PDT Comment hidden (obsolete)
Comment 7 Sam Weinig 2020-06-06 13:40:25 PDT Comment hidden (obsolete)
Comment 8 Sam Weinig 2020-06-06 14:23:10 PDT Comment hidden (obsolete)
Comment 9 Sam Weinig 2020-06-06 14:51:22 PDT Comment hidden (obsolete)
Comment 10 Sam Weinig 2020-06-06 16:09:18 PDT 
Created attachment 401281 [details]
Patch
Comment 11 Simon Fraser (smfr) 2020-06-08 11:44:59 PDT 
Comment on attachment 401281 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=401281&action=review

> Source/WebCore/platform/graphics/ColorUtilities.h:54
> +inline uint8_t convertPrescaledToComponentByte(float f)

Not really a fan of "prescaled". Does it mean "in the range 0-1"? Can't think of a better word though.
Comment 12 Darin Adler 2020-06-08 12:25:33 PDT 
Probably obvious to both of you but: We do need terminology for 0-1 and 0-255 for floating point components. Sadly it seems we need code that deals with both. And when overloading a function so it can take either integers or floating point, it seems really bad to have a floating point overload that takes 0-1 and an integer ones that takes 0-255, so we’ll want to avoid that.
Comment 13 EWS 2020-06-08 12:37:25 PDT 
Committed r262733: <https://trac.webkit.org/changeset/262733>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 401281 [details].
Comment 14 Radar WebKit Bug Importer 2020-06-08 12:38:35 PDT 
<rdar://problem/64130839>
Comment 15 Simon Fraser (smfr) 2020-06-08 13:00:45 PDT 
(In reply to Darin Adler from comment #12)
> Probably obvious to both of you but: We do need terminology for 0-1 and
> 0-255 for floating point components. Sadly it seems we need code that deals
> with both. And when overloading a function so it can take either integers or
> floating point, it seems really bad to have a floating point overload that
> takes 0-1 and an integer ones that takes 0-255, so we’ll want to avoid that.

Yes!
Comment 16 Sam Weinig 2020-06-08 13:08:20 PDT 
We need the one taking 0-255 float values in exactly one place right now, in the CSS parser. Everywhere else is 0-1 float or 0-255 int.
Comment 17 Sam Weinig 2020-06-08 13:12:08 PDT 
(In reply to Simon Fraser (smfr) from comment #15)
> (In reply to Darin Adler from comment #12)
> > Probably obvious to both of you but: We do need terminology for 0-1 and
> > 0-255 for floating point components. Sadly it seems we need code that deals
> > with both. And when overloading a function so it can take either integers or
> > floating point, it seems really bad to have a floating point overload that
> > takes 0-1 and an integer ones that takes 0-255, so we’ll want to avoid that.
> 
> Yes!

The naming I was thinking about was:

makeSimpleColorFromSRGBABytes
makeSimpleColorFromSRGBAFloats

(in the patch for https://bugs.webkit.org/show_bug.cgi?id=180620, Darin used makeSimpleColorFromRGBABytes for 0-255 ints and makeSimpleColorFromSRGBA for 0-1 floats).

Not wed to this.