Summary: | Add color-gamut media query support | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dean Jackson <dino> | ||||||
Component: | New Bugs | Assignee: | Dean Jackson <dino> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | ashvayka, commit-queue, eoconnor, jer.noble, jonlee, simon.fraser, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Dean Jackson
2016-03-29 16:20:38 PDT
Created attachment 275147 [details]
Patch
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.
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. Created attachment 275577 [details]
Patch
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.
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 (In reply to Dean Jackson from comment #5) > Created attachment 275577 [details] > Patch Landed in https://trac.webkit.org/changeset/199024. |