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

Description Sam Weinig 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.
Comment 1 Sam Weinig 2020-12-25 13:24:25 PST Comment hidden (obsolete)
Comment 2 Sam Weinig 2020-12-25 13:26:26 PST Comment hidden (obsolete)
Comment 3 Sam Weinig 2020-12-25 15:15:19 PST Comment hidden (obsolete)
Comment 4 Sam Weinig 2020-12-25 16:22:44 PST Comment hidden (obsolete)
Comment 5 Sam Weinig 2020-12-25 17:00:01 PST Comment hidden (obsolete)
Comment 6 Sam Weinig 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.
Comment 7 Sam Weinig 2020-12-25 18:45:04 PST Comment hidden (obsolete)
Comment 8 Sam Weinig 2020-12-25 18:49:58 PST Comment hidden (obsolete)
Comment 9 Sam Weinig 2020-12-25 19:03:22 PST Comment hidden (obsolete)
Comment 10 Sam Weinig 2020-12-25 19:57:22 PST Comment hidden (obsolete)
Comment 11 Sam Weinig 2020-12-25 20:07:02 PST Comment hidden (obsolete)
Comment 12 Sam Weinig 2020-12-25 20:12:17 PST Comment hidden (obsolete)
Comment 13 Sam Weinig 2020-12-25 20:18:23 PST 
Created attachment 416771 [details]
Patch
Comment 14 Sam Weinig 2020-12-25 20:45:25 PST 
Created attachment 416772 [details]
Patch
Comment 15 Sam Weinig 2020-12-25 20:51:44 PST 
Created attachment 416773 [details]
Patch
Comment 16 Sam Weinig 2020-12-25 20:57:05 PST 
Created attachment 416774 [details]
Patch
Comment 17 Sam Weinig 2020-12-25 21:04:42 PST 
Created attachment 416775 [details]
Patch
Comment 18 Sam Weinig 2020-12-25 21:09:04 PST 
Created attachment 416776 [details]
Patch
Comment 19 Sam Weinig 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.
Comment 20 EWS 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].
Comment 21 Radar WebKit Bug Importer 2020-12-26 09:43:22 PST 
<rdar://problem/72682108>