New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 769760 link

Starred by 7 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Regression

Blocking:
issue 770214


Show other hotlists

Hotlists containing this issue:
Hotlist-1
Hotlist-1


Sign in to add a comment

Canvas 2D context putImageData renders wrong pixel data

Reported by c.mat...@gmail.com, Sep 28 2017

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/61.0.3163.100 Safari/537.36

Steps to reproduce the problem:
1. open test case file

What is the expected behavior?
The test case file javascript

 - creates 2 canvas elements
 - on window.requestAnimationFrame 
     -puts red pixel data into the first and green into the second with putImageData
     -copys the canvases into canvas DOM elements with drawImage

Expected behaviour is that 1 red and 1 green canvas is displayed 

What went wrong?
2 green canvases are displayed. 

Did this work before? Yes 

Does this work in other browsers? Yes

Chrome version: 61.0.3163.100  Channel: stable
OS Version: 10.0
Flash Version: 

disabling hardware acceleration for the canvas fixes the problem. 

calling getImageData on the off-screen canvas before calling putImageData also fixes the problem.
 
chromeBug.html
3.2 KB View Download
Labels: Needs-Bisect Needs-Triage-M61

Comment 2 by junov@chromium.org, Sep 28 2017

Owner: junov@chromium.org
Status: Started (was: Unconfirmed)
Was able to repro on Win10, not on linux.

Comment 3 by junov@chromium.org, Sep 28 2017

Components: Internals>Skia
Labels: -Pri-2 -Needs-Bisect ReleaseBlock-Stable M-62 Pri-1
Owner: robertphillips@chromium.org
Status: Assigned (was: Started)
Bisected using bot win32 and win64 build archives.  The intersection of the two bisects gives this revision range: 

https://chromium.googlesource.com/chromium/src/+log/bc7cb0b8fcc83513f65f7ea8e5a3475b26d66354..5c36dc747da0feb63a206edaf0dfc470471ade20

This range containg the following skia roll:
https://skia.googlesource.com/skia.git/+log/4f12f1e2093c..eee3c09e96d0

Suspecting:
https://skia.googlesource.com/skia.git/+/fbcef6eb8abad142daf45418516550f7635b4a52



Comment 4 by junov@chromium.org, Sep 28 2017

You can see the canvas flicker red when you hit page reload.  So the initial draw is probably okay, and problem happens in later animation frames.

Comment 5 by junov@chromium.org, Sep 28 2017

Here is an alternate version of the test case that clearly shows that it is specifically the putImageData operation that is malfunctioning
chromeBug-alternate.html
2.8 KB View Download

Comment 6 by junov@chromium.org, Sep 28 2017

@c.matear: If you need a temporary workaround for website that is in production, try resetting the canvas instead of calling getImageData. getImageData is costly and will cause the canvas to switch to software rendering mode, which will slow down your web application.  To reset the 2d context in a lightweight manner, do this right before the call to putImageData:

offscreenContext2d.canvas.width = offscreenContext2d.canvas.width;

Comment 7 by c.mat...@gmail.com, Sep 29 2017

@junov, Thanks for looking at this so quickly. We have tried the workaround that you have suggested but it doesn’t appear to work for us. It seems that it only works for smaller image sizes. 

In chromeBug-alternate the image size is 200 x 200 px and the fix works, 
In the original chromeBug file the image size is 400 x 400, the suggested workaround doesn’t work for that. If I change the image data size to 200x200 in chromeBug then the suggested workaround works. 

We did find that doing:
offscreenContext2d.canvas.width = offscreenContext2d.canvas.width+1;
offscreenContext2d.canvas.width = offscreenContext2d.canvas.width-1;

did fix our problem, however we have measured the performance of this and found a measurable difference, is there something else we can do? 

Comment 8 by junov@chromium.org, Sep 29 2017

I'm out of ideas for workarounds. The +1/-1 trick you found is slow because it results a buffer reallocation and a clear. It's not great, but it might be your best solution until we can deploy a fix to the stable channel.

robertphillips@: Any update?
Status: Started (was: Assigned)
I've reproduced the bug locally and have begun trying to figure out what is going wrong.
I think I have a very similar / identical issue with the same windows and Chrome version as c.mat:

I do linewisely populate 2 canvas elements with red and green lines, respectively. With hardware-acc. enabled, both are red, with the second one only having the last line being green (whereas the entire canvas should be green).

https://jsfiddle.net/tnawf4kj/5/

Interestingly, if I avoid to change the canvas size in the JS part, it works correctly... Changing width +1-1 doesnt change the outcome.

Any workaround ideas for my case?
Project Member

Comment 12 by bugdroid1@chromium.org, Oct 2 2017

Comment 13 by kbr@chromium.org, Oct 2 2017

Blocking: 770214

Comment 14 by kbr@chromium.org, Oct 2 2017

Cc: kbr@chromium.org
This is likely the same issue as seen in a partner's application in Issue 770214. We'd like to make sure this is merged back to M62 (and that it fixes the bug). Thanks.

Roger that. I'll definitely push it back to M62 (once it has cooked for a while) and see what people think about pushing it back to M61.

The fix from #12 (https://skia-review.googlesource.com/c/skia/+/54004) rolled into Chrome on 10/2 in https://chromium-review.googlesource.com/c/chromium/src/+/695936 at r505849.

This made it into today's Mac 63.0.3231.0 Canary but, unfortunately, not the Windows Canary.
Labels: Merge-Request-62
Project Member

Comment 18 by sheriffbot@chromium.org, Oct 3 2017

Labels: -Merge-Request-62 Merge-Review-62 Hotlist-Merge-Review
This bug requires manual review: We are only 13 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Issue 770214 has been merged into this issue.

Comment 20 by kbr@chromium.org, Oct 3 2017

Thanks robertphillips@. Is there a regression test somewhere to prevent a bug like this from happening again?

Comment 21 by kbr@chromium.org, Oct 3 2017

Cc: junov@chromium.org
Replying to #19. There isn't a regression test yet. The fix CL is a rather brute force disabling of the feature just to get the everything working again. I'll add one to Skia before I re-enable the feature.

Currently I believe the issue has to do with how Skia changes its resource allocation policy (to favor flushes over VRAM usage) when ANGLE is being used but the details are still TBD.
Thanks for the fix. Let's wait until this is tested in Windows Canary, and I'll re-review merge. 
Hi,
Testing in the latest canary version in Windows this appears to be fixed. I see in this thread there was discussion of pushing to M62, and possibly M61. Can I ask, 
has it made it into M62? When would M62 make it into the main windows release of Chrome? Is M62 released as a stable Chrome release? I see Chromium M62 is the ~17th October (https://www.chromium.org/developers/calendar). What are the thoughts about a port to M61?
Thanks!
Dave.

Comment 25 Deleted

Comment 26 by sger...@gmail.com, Oct 4 2017

I thought I'd chime in and say that we were also affected by this and have released an update that utilizes the workaround suggested in #7 until the fix on Canary gets into Stable:

context.canvas.width = context.canvas.width + 1;
context.canvas.width = context.canvas.width - 1;

I'm not sure how much this adds to the conversation, but before I found this thread I was doing some testing on my own and created this JSFiddle: http://jsfiddle.net/avux1q3r/5/ which clearly shows the behavior when you click the "Render" button twice (the two canvases should be colored red and green, but on the second click, they are both green). Interestingly, it seems to be only affecting canvases whose total pixel count is less than 65537, so, canvases that are 256x256 are not affected by the bug, but add a single pixel in width and height (so 257x257) and now the problem appears. This is true both in square canvases as well as rectangular ones (512x128 works fine, but 513x129 does not). Not sure if that helps diagnose the root cause at all, but figured it couldn't hurt to post.
Labels: -Merge-Review-62 Merge-Approved-62
Thanks for the fix robertphillips@! Approving this merge for M62. Branch:3202
robertphillips@, Merge request for M62 has been approved. Can you please do the merge ASAP?

Thank you!
https://skia-review.googlesource.com/c/skia/+/55762 ([M62 cherry-pick] Temporarily disable deferred texture proxies)
Project Member

Comment 30 by bugdroid1@chromium.org, Oct 5 2017

Labels: merge-merged-m62
The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/00cccf1cbf45591039b0a8d0dbb38115fac201ee

commit 00cccf1cbf45591039b0a8d0dbb38115fac201ee
Author: Robert Phillips <robertphillips@google.com>
Date: Thu Oct 05 20:57:46 2017

[M62 cherry-pick] Temporarily disable deferred texture proxies

No-Tree-Checks: true
No-Try: true
No-Presubmit: true
Bug:  769760 
Change-Id: I1e9c1d7c33d63b46b4bbef7a9f2e6bc2a5e210bc
Reviewed-on: https://skia-review.googlesource.com/55762
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>

[modify] https://crrev.com/00cccf1cbf45591039b0a8d0dbb38115fac201ee/tests/ResourceAllocatorTest.cpp
[modify] https://crrev.com/00cccf1cbf45591039b0a8d0dbb38115fac201ee/src/gpu/GrSurfaceProxy.cpp

Labels: -Merge-Approved-62 Merge-Request-61
re the M61 merge request, this is perhaps as clear cut as a cherry pick can get and is very safe.
Cc: bsalomon@chromium.org
As of today (10/6), the fix can only be found in the M63 canaries (63.0.3231.0 and above). It has not yet made it into the dev or beta versions.
Labels: TE-Verified-M63 TE-Verified-63.0.3235.0
Verified this issue on chrome latest canary M63-63.0.3235.0 by following steps mentioned in the original comment, observed one red and one green canvases displayed as expected. Hence adding TE-Verified label for M-63.

Thanks!
769760.PNG
32.5 KB View Download
Labels: -Merge-Request-61 Merge-Rejected-61
Rejecting merge to M61 as we're not planning any further M61 releases for Desktop.
The initial fix for this bug is now in the 62.0.3202.47 (and above) Dev branch. Note that a more thorough fix for the related  crbug.com/769898  is still only in Canary builds.
Labels: -M-62 M-63
Project Member

Comment 37 by bugdroid1@chromium.org, Oct 17 2017

The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/7bbbf62d6ec14aea43e7dff7933bf9fb98575dca

commit 7bbbf62d6ec14aea43e7dff7933bf9fb98575dca
Author: Robert Phillips <robertphillips@google.com>
Date: Tue Oct 17 13:38:12 2017

Fix GrContext::writePixels bug

Bug:  769760 
Change-Id: I63603c036a8eef5eec66afb6ac4e937f556bbb63
Reviewed-on: https://skia-review.googlesource.com/59681
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>

[modify] https://crrev.com/7bbbf62d6ec14aea43e7dff7933bf9fb98575dca/tests/WritePixelsTest.cpp
[modify] https://crrev.com/7bbbf62d6ec14aea43e7dff7933bf9fb98575dca/src/gpu/GrContext.cpp

Tested this issue on Windows 7 using chrome latest Canary-64.0.3247.0 as per C#0.

Observed 2 blocks.1 block with Red color & 2nd block with Green color.

Please find the attached screen shot for reference.
Thanks..!!
769760.PNG
118 KB View Download
[Bulk Edit]
URGENT - PTAL.
M63 Stable promotion is coming soon and your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged into the release branch ASAP. Thank you.

Project Member

Comment 40 by bugdroid1@chromium.org, Oct 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a92fe336f8d823fa96c5916508303f10ce30629e

commit a92fe336f8d823fa96c5916508303f10ce30629e
Author: Robert Phillips <robertphillips@google.com>
Date: Tue Oct 31 20:36:30 2017

Reenable Skia's deferred proxies

Bug:  774090 ,  769898 ,  769760 
Change-Id: Id6ddfa82a2c16da476b2caaa93d5d3cd99aaedc8
Reviewed-on: https://chromium-review.googlesource.com/744683
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Salomon <bsalomon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512944}
[modify] https://crrev.com/a92fe336f8d823fa96c5916508303f10ce30629e/skia/config/SkUserConfig.h
[modify] https://crrev.com/a92fe336f8d823fa96c5916508303f10ce30629e/third_party/WebKit/LayoutTests/platform/mac/virtual/gpu-rasterization/images/color-profile-background-image-cross-fade-expected.png
[modify] https://crrev.com/a92fe336f8d823fa96c5916508303f10ce30629e/third_party/WebKit/LayoutTests/platform/mac/virtual/gpu-rasterization/images/color-profile-background-image-cross-fade-png-expected.png
[modify] https://crrev.com/a92fe336f8d823fa96c5916508303f10ce30629e/third_party/WebKit/LayoutTests/platform/mac/virtual/gpu-rasterization/images/color-profile-mask-image-svg-expected.png
[modify] https://crrev.com/a92fe336f8d823fa96c5916508303f10ce30629e/third_party/WebKit/LayoutTests/platform/win/virtual/gpu-rasterization/images/color-profile-background-image-cross-fade-expected.png
[modify] https://crrev.com/a92fe336f8d823fa96c5916508303f10ce30629e/third_party/WebKit/LayoutTests/platform/win/virtual/gpu-rasterization/images/color-profile-background-image-cross-fade-png-expected.png
[modify] https://crrev.com/a92fe336f8d823fa96c5916508303f10ce30629e/third_party/WebKit/LayoutTests/platform/win/virtual/gpu-rasterization/images/color-profile-mask-image-svg-expected.png

Status: Fixed (was: Started)
The fix in #40 landed in Chrome at r512944 and is in the 64.0.3255.0 Canary.

I have verified that this bug does not reappear even though the causal feature has been re-enabled.

Sign in to add a comment