Bug 155994 – Add color-gamut media query support

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

[Dean Jackson]

Add color-gamut media query support

[Dean Jackson]

<rdar://problem/23282326>

[Dean Jackson]

Created attachment 275147 [details]
Patch

[WebKit Commit Bot]

Attachment 275147 [details] did not pass style-queue:


ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:253:  color_gamutMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 1 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Darin Adler]

Comment on attachment 275147 [details]
Patch

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

They breaks GTK, EFL, and Windows builds because screenSupportsExtendedColor is declared but not defined for all those platforms. Please fix that before landing!

> Source/WebCore/css/MediaQueryEvaluator.cpp:258
> +    const CSSValueID id = downcast<CSSPrimitiveValue>(*value).getValueID();

The const here seems gratuitous. Most local variables could be marked const.

> Source/WebCore/css/MediaQueryEvaluator.cpp:265
> +    // FIXME: At some point we should start detecting displays that
> +    // support more colors. For the moment we'll just assume an "extended
> +    // color" display is at least as good as P3.

I think this FIXME could be clearer. I think the real issue is that we will always return false for rec2020 and it might be better to be more specific about this. Maybe the code should be written like this:

    switch (downcast<CSSPrimitiveValue>(*value).getValueID()) {
    case CSSValueSrgb:
        return true;
    case CSSValueP3:
        return screenSupportsExtendedColor();
    case CSSValueRec2020:
        return false;
    }

    ASSERT_NOT_REACHED();
    return true;

I think the behavior would be clearer then; the FIXME could go in the CSSValueRec2020 case.

> Source/WebCore/css/MediaQueryEvaluator.cpp:267
> +    return (id == CSSValueP3 && screenSupportsExtendedColor());

Please omit the parentheses if you actually keep this.

[Dean Jackson]

Created attachment 275577 [details]
Patch

[WebKit Commit Bot]

Attachment 275577 [details] did not pass style-queue:


ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:253:  color_gamutMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 1 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Dean Jackson]

Comment on attachment 275147 [details]
Patch

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

>> Source/WebCore/css/MediaQueryEvaluator.cpp:265
>> +    // color" display is at least as good as P3.
> 
> I think this FIXME could be clearer. I think the real issue is that we will always return false for rec2020 and it might be better to be more specific about this. Maybe the code should be written like this:
> 
>     switch (downcast<CSSPrimitiveValue>(*value).getValueID()) {
>     case CSSValueSrgb:
>         return true;
>     case CSSValueP3:
>         return screenSupportsExtendedColor();
>     case CSSValueRec2020:
>         return false;
>     }
> 
>     ASSERT_NOT_REACHED();
>     return true;
> 
> I think the behavior would be clearer then; the FIXME could go in the CSSValueRec2020 case.

Much better

[Alexey Shvayka]

(In reply to Dean Jackson from comment #5)
> Created attachment 275577 [details]
> Patch

Landed in https://trac.webkit.org/changeset/199024.

[Radar WebKit Bug Importer]

<rdar://problem/61313919>