Bug 235004 – Add helpers to access CoreGraphics color spaces more easily in generic contexts

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Sam Weinig]

Add helpers to access CoreGraphics color spaces more easily in generic contexts

[Sam Weinig]

Created attachment 448674 [details]
Patch

[Sam Weinig]

Created attachment 448694 [details]
Patch

[Darin Adler]

Comment on attachment 448694 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=448694&action=review

> Source/WebCore/platform/graphics/cg/ColorSpaceCG.h:141
> +    template<ColorSpace C> static auto test(int) -> TrueIfDefined<decltype(CGColorSpaceMapping<C>::colorSpace())>;

Consider "space" or "colorSpace" instead of C? Here and just below.

> Source/WebCore/platform/graphics/cg/ColorSpaceCG.h:142
> +    template<ColorSpace C> static auto test(long) -> std::false_type;

Omit "C" here.

> Source/WebCore/platform/graphics/cg/ColorSpaceCG.h:145
> +template<ColorSpace C> inline constexpr bool HasCGColorSpaceMapping = decltype(HasCGColorSpaceMappingHelper::test<C>(0))();

This looks more complicated than what ColorTypes.h uses for HasDescriptorMember, etc. And HasBeginFunctionMember in Hasher.h is what I came up with when I needed to solve a similar problem. I am not sure if the problems are close enough to the same, but I am surprised we need more code here.

HasModernDecoder in ArgumentCoder.h is more complex, closer to what we have here.

Would like it if the idioms were similar for cases like these, and I don’t understand what leads to this more complex thing.

[Sam Weinig]

(In reply to Darin Adler from comment #3)
> Comment on attachment 448694 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=448694&action=review
> 
> > Source/WebCore/platform/graphics/cg/ColorSpaceCG.h:141
> > +    template<ColorSpace C> static auto test(int) -> TrueIfDefined<decltype(CGColorSpaceMapping<C>::colorSpace())>;
> 
> Consider "space" or "colorSpace" instead of C? Here and just below.
> 
> > Source/WebCore/platform/graphics/cg/ColorSpaceCG.h:142
> > +    template<ColorSpace C> static auto test(long) -> std::false_type;
> 
> Omit "C" here.
> 
> > Source/WebCore/platform/graphics/cg/ColorSpaceCG.h:145
> > +template<ColorSpace C> inline constexpr bool HasCGColorSpaceMapping = decltype(HasCGColorSpaceMappingHelper::test<C>(0))();
> 
> This looks more complicated than what ColorTypes.h uses for
> HasDescriptorMember, etc. And HasBeginFunctionMember in Hasher.h is what I
> came up with when I needed to solve a similar problem. I am not sure if the
> problems are close enough to the same, but I am surprised we need more code
> here.
> 
> HasModernDecoder in ArgumentCoder.h is more complex, closer to what we have
> here.
> 
> Would like it if the idioms were similar for cases like these, and I don’t
> understand what leads to this more complex thing.

Dang, I was trying to remember where I (or someone else) had done this before and was coming up blank. These options are much more clear. Thanks.

[Sam Weinig]

Created attachment 448706 [details]
Patch

[Sam Weinig]

> I don’t understand what leads to this more complex thing.

That magical intangible technical constraint of my feeble memory :).

[Sam Weinig]

Created attachment 448707 [details]
Patch

[EWS]

Committed r287821 (245873@main): <https://commits.webkit.org/245873@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 448707 [details].

[Radar WebKit Bug Importer]

<rdar://problem/87312687>

[EWS]

Patch 448706 does not build