Bug 89018 – [Chromium] webkitImageSmoothingEnabled canvas property does not work on redraw

• Component: Canvas
• Status: RESOLVED FIXED

[Justin Novosad]

This is in reference to chromium bug: crbug.com/130925

Attached test page shows canvas to canvas copy that appears to be filtered despite having set the webkitImageSmoothingEnabled property set to false.
Eliminating the animation loop makes the problem go away.  The problem only manifests itself on redraw.

The source of the problem is that the accelerated compositor modifies the state of an openGL texture object (sets filtering to linear).  That object is shared with a skia rendering context and the compositor does not sending a state invalidation signal to skia. Therefore, the state cache maintained by skia is no longer in sync.  This explains why the problem only happens on redraw, i.e. the second draw happens after the compositor has drawn the source canvas to screen at least once.

[Justin Novosad]

Created attachment 147358 [details]
test html file

[Justin Novosad]

Created attachment 147573 [details]
Patch

[Stephen White]

Comment on attachment 147573 [details]
Patch

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

Clean up the ChangeLog a bit, and you're good to go.

> Source/WebCore/ChangeLog:10
> +        When an accelerated canvas layer prepares it texture for the

Nit:  it -> its

> Source/WebCore/ChangeLog:12
> +        proprties that are cached by skia, since the compositor may modify them.

Since the bug title above is not very precise, I'd further describe the problematic case here.  Something like "webkitImageSmoothingEnabled=false on a canvas used as a source for canvas-to-canvas draws does not work on compositor redraw, since the compositor sets the texture filtering mode back to linear at draw time.  Since texture attributes are associated with the texture object in GL, this leaves the value stored in the skia state cache out-of-date."

> LayoutTests/fast/canvas/script-tests/canvas-imageSmoothingEnabled-repaint.js:72
> +window.onload = onLoadHandler;

Thanks for taking the extra time to make a test for this tricky case!

[Justin Novosad]

Created attachment 147579 [details]
Patch

[Stephen White]

Comment on attachment 147579 [details]
Patch

Thanks.  r=me

[WebKit Review Bot]

Comment on attachment 147579 [details]
Patch

Clearing flags on attachment: 147579

Committed r120344: <http://trac.webkit.org/changeset/120344>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Beth Dakin]

This test fails on the Mac bots. I will skip it.

[Beth Dakin]

(In reply to comment #8)
> This test fails on the Mac bots. I will skip it.

Skipped with r120952. Filed https://bugs.webkit.org/show_bug.cgi?id=89680