Bug 208971

Summary:

[GPU Process] GraphicsContextStateChange must accumulate fill and stroke fields as single properties

Product:

WebKit

Reporter:

Said Abou-Hallawa <sabouhallawa>

Component:

Canvas

Assignee:

Said Abou-Hallawa <sabouhallawa>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, dino, mmaxfield, simon.fraser, thorton, webkit-bug-importer, wenson_hsieh

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
test case	none
Patch	none
Patch	none

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>