Bug 208971 - [GPU Process] GraphicsContextStateChange must accumulate fill and stroke fields as single properties
Summary: [GPU Process] GraphicsContextStateChange must accumulate fill and stroke fiel...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Canvas (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Said Abou-Hallawa
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-03-11 19:46 PDT by Said Abou-Hallawa
Modified: 2020-03-12 01:11 PDT (History)
7 users (show)

See Also:


Attachments
test case (746 bytes, text/html)
2020-03-11 19:49 PDT, Said Abou-Hallawa
no flags Details
Patch (6.19 KB, patch)
2020-03-11 20:24 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (9.52 KB, patch)
2020-03-11 22:27 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Said Abou-Hallawa 2020-03-11 19:46:26 PDT
The fill and stroke can be color, gradient or pattern. When setting the fill or the stroke of the GraphicsContextState to any of them, the other two are nullified. GraphicsContextStateChange should behave similarly when any of the fill or the stroke flags is true.
Comment 1 Said Abou-Hallawa 2020-03-11 19:49:24 PDT
Created attachment 393331 [details]
test case
Comment 2 Said Abou-Hallawa 2020-03-11 19:49:43 PDT
Repro steps:

1. Launch mini-browser
2. Enable Settings/Internal Features/Render Canvas in GPU Process
3. Open the attached test case

Result: the test case shows a red rectangle
Expected: the test case shows a green rectangle
Comment 3 Said Abou-Hallawa 2020-03-11 19:55:06 PDT
The same bug happens when enabling Settings/Enable Display List Drawing.
Comment 4 Said Abou-Hallawa 2020-03-11 20:24:45 PDT
Created attachment 393332 [details]
Patch
Comment 5 Said Abou-Hallawa 2020-03-11 20:26:10 PDT
This fixes the following tests:

canvas/philip/tests/2d.pattern.modify.canvas2.html
canvas/philip/tests/2d.pattern.modify.image2.html

when using the following command:

run-webkit-tests --debug --no-retry  --internal-feature RenderCanvasInGPUProcessEnabled LayoutTests/canvas/philip/tests/2d.pattern.modify.canvas2.html LayoutTests/canvas/philip/tests/2d.pattern.modify.image2.html
Comment 6 Simon Fraser (smfr) 2020-03-11 20:44:38 PDT
Comment on attachment 393332 [details]
Patch

Should there be a TestExpectations change?
Comment 7 Said Abou-Hallawa 2020-03-11 22:15:57 PDT
(In reply to Simon Fraser (smfr) from comment #6)
> Comment on attachment 393332 [details]
> Patch
> 
> Should there be a TestExpectations change?

The two tests I mentioned above fail only when running 'run-webkit-tests --internal-feature RenderCanvasInGPUProcessEnabled'. The attached test case fails in the browser when enabling GPU reddening or DisplayList rendering. The patch fixes the following scenario:

    var pattern = ctx1.createPattern(canvas2, 'no-repeat');
    ctx1.fillStyle = pattern;
    ctx1.fillRect(0, 0, canvas1.width, canvas1.height);

    ctx1.fillStyle = 'red';
    ctx1.fillRect(0, 0, canvas1.width, canvas1.height);

    ctx1.fillStyle = pattern;
    ctx1.fillRect(0, 0, canvas1.width, canvas1.height);

The bug is in the last fillStyle. GraphicsContextStateChange::changesFromState() returns 0 because although the flag GraphicsContextState::FillPatternChange is set, the value of the property 'fillPattern' is the same in m_state and state. So no SetState item is created for the last fillRect.
Comment 8 Said Abou-Hallawa 2020-03-11 22:27:43 PDT
Created attachment 393341 [details]
Patch
Comment 9 WebKit Commit Bot 2020-03-12 01:10:20 PDT
Comment on attachment 393341 [details]
Patch

Clearing flags on attachment: 393341

Committed r258317: <https://trac.webkit.org/changeset/258317>
Comment 10 WebKit Commit Bot 2020-03-12 01:10:22 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 Radar WebKit Bug Importer 2020-03-12 01:11:15 PDT
<rdar://problem/60360906>