Bug 220146

Summary: Simplify adding new color spaces to WebCore
Product: WebKit Reporter: Sam Weinig <sam>
Component: PlatformAssignee: Sam Weinig <sam>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, darin, dino, ews-watchlist, gyuyoung.kim, ryuan.choi, sergio, 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
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
none
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
none
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch none

Sam Weinig
Reported 2020-12-25 12:48:25 PST
The spring refactoring of color made it quite a bit easier to add a new color space to WebCore without touching too many places, but it is still a bit more than necessary. Currently you need to: - Add new value to ColorSpace enum in ColorSpace.h - Add a new type to ColorTypes.h - Add conversions to ColorConversion.h/cpp (it's quite unclear which ones you would need to add) - Add serialization to ColorSerialization.h/cpp. - Update switch statement in ColorSpace TextStream overload in Color.cpp - Add override of Color constructor in Color.h - Add override of Color::setColor in Color.h - Update switch statement in ExtendedColor::callOnUnderlyingType in ExtendedColor.h - Add new color space accessor to GraphicsContextCG.h/cpp. - Update switch statement in cachedCGColorSpace in GraphicsContextCG.h - Update switch statement in ColorCG.cpp - Add css parsing support. Many of these can be simplified, clarified or removed. I think we can improve things such that the steps get reduced to: - Add new value to ColorSpace enum in ColorSpace.h - Update switch statement in ColorSpace TextStream overload (but lets move it to ColorSpace.cpp so it's clear you need to do this when adding the enum value). - Add a new type to ColorTypes.h - Add serialization to ColorSerialization.h/cpp. - Add conversions to ColorConversion.h/cpp (but make it really clear which conversions are needed). - Add new color space accessor to GraphicsContextCG.h/cpp. - Update switch statement in cachedCGColorSpace in GraphicsContextCG.h - Add css parsing support. Not amazing, but will be an improvement.
Attachments
Patch (42.02 KB, patch)
2020-12-25 13:24 PST, Sam Weinig 		no flags
DetailsFormatted DiffDiff
Patch (42.02 KB, patch)
2020-12-25 13:26 PST, Sam Weinig 		no flags
DetailsFormatted DiffDiff
Patch (42.13 KB, patch)
2020-12-25 15:15 PST, Sam Weinig 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (42.14 KB, patch)
2020-12-25 16:22 PST, Sam Weinig 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (42.65 KB, patch)
2020-12-25 17:00 PST, Sam Weinig 		no flags
DetailsFormatted DiffDiff
Patch (38.34 KB, patch)
2020-12-25 18:45 PST, Sam Weinig 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (38.02 KB, patch)
2020-12-25 18:49 PST, Sam Weinig 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (34.40 KB, patch)
2020-12-25 19:03 PST, Sam Weinig 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (42.66 KB, patch)
2020-12-25 19:57 PST, Sam Weinig 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (43.33 KB, patch)
2020-12-25 20:07 PST, Sam Weinig 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (42.67 KB, patch)
2020-12-25 20:12 PST, Sam Weinig 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (39.58 KB, patch)
2020-12-25 20:18 PST, Sam Weinig 		no flags
DetailsFormatted DiffDiff
Patch (41.85 KB, patch)
2020-12-25 20:45 PST, Sam Weinig 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (42.08 KB, patch)
2020-12-25 20:51 PST, Sam Weinig 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (42.46 KB, patch)
2020-12-25 20:57 PST, Sam Weinig 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (42.73 KB, patch)
2020-12-25 21:04 PST, Sam Weinig 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (42.27 KB, patch)
2020-12-25 21:09 PST, Sam Weinig 		no flags
DetailsFormatted DiffDiff
Show Obsolete (12) View All Add attachment proposed patch, testcase, etc.
Sam Weinig
Comment 1 2020-12-25 13:24:25 PST Comment hidden (obsolete)
Sam Weinig
Comment 2 2020-12-25 13:26:26 PST Comment hidden (obsolete)
Sam Weinig
Comment 3 2020-12-25 15:15:19 PST Comment hidden (obsolete)
Sam Weinig
Comment 4 2020-12-25 16:22:44 PST Comment hidden (obsolete)
Sam Weinig
Comment 5 2020-12-25 17:00:01 PST Comment hidden (obsolete)
Sam Weinig
Comment 6 2020-12-25 18:32:36 PST
Anyone have any idea what these windows failures are about? I can't make heads or tails from the MSVC error.
Sam Weinig
Comment 7 2020-12-25 18:45:04 PST Comment hidden (obsolete)
Sam Weinig
Comment 8 2020-12-25 18:49:58 PST Comment hidden (obsolete)
Sam Weinig
Comment 9 2020-12-25 19:03:22 PST Comment hidden (obsolete)
Sam Weinig
Comment 10 2020-12-25 19:57:22 PST Comment hidden (obsolete)
Sam Weinig
Comment 11 2020-12-25 20:07:02 PST Comment hidden (obsolete)
Sam Weinig
Comment 12 2020-12-25 20:12:17 PST Comment hidden (obsolete)
Sam Weinig
Comment 13 2020-12-25 20:18:23 PST
Created attachment 416771 [details] Patch
Sam Weinig
Comment 14 2020-12-25 20:45:25 PST
Created attachment 416772 [details] Patch
Sam Weinig
Comment 15 2020-12-25 20:51:44 PST
Created attachment 416773 [details] Patch
Sam Weinig
Comment 16 2020-12-25 20:57:05 PST
Created attachment 416774 [details] Patch
Sam Weinig
Comment 17 2020-12-25 21:04:42 PST
Created attachment 416775 [details] Patch
Sam Weinig
Comment 18 2020-12-25 21:09:04 PST
Created attachment 416776 [details] Patch
Sam Weinig
Comment 19 2020-12-26 08:09:45 PST
Seems that by making callWithColorType only templatized on the functor, and not the component type makes the issue go away. Still not clear what is causing it, but will try to work that out in another bug.
EWS
Comment 20 2020-12-26 09:42:59 PST
Committed r271089: <https://trac.webkit.org/changeset/271089> All reviewed patches have been landed. Closing bug and clearing flags on attachment 416776 [details].
Radar WebKit Bug Importer
Comment 21 2020-12-26 09:43:22 PST
<rdar://problem/72682108>
Note You need to log in before you can comment on or make changes to this bug.