Bug 220146 - Simplify adding new color spaces to WebCore
Summary: Simplify adding new color spaces to WebCore
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sam Weinig
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-12-25 12:48 PST by Sam Weinig
Modified: 2020-12-26 09:43 PST (History)
9 users (show)

See Also:


Attachments
Patch (42.02 KB, patch)
2020-12-25 13:24 PST, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (42.02 KB, patch)
2020-12-25 13:26 PST, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (42.13 KB, patch)
2020-12-25 15:15 PST, Sam Weinig
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (42.14 KB, patch)
2020-12-25 16:22 PST, Sam Weinig
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (42.65 KB, patch)
2020-12-25 17:00 PST, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (38.34 KB, patch)
2020-12-25 18:45 PST, Sam Weinig
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (38.02 KB, patch)
2020-12-25 18:49 PST, Sam Weinig
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (34.40 KB, patch)
2020-12-25 19:03 PST, Sam Weinig
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (42.66 KB, patch)
2020-12-25 19:57 PST, Sam Weinig
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (43.33 KB, patch)
2020-12-25 20:07 PST, Sam Weinig
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (42.67 KB, patch)
2020-12-25 20:12 PST, Sam Weinig
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (39.58 KB, patch)
2020-12-25 20:18 PST, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (41.85 KB, patch)
2020-12-25 20:45 PST, Sam Weinig
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (42.08 KB, patch)
2020-12-25 20:51 PST, Sam Weinig
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (42.46 KB, patch)
2020-12-25 20:57 PST, Sam Weinig
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (42.73 KB, patch)
2020-12-25 21:04 PST, Sam Weinig
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (42.27 KB, patch)
2020-12-25 21:09 PST, Sam Weinig
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>