Color conversions: Don't optimize Oklab to XYZD65 conversions [chromium/src : main]
0 views
Skip to first unread message
ccameron chromium (Gerrit)
Oct 11, 2025, 12:18:03 PM (4 days ago) Oct 11
to ccameron chromium, Philip Rogers, Kaylee Lubick, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, blink-reviews-p...@chromium.org, blink-...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, kinuko...@chromium.org
Attention needed from Kaylee Lubick and Philip Rogers
ccameron chromium added 2 comments![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 3 (Latest):
ccameron chromium . resolved
+pdr for blink OWNERship, +kjlubick for the giant stack of patches...
File ui/gfx/color_conversions_unittest.cc
Line 151, Patchset 2:
for (auto& color_pair : colors_tests) {ccameron chromium . unresolved
All of these variables were mis-labeled (input was xyz, not Lab).
Related details
Attention is currently required from:
- Kaylee Lubick
- Philip Rogers
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kaylee Lubick (Gerrit)
Oct 14, 2025, 4:22:17 PM (yesterday) Oct 14
to ccameron chromium, Philip Rogers, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, blink-reviews-p...@chromium.org, blink-...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, kinuko...@chromium.org
Attention needed from Philip Rogers and ccameron chromium
Kaylee Lubick voted and added 2 comments![]( "Open in Gerrit")
Votes added by Kaylee Lubick
| Code-Review | +1 |
2 comments
File ui/gfx/color_conversions.cc
Line 238, Patchset 3 (Latest):
const skcms::Vector3 lms_cubed{{Kaylee Lubick . unresolved
I had to go reference https://bottosson.github.io/posts/oklab/ to figure out why there's a cube/cuberoot here. Do you mind adding a reminder comment that it's just part of the XYZ->Oklab conversion?
Line 245, Patchset 3 (Latest):
const auto xyz = skcms::Matrix3x3_apply(getXYDZ65toXYZD50matrix(), xyz_d65);Kaylee Lubick . unresolved
Can you add a reminder here that OkLab is defined w/r to the D65 white point, thus the "stop" in that conversion before going onto D50 (and I'm guessing D50 is more commonly useful which is why a lot of D65 stuff is being deleted in the following CL)?
Related details
Attention is currently required from:
- Philip Rogers
- ccameron chromium
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
ccameron chromium (Gerrit)
10:46 AM (10 hours ago) 10:46 AM
to ccameron chromium, Kaylee Lubick, Philip Rogers, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, blink-reviews-p...@chromium.org, blink-...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, kinuko...@chromium.org
Attention needed from Philip Rogers
ccameron chromium added 3 comments![]( "Open in Gerrit")
Patchset-level comments
File ui/gfx/color_conversions.cc
I had to go reference https://bottosson.github.io/posts/oklab/ to figure out why there's a cube/cuberoot here. Do you mind adding a reminder comment that it's just part of the XYZ->Oklab conversion?
ccameron chromium
Done
Line 245, Patchset 3:
const auto xyz = skcms::Matrix3x3_apply(getXYDZ65toXYZD50matrix(), xyz_d65);Kaylee Lubick . resolved
Can you add a reminder here that OkLab is defined w/r to the D65 white point, thus the "stop" in that conversion before going onto D50 (and I'm guessing D50 is more commonly useful which is why a lot of D65 stuff is being deleted in the following CL)?
ccameron chromium
Done
Related details
Attention is currently required from:
- Philip Rogers
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kaylee Lubick (Gerrit)
11:02 AM (10 hours ago) 11:02 AM
to ccameron chromium, Philip Rogers, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, blink-reviews-p...@chromium.org, blink-...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, kinuko...@chromium.org
Attention needed from Philip Rogers and ccameron chromium
Kaylee Lubick voted and added 1 comment![]( "Open in Gerrit")
Votes added by Kaylee Lubick
| Code-Review | +1 |
1 comment
File ui/gfx/color_conversions_unittest.cc
Line 151, Patchset 2:
for (auto& color_pair : colors_tests) {ccameron chromium . resolved
All of these variables were mis-labeled (input was xyz, not Lab).
Attention is currently required from:
- Philip Rogers
- ccameron chromium
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Philip Rogers (Gerrit)
4:33 PM (5 hours ago) 4:33 PM
to ccameron chromium, Kaylee Lubick, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, blink-reviews-p...@chromium.org, blink-...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, kinuko...@chromium.org
Attention needed from ccameron chromium
Philip Rogers voted and added 1 comment![]( "Open in Gerrit")
Votes added by Philip Rogers
| Code-Review | +1 |
1 comment
Patchset-level comments
Attention is currently required from:
- ccameron chromium
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages