Issue metadata
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.
,
Sep 28 2017
Was able to repro on Win10, not on linux.
,
Sep 28 2017
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
,
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.
,
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
,
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;
,
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?
,
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?
,
Sep 29 2017
,
Sep 29 2017
I've reproduced the bug locally and have begun trying to figure out what is going wrong.
,
Oct 1 2017
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?
,
Oct 2 2017
The following revision refers to this bug: https://skia.googlesource.com/skia/+/73f7e1dbdd2921343262b1a57f74a02700edef68 commit 73f7e1dbdd2921343262b1a57f74a02700edef68 Author: Robert Phillips <robertphillips@google.com> Date: Mon Oct 02 18:25:44 2017 Temporarily disable deferred texture proxies Bug: 769760 Change-Id: Id4197a73737f3a8df9159d9eb081be094df1f731 Reviewed-on: https://skia-review.googlesource.com/54004 Commit-Queue: Robert Phillips <robertphillips@google.com> Reviewed-by: Greg Daniel <egdaniel@google.com> [modify] https://crrev.com/73f7e1dbdd2921343262b1a57f74a02700edef68/tests/ResourceAllocatorTest.cpp [modify] https://crrev.com/73f7e1dbdd2921343262b1a57f74a02700edef68/src/gpu/GrSurfaceProxy.cpp [modify] https://crrev.com/73f7e1dbdd2921343262b1a57f74a02700edef68/tests/TextureProxyTest.cpp
,
Oct 2 2017
,
Oct 2 2017
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.
,
Oct 2 2017
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.
,
Oct 3 2017
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.
,
Oct 3 2017
,
Oct 3 2017
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
,
Oct 3 2017
Issue 770214 has been merged into this issue.
,
Oct 3 2017
Thanks robertphillips@. Is there a regression test somewhere to prevent a bug like this from happening again?
,
Oct 3 2017
,
Oct 3 2017
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.
,
Oct 3 2017
Thanks for the fix. Let's wait until this is tested in Windows Canary, and I'll re-review merge.
,
Oct 4 2017
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.
,
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.
,
Oct 4 2017
Thanks for the fix robertphillips@! Approving this merge for M62. Branch:3202
,
Oct 5 2017
robertphillips@, Merge request for M62 has been approved. Can you please do the merge ASAP? Thank you!
,
Oct 5 2017
https://skia-review.googlesource.com/c/skia/+/55762 ([M62 cherry-pick] Temporarily disable deferred texture proxies)
,
Oct 5 2017
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
,
Oct 5 2017
re the M61 merge request, this is perhaps as clear cut as a cherry pick can get and is very safe.
,
Oct 6 2017
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.
,
Oct 9 2017
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!
,
Oct 9 2017
Rejecting merge to M61 as we're not planning any further M61 releases for Desktop.
,
Oct 12 2017
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.
,
Oct 16 2017
,
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
,
Oct 24 2017
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..!!
,
Oct 30 2017
[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.
,
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
,
Nov 1 2017
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 |
|||||||||||||||||||||||
Comment 1 by ligim...@chromium.org
, Sep 28 2017