⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 267645 in webkit


Ignore:
Timestamp:
Sep 26, 2020, 7:48:39 PM (6 years ago)
Author:
Wenson Hsieh
Message:

Remove support for setting CMYKA fill and stroke colors in 2D canvas
​https://bugs.webkit.org/show_bug.cgi?id=217002

Reviewed by Darin Adler.

Source/WebCore:

Remove support for setFillColor(c, m, y, k, a) and setStrokeColor(c, m, y, k, a) on
CanvasRenderingContext2D, which was initially introduced in 2004 with <​https://trac.webkit.org/r6730>.
Setting colors from CMYKA components is non-standard and unsupported in both Firefox and Chrome;
importantly, this is implemented in CanvasStyle::applyStrokeColor by calling directly into CoreGraphics
API using the platform CGContextRef, which is null when using the GPU Process; this subsequently causes
the web process to crash.

In lieu of adding new DisplayList items to support setting fill and stroke colors with CMYKA components,
we can take this opportunity to just drop support for this canvas API entirely. Fixes the following tests
when using the GPU Process to render canvas:

  • fast/canvas/canvas-color-serialization.html
  • fast/canvas/canvas-overloads-setFillColor.html
  • fast/canvas/canvas-overloads-setStrokeColor.html
  • html/canvas/CanvasRenderingContext2D.idl:
  • html/canvas/CanvasRenderingContext2DBase.cpp:
  • html/canvas/CanvasStyle.cpp:

(WebCore::CanvasStyle::isEquivalentColor const):
(WebCore::CanvasStyle::applyStrokeColor const):
(WebCore::CanvasStyle::applyFillColor const):

  • html/canvas/CanvasStyle.h:

(WebCore::CanvasStyle::color const):

LayoutTests:

Remove calls to CanvasRenderingContext2D.setFillColor(c, m, y, k, a) and
CanvasRenderingContext2D.setStrokeColor(c, m, y, k, a) in these layout tests.

  • fast/canvas/canvas-color-serialization-expected.txt:
  • fast/canvas/canvas-color-serialization.html:
  • fast/canvas/set-colors-expected.txt:
  • fast/canvas/set-colors.html:
  • inspector/canvas/recording-2d-full-expected.txt:
  • inspector/canvas/resources/recording-2d.js:
  • platform/ios/fast/canvas/set-colors-expected.txt:
  • platform/mac/fast/canvas/set-colors-expected.txt:
Location:
trunk
Files:
14 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r267644 r267645  
     12020-09-26  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        Remove support for setting CMYKA fill and stroke colors in 2D canvas
     4        https://bugs.webkit.org/show_bug.cgi?id=217002
     5
     6        Reviewed by Darin Adler.
     7
     8        Remove calls to `CanvasRenderingContext2D.setFillColor(c, m, y, k, a)` and
     9        `CanvasRenderingContext2D.setStrokeColor(c, m, y, k, a)` in these layout tests.
     10
     11        * fast/canvas/canvas-color-serialization-expected.txt:
     12        * fast/canvas/canvas-color-serialization.html:
     13        * fast/canvas/set-colors-expected.txt:
     14        * fast/canvas/set-colors.html:
     15        * inspector/canvas/recording-2d-full-expected.txt:
     16        * inspector/canvas/resources/recording-2d.js:
     17        * platform/ios/fast/canvas/set-colors-expected.txt:
     18        * platform/mac/fast/canvas/set-colors-expected.txt:
     19
    1202020-09-26  Darin Adler  <darin@apple.com>
    221

  • trunk/LayoutTests/fast/canvas/canvas-color-serialization-expected.txt

    r170933 r267645  
    2525PASS trySettingStrokeColorRGBA(255, 0, 0, 0.4) is 'rgba(255, 0, 0, 0.4)'
    2626PASS trySettingShadowRGBA(255, 0, 0, 0.4) is 'rgba(255, 0, 0, 0.4)'
    27 PASS trySettingFillColorCMYKA(0, 0, 0, 0, 0) is 'rgba(255, 255, 255, 0)'
    28 PASS trySettingStrokeColorCMYKA(0, 0, 0, 0, 0) is 'rgba(255, 255, 255, 0)'
    29 PASS trySettingShadowCMYKA(0, 0, 0, 0, 0) is 'rgba(255, 255, 255, 0)'
    30 PASS trySettingFillColorCMYKA(0, 0, 0, 0, 1) is '#ffffff'
    31 PASS trySettingStrokeColorCMYKA(0, 0, 0, 0, 1) is '#ffffff'
    32 PASS trySettingShadowCMYKA(0, 0, 0, 0, 1) is '#ffffff'
    33 PASS trySettingFillColorCMYKA(0, 1, 0, 0, 0) is 'rgba(255, 0, 255, 0)'
    34 PASS trySettingStrokeColorCMYKA(0, 1, 0, 0, 0) is 'rgba(255, 0, 255, 0)'
    35 PASS trySettingShadowCMYKA(0, 1, 0, 0, 0) is 'rgba(255, 0, 255, 0)'
    36 PASS trySettingFillColorCMYKA(0, 1, 0, 0, 1) is '#ff00ff'
    37 PASS trySettingStrokeColorCMYKA(0, 1, 0, 0, 1) is '#ff00ff'
    38 PASS trySettingShadowCMYKA(0, 1, 0, 0, 1) is '#ff00ff'
    39 PASS trySettingFillColorCMYKA(0, 0, 0, 1, 0) is 'rgba(0, 0, 0, 0)'
    40 PASS trySettingStrokeColorCMYKA(0, 0, 0, 1, 0) is 'rgba(0, 0, 0, 0)'
    41 PASS trySettingShadowCMYKA(0, 0, 0, 1, 0) is 'rgba(0, 0, 0, 0)'
    42 PASS trySettingFillColorCMYKA(0, 0, 0, 1, 1) is '#000000'
    43 PASS trySettingStrokeColorCMYKA(0, 0, 0, 1, 1) is '#000000'
    44 PASS trySettingShadowCMYKA(0, 0, 0, 1, 1) is '#000000'
    4527PASS trySettingFillColorGrayLevel(0) is '#000000'
    4628PASS trySettingStrokeColorGrayLevel(0) is '#000000'

  • trunk/LayoutTests/fast/canvas/canvas-color-serialization.html

    r217390 r267645  
    8282}
    8383
    84 function trySettingFillColorCMYKA(c, m, y, k, a) {
    85     ctx.fillStyle = '#666';
    86     ctx.setFillColor(c, m, y, k, a);
    87     return ctx.fillStyle;
    88 }
    89 
    90 function trySettingStrokeColorCMYKA(c, m, y, k, a) {
    91     ctx.strokeStyle = '#666';
    92     ctx.setStrokeColor(c, m, y, k, a);
    93     return ctx.strokeStyle;
    94 }
    95 
    96 function trySettingShadowCMYKA(c, m, y, k, a) {
    97     ctx.strokeStyle = '#666';
    98     ctx.setStrokeColor(c, m, y, k, a);
    99     return ctx.strokeStyle;
    100 }
    101 
    102 function trySettingCMYKA(c, m, y, k, a, expected) {
    103     shouldBe("trySettingFillColorCMYKA(" + c + ", " + m + ", " + y + ", " + k + ", " + a + ")", expected);
    104     shouldBe("trySettingStrokeColorCMYKA(" + c + ", " + m + ", " + y + ", " + k + ", " + a + ")", expected);
    105     shouldBe("trySettingShadowCMYKA(" + c + ", " + m + ", " + y + ", " + k + ", " + a + ")", expected);
    106 }
    107 
    10884function trySettingFillColorWithOverrideAlpha(color, alpha) {
    10985    ctx.fillStyle = '#666';
    … …  
    199175trySettingRGBA(255, 0, 0, 0.0, "'rgba(255, 0, 0, 0)'");
    200176trySettingRGBA(255, 0, 0, 0.4, "'rgba(255, 0, 0, 0.4)'");
    201 
    202 trySettingCMYKA(0, 0, 0, 0, 0.0, "'rgba(255, 255, 255, 0)'");
    203 trySettingCMYKA(0, 0, 0, 0, 1.0, "'#ffffff'");
    204 trySettingCMYKA(0, 1, 0, 0, 0.0, "'rgba(255, 0, 255, 0)'");
    205 trySettingCMYKA(0, 1, 0, 0, 1.0, "'#ff00ff'");
    206 trySettingCMYKA(0, 0, 0, 1, 0.0, "'rgba(0, 0, 0, 0)'");
    207 trySettingCMYKA(0, 0, 0, 1, 1.0, "'#000000'");
    208177
    209178trySettingGrayLevel(0.0, "'#000000'");

  • trunk/LayoutTests/fast/canvas/set-colors-expected.txt

    r158528 r267645  
    3232PASS testSetFillColor('0, 1, 0, 1') is green
    3333PASS testSetFillColor('0, 1, 0, 0.8') is translucentGreen
    34 FAIL testSetFillColor('0, 0, 0, 1, 1') should be #1a1a1a. Was #000000.
    35 FAIL testSetFillColor('0, 0, 0, 1, 0.8') should be rgba(25, 25, 25, 0.8). Was rgba(0, 0, 0, 0.8).
    36 PASS testSetFillColor('0, 0, 0, 1, 0') is transparent
    3734PASS testStrokeStyle('transparent') is transparent
    3835PASS testStrokeStyle('blue') is blue
    … …  
    6360PASS testSetStrokeColor('0, 1, 0, 1') is green
    6461PASS testSetStrokeColor('0, 1, 0, 0.8') is translucentGreen
    65 FAIL testSetStrokeColor('0, 0, 0, 1, 1') should be #1a1a1a. Was #000000.
    66 FAIL testSetStrokeColor('0, 0, 0, 1, 0.8') should be rgba(25, 25, 25, 0.8). Was rgba(0, 0, 0, 0.8).
    67 PASS testSetStrokeColor('0, 0, 0, 1, 0') is transparent
    6862PASS successfullyParsed is true
    6963

  • trunk/LayoutTests/fast/canvas/set-colors.html

    r217390 r267645  
    133133shouldBe("testSetFillColor('0, 1, 0, 1')", "green");
    134134shouldBe("testSetFillColor('0, 1, 0, 0.8')", "translucentGreen");
    135 shouldBe("testSetFillColor('0, 0, 0, 1, 1')", "'#1a1a1a'");                   // This test is expected to fail on older versions of Mac OS X.
    136 shouldBe("testSetFillColor('0, 0, 0, 1, 0.8')", "'rgba(25, 25, 25, 0.8)'");   // Ditto.
    137 shouldBe("testSetFillColor('0, 0, 0, 1, 0')", "transparent");
    138135shouldBe("testStrokeStyle('transparent')", "transparent");
    139136shouldBe("testStrokeStyle('blue')", "blue");
    … …  
    164161shouldBe("testSetStrokeColor('0, 1, 0, 1')", "green");
    165162shouldBe("testSetStrokeColor('0, 1, 0, 0.8')", "translucentGreen");
    166 shouldBe("testSetStrokeColor('0, 0, 0, 1, 1')", "'#1a1a1a'");                  // This test is expected to fail on older versions of Mac OS X.
    167 shouldBe("testSetStrokeColor('0, 0, 0, 1, 0.8')", "'rgba(25, 25, 25, 0.8)'");  // Ditto.
    168 shouldBe("testSetStrokeColor('0, 0, 0, 1, 0')", "transparent");
    169163</script>
    170164<script src="../../resources/js-test-post.js"></script>

  • trunk/LayoutTests/inspector/canvas/recording-2d-full-expected.txt

    r267000 r267645  
    696696        1: (anonymous function)
    697697        2: executeFrameFunction
    698     5: setFillColor(9, 10, 11, 12, 13)
    699       swizzleTypes: [Number, Number, Number, Number, Number]
    700       trace:
    701         0: setFillColor
    702         1: (anonymous function)
    703         2: executeFrameFunction
    704698  50: (duration)
    705699    0: setLineCap("")
    … …  
    829823        1: (anonymous function)
    830824        2: executeFrameFunction
    831     5: setStrokeColor(9, 10, 11, 12, 13)
    832       swizzleTypes: [Number, Number, Number, Number, Number]
    833       trace:
    834         0: setStrokeColor
    835         1: (anonymous function)
    836         2: executeFrameFunction
    837825  57: (duration)
    838826    0: setTransform(1, 2, 3, 4, 5, 6)

  • trunk/LayoutTests/inspector/canvas/resources/recording-2d.js

    r246016 r267645  
    274274            ctx.setFillColor(3, 4);
    275275            ctx.setFillColor(5, 6, 7, 8);
    276             ctx.setFillColor(9, 10, 11, 12, 13);
    277276        },
    278277        () => {
    … …  
    309308            ctx.setStrokeColor(3, 4);
    310309            ctx.setStrokeColor(5, 6, 7, 8);
    311             ctx.setStrokeColor(9, 10, 11, 12, 13);
    312310        },
    313311        () => {

  • trunk/LayoutTests/platform/ios/fast/canvas/set-colors-expected.txt

    r199788 r267645  
    3232PASS testSetFillColor('0, 1, 0, 1') is green
    3333PASS testSetFillColor('0, 1, 0, 0.8') is translucentGreen
    34 FAIL testSetFillColor('0, 0, 0, 1, 1') should be #1a1a1a. Was #1a1919.
    35 FAIL testSetFillColor('0, 0, 0, 1, 0.8') should be rgba(25, 25, 25, 0.8). Was rgba(26, 25, 25, 0.8).
    36 PASS testSetFillColor('0, 0, 0, 1, 0') is transparent
    3734PASS testStrokeStyle('transparent') is transparent
    3835PASS testStrokeStyle('blue') is blue
    … …  
    6360PASS testSetStrokeColor('0, 1, 0, 1') is green
    6461PASS testSetStrokeColor('0, 1, 0, 0.8') is translucentGreen
    65 FAIL testSetStrokeColor('0, 0, 0, 1, 1') should be #1a1a1a. Was #1a1919.
    66 FAIL testSetStrokeColor('0, 0, 0, 1, 0.8') should be rgba(25, 25, 25, 0.8). Was rgba(26, 25, 25, 0.8).
    67 PASS testSetStrokeColor('0, 0, 0, 1, 0') is transparent
    6862PASS successfullyParsed is true
    6963

  • trunk/LayoutTests/platform/mac/fast/canvas/set-colors-expected.txt

    r106179 r267645  
    3232PASS testSetFillColor('0, 1, 0, 1') is green
    3333PASS testSetFillColor('0, 1, 0, 0.8') is translucentGreen
    34 FAIL testSetFillColor('0, 0, 0, 1, 1') should be #1a1a1a. Was #1a1919.
    35 FAIL testSetFillColor('0, 0, 0, 1, 0.8') should be rgba(25, 25, 25, 0.8). Was rgba(26, 25, 25, 0.8).
    36 PASS testSetFillColor('0, 0, 0, 1, 0') is transparent
    3734PASS testStrokeStyle('transparent') is transparent
    3835PASS testStrokeStyle('blue') is blue
    … …  
    6360PASS testSetStrokeColor('0, 1, 0, 1') is green
    6461PASS testSetStrokeColor('0, 1, 0, 0.8') is translucentGreen
    65 FAIL testSetStrokeColor('0, 0, 0, 1, 1') should be #1a1a1a. Was #1a1919.
    66 FAIL testSetStrokeColor('0, 0, 0, 1, 0.8') should be rgba(25, 25, 25, 0.8). Was rgba(26, 25, 25, 0.8).
    67 PASS testSetStrokeColor('0, 0, 0, 1, 0') is transparent
    6862PASS successfullyParsed is true
    6963

  • trunk/Source/WebCore/ChangeLog

    r267639 r267645  
     12020-09-26  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        Remove support for setting CMYKA fill and stroke colors in 2D canvas
     4        https://bugs.webkit.org/show_bug.cgi?id=217002
     5
     6        Reviewed by Darin Adler.
     7
     8        Remove support for `setFillColor(c, m, y, k, a)` and `setStrokeColor(c, m, y, k, a)` on
     9        `CanvasRenderingContext2D`, which was initially introduced in 2004 with <https://trac.webkit.org/r6730>.
     10        Setting colors from CMYKA components is non-standard and unsupported in both Firefox and Chrome;
     11        importantly, this is implemented in `CanvasStyle::applyStrokeColor` by calling directly into CoreGraphics
     12        API using the platform `CGContextRef`, which is null when using the GPU Process; this subsequently causes
     13        the web process to crash.
     14
     15        In lieu of adding new DisplayList items to support setting fill and stroke colors with CMYKA components,
     16        we can take this opportunity to just drop support for this canvas API entirely. Fixes the following tests
     17        when using the GPU Process to render canvas:
     18        - fast/canvas/canvas-color-serialization.html
     19        - fast/canvas/canvas-overloads-setFillColor.html
     20        - fast/canvas/canvas-overloads-setStrokeColor.html
     21
     22        * html/canvas/CanvasRenderingContext2D.idl:
     23        * html/canvas/CanvasRenderingContext2DBase.cpp:
     24        * html/canvas/CanvasStyle.cpp:
     25        (WebCore::CanvasStyle::isEquivalentColor const):
     26        (WebCore::CanvasStyle::applyStrokeColor const):
     27        (WebCore::CanvasStyle::applyFillColor const):
     28        * html/canvas/CanvasStyle.h:
     29        (WebCore::CanvasStyle::color const):
     30
    1312020-09-26  Chris Dumez  <cdumez@apple.com>
    232

  • trunk/Source/WebCore/html/canvas/CanvasRenderingContext2D.idl

    r267000 r267645  
    4747    undefined setStrokeColor(unrestricted float grayLevel, optional float alpha = 1);
    4848    undefined setStrokeColor(unrestricted float r, unrestricted float g, unrestricted float b, unrestricted float a);
    49     undefined setStrokeColor(unrestricted float c, unrestricted float m, unrestricted float y, unrestricted float k, unrestricted float a);
    5049    undefined setFillColor(DOMString color, optional unrestricted float alpha);
    5150    undefined setFillColor(unrestricted float grayLevel, optional unrestricted float alpha = 1);
    5251    undefined setFillColor(unrestricted float r, unrestricted float g, unrestricted float b, unrestricted float a);
    53     undefined setFillColor(unrestricted float c, unrestricted float m, unrestricted float y, unrestricted float k, unrestricted float a);
    5452
    5553    // Non-standard functionality (CanvasImageData).

  • trunk/Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp

    r264585 r267645  
    932932}
    933933
    934 void CanvasRenderingContext2DBase::setStrokeColor(float c, float m, float y, float k, float a)
    935 {
    936     auto color = CMYKA { c, m, y, k, a };
    937     if (state().strokeStyle.isEquivalent(color))
    938         return;
    939     setStrokeStyle(CanvasStyle(color));
    940 }
    941 
    942934void CanvasRenderingContext2DBase::setFillColor(const String& color, Optional<float> alpha)
    943935{
    … …  
    966958{
    967959    auto color = SRGBA { r, g, b, a };
    968     if (state().fillStyle.isEquivalent(color))
    969         return;
    970     setFillStyle(CanvasStyle(color));
    971 }
    972 
    973 void CanvasRenderingContext2DBase::setFillColor(float c, float m, float y, float k, float a)
    974 {
    975     auto color = CMYKA { c, m, y, k, a };
    976960    if (state().fillStyle.isEquivalent(color))
    977961        return;

  • trunk/Source/WebCore/html/canvas/CanvasStyle.cpp

    r264333 r267645  
    100100}
    101101
    102 CanvasStyle::CanvasStyle(const CMYKA<float>& colorComponents)
    103     : m_style(CMYKAColor { convertToComponentBytes(toSRGBA(colorComponents)), colorComponents })
    104 {
    105 }
    106 
    107102CanvasStyle::CanvasStyle(CanvasGradient& gradient)
    108103    : m_style(makeRefPtr(gradient))
    … …  
    149144        return WTF::get<Color>(m_style) == WTF::get<Color>(other.m_style);
    150145
    151     if (WTF::holds_alternative<CMYKAColor>(m_style) && WTF::holds_alternative<CMYKAColor>(other.m_style))
    152         return WTF::get<CMYKAColor>(m_style).components == WTF::get<CMYKAColor>(other.m_style).components;
    153 
    154146    return false;
    155147}
    … …  
    160152}
    161153
    162 bool CanvasStyle::isEquivalent(const CMYKA<float>& components) const
    163 {
    164     return WTF::holds_alternative<CMYKAColor>(m_style) && WTF::get<CMYKAColor>(m_style).components == components;
    165 }
    166 
    167154void CanvasStyle::applyStrokeColor(GraphicsContext& context) const
    168155{
    … …  
    170157        [&context] (const Color& color) {
    171158            context.setStrokeColor(color);
    172         },
    173         [&context] (const CMYKAColor& color) {
    174             // FIXME: Do this through platform-independent GraphicsContext API.
    175             // We'll need a fancier Color abstraction to support CMYKA correctly
    176 #if USE(CG)
    177             CGContextSetCMYKStrokeColor(context.platformContext(), color.components.cyan, color.components.magenta, color.components.yellow, color.components.black, color.components.alpha);
    178 #else
    179             context.setStrokeColor(color.colorConvertedToSRGBA);
    180 #endif
    181159        },
    182160        [&context] (const RefPtr<CanvasGradient>& gradient) {
    … …  
    201179            context.setFillColor(color);
    202180        },
    203         [&context] (const CMYKAColor& color) {
    204             // FIXME: Do this through platform-independent GraphicsContext API.
    205             // We'll need a fancier Color abstraction to support CMYKA correctly
    206 #if USE(CG)
    207             CGContextSetCMYKFillColor(context.platformContext(), color.components.cyan, color.components.magenta, color.components.yellow, color.components.black, color.components.alpha);
    208 #else
    209             context.setFillColor(color.colorConvertedToSRGBA);
    210 #endif
    211         },
    212181        [&context] (const RefPtr<CanvasGradient>& gradient) {
    213182            context.setFillGradient(gradient->gradient());

  • trunk/Source/WebCore/html/canvas/CanvasStyle.h

    r263788 r267645  
    6969    struct Invalid { };
    7070
    71     struct CMYKAColor {
    72         Color colorConvertedToSRGBA;
    73         CMYKA<float> components;
    74     };
    75 
    7671    struct CurrentColor {
    7772        Optional<float> overrideAlpha;
    … …  
    8075    CanvasStyle(CurrentColor);
    8176
    82     Variant<Invalid, Color, CMYKAColor, RefPtr<CanvasGradient>, RefPtr<CanvasPattern>, CurrentColor> m_style;
     77    Variant<Invalid, Color, RefPtr<CanvasGradient>, RefPtr<CanvasPattern>, CurrentColor> m_style;
    8378};
    8479
    … …  
    110105inline String CanvasStyle::color() const
    111106{
    112     return serializationForHTML(WTF::holds_alternative<Color>(m_style) ? WTF::get<Color>(m_style) : WTF::get<CMYKAColor>(m_style).colorConvertedToSRGBA);
     107    return serializationForHTML(WTF::get<Color>(m_style));
    113108}
    114109
Note: See TracChangeset for help on using the changeset viewer.