Bug 213707

Summary: Simplify Color's interface by removing isDark()
Product: WebKit Reporter: Sam Weinig <sam>
Component: New BugsAssignee: Sam Weinig <sam>
Status: RESOLVED FIXED    
Severity: Normal CC: changseok, darin, esprehn+autocc, ews-watchlist, glenn, kondapallykalyan, mifenton, pdr, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Sam Weinig
Reported 2020-06-28 18:13:48 PDT
Simplify Color's interface by removing isDark() and lightness()
Attachments
Patch (14.89 KB, patch)
2020-06-28 18:21 PDT, Sam Weinig 		no flags
DetailsFormatted DiffDiff
Patch (4.47 KB, patch)
2020-06-29 15:14 PDT, Sam Weinig 		no flags
DetailsFormatted DiffDiff
Patch (4.46 KB, patch)
2020-06-29 15:17 PDT, Sam Weinig 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Sam Weinig
Comment 1 2020-06-28 18:21:16 PDT
Created attachment 403022 [details] Patch
Sam Weinig
Comment 2 2020-06-28 20:14:46 PDT
Not a huge surprise that paste-dark-mode-color-filtered.html is failing, as the heuristic is changing. Need to see if the new results are acceptable.
Darin Adler
Comment 3 2020-06-29 09:57:36 PDT
Comment on attachment 403022 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=403022&action=review > Source/WebCore/ChangeLog:8 > + - Move Color::isDark to RenderThemeIOS.mm, it's one client and rename it it's -> its > Source/WebCore/rendering/RenderThemeIOS.mm:1055 > +static bool useConvexGradient(const Color& backgroundColor) Typically we try not to name functions that compute values with verbs. So we would name it shouldUseConvexGradient. > Source/WebKit/ChangeLog:10 > + - Remove WebCore::lightness() and Color::lightness(), and replace with > + Color::luminance(), which is almost the same, but likely to be more > + in line with perceptual lightness people had in mind. Maybe do the behavior change, the switch to luminance, first in a patch that doesn’t also refactor or remove.
Sam Weinig
Comment 4 2020-06-29 15:14:35 PDT
Created attachment 403120 [details] Patch
Sam Weinig
Comment 5 2020-06-29 15:17:45 PDT
Created attachment 403121 [details] Patch
Sam Weinig
Comment 6 2020-06-29 15:18:57 PDT
Just going to do the isDark change in this one. Leave the lightness() for separate change.
EWS
Comment 7 2020-06-29 18:32:06 PDT
Committed r263713: <https://trac.webkit.org/changeset/263713> All reviewed patches have been landed. Closing bug and clearing flags on attachment 403121 [details].
Radar WebKit Bug Importer
Comment 8 2020-06-29 18:33:14 PDT
<rdar://problem/64917999>
Note You need to log in before you can comment on or make changes to this bug.