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

Issue 807427 link

Starred by 6 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows , Mac
Pri: 2
Type: Bug-Regression



Sign in to add a comment

Graphical glitch: canvas has a smaller copy of itself overlayed

Reported by liud...@gmail.com, Jan 30 2018

Issue description

Chrome Version       : 64.0.3282.119
OS Version: OS X 10.12.6
URLs (if applicable) :
Other browsers tested:
  Add OK or FAIL after other browsers where you have tested this issue:
     Safari: OK
    Firefox: OK
    IE/Edge: OK

What steps will reproduce the problem?
1. Go to preview.scratch.mit.edu
2. Click "Try it"
3. Click the "Costumes" tab in the upper left
4. Click the dropdown next to Fill in the upper left
5. Click the eyedropper tool icon
6. Click anywhere on the cat to eyedrop a color
7. Move the mouse

Here is a GIF of the repro steps, starting from step 4.
https://user-images.githubusercontent.com/567844/35537979-46b72a2e-051a-11e8-876d-a84d7b6437e4.gif

What is the expected result?
Eyedropping shouldn't affect what the canvas looks like. If you continue to interact with the paint editor, everything works as expected.

What happens instead of that?
A smaller copy of the canvas appears over the larger canvas. The mouse events are calculated according to the coordinates on the larger canvas, but only the smaller canvas changes. The experience is really broken.

Please provide any additional information below. Attach a screenshot if
possible.

This reproes consistently. This only happens on Chrome 64 on high DPI devices.

According to bisect builds script:
You are probably looking for a change made after 518979 (known good), but no later than 518987 (first known bad).
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/4138b8862b533cba8a7d8efb5afe10f2bc90f52b..10824ac72d02c94083e3a585960e11e8e60adaf8

This bug was originally filed at: https://github.com/LLK/scratch-paint/issues/276
There might be more information in that issue.

UserAgentString: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/64.0.3282.119 Safari/537.36



 
Cc: xlai@chromium.org
Components: Blink>Canvas
Labels: -Type-Bug M-64 Type-Bug-Regression
Based on the bisect, I'm guessing it was this change:
https://chromium.googlesource.com/chromium/src/+/6c4cca504533b317b3b61e7417e4582a45f58881

+xlai
Cc: -xlai@chromium.org kbr@chromium.org gov...@chromium.org abdulsyed@chromium.org junov@chromium.org
Labels: -Pri-3 M-65 Pri-2
Owner: xlai@chromium.org
Status: Assigned (was: Unconfirmed)
Adding reviewers also to analyze the severity/impact on M64 stable launch.
Cc: ellyjo...@chromium.org

Comment 4 by gov...@chromium.org, Jan 31 2018

Cc: pbomm...@chromium.org

Comment 5 by xlai@chromium.org, Feb 1 2018

Labels: Needs-Feedback
Unable to reproduce this on non-DPI Mac machine on Version 64.0.3282.119.

I have very little information to dig into debugging. What kind of canvas API are called in the process of using the eyedrop tool? Could you reproduce this
in a smaller test case like in JSFiddle?
xlai: Could you find a high-DPI Mac to test with, or perhaps configure macOS to do a 2x scale somehow? Given the bug involves the canvas size getting halved, it's likely that this only reproduces when the scale factor is > 1.

Comment 7 by xlai@chromium.org, Feb 1 2018

davidben@: My machine could not scale 2x (scaling down resolution cannot reproduce the bug either) but I will find one high-DPI to reproduce it tonight. 

The point I tried to make in Comment 5 is actually about the lack of
more information that could aid in debugging--what canvas API are called, and in what sequence of calls, in using the eyedrop in this example? 

I can still try debugging based on the information about high-DPI difference but
if more information can be provided by the user that could help me speed up the debugging time.

Comment 8 by liud...@gmail.com, Feb 1 2018

The eyedropper is running the following code on mouse down:
https://github.com/LLK/scratch-paint/blob/develop/src/helper/tools/eye-dropper.js#L38

By turning on "paint flashing" in the render tool of Chrome devtools, you can see that the miniature copy of the canvas appears immediately after the next render of the canvas.

Comment 9 by liud...@gmail.com, Feb 2 2018

I've made a JS Fiddle at https://jsfiddle.net/ts8wvpza/6/
It reproduces the bug as soon as you mouse over the canvas. It seems to be caused by calling context.getImageData and then rerendering.

Comment 10 by xlai@chromium.org, Feb 2 2018

Labels: -Needs-Feedback
Status: Started (was: Assigned)
Thanks liudi08@gmail.com for coming up with the JS Fiddle. I'm looking into it.

Comment 11 by xlai@chromium.org, Feb 2 2018

I am pretty sure that this CL will fix the problem in a high-DPI mac machine:
https://chromium-review.googlesource.com/c/chromium/src/+/899962

I will try to land it today and merge it to M64 and M65 asap (M64 merge CL is ready,
waiting for review here: 
https://chromium-review.googlesource.com/c/chromium/src/+/899913).

Sorry for the trouble.
Project Member

Comment 12 by bugdroid1@chromium.org, Feb 2 2018

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

commit 07062c952f820196c880cea5bb05158881662d45
Author: xlai <xlai@chromium.org>
Date: Fri Feb 02 21:14:31 2018

Set CanvasResourceHost when canvas replaces its canvas2d layer bridge

TBR=junov@chromium.org

Bug:  807636 ,  807427 
Change-Id: Ic5fe5abbf5deaed883c005280fd8418cf49ca9d2
Reviewed-on: https://chromium-review.googlesource.com/899962
Commit-Queue: Olivia Lai <xlai@chromium.org>
Reviewed-by: Mohammad Reza Zakerinasab <zakerinasab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534152}
[modify] https://crrev.com/07062c952f820196c880cea5bb05158881662d45/third_party/WebKit/Source/core/html/canvas/HTMLCanvasElement.cpp
[modify] https://crrev.com/07062c952f820196c880cea5bb05158881662d45/third_party/WebKit/Source/modules/canvas/canvas2d/CanvasRenderingContext2DTest.cpp

Comment 13 by xlai@chromium.org, Feb 3 2018

Labels: Merge-Request-65 Merge-Request-64
Requesting merge fix to M64, M65 to reduce negative impact on user.
Project Member

Comment 14 by sheriffbot@chromium.org, Feb 3 2018

Labels: -Merge-Request-64 Hotlist-Merge-Review Merge-Review-64
This bug requires manual review: Request affecting a post-stable build
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
This just landed 4 hours ago, and doesn't have enough Canary coverage. Can you please explain why this is absolutely needed for M64? We're already rolling out M64 in stable. 

Labels: -Merge-Review-64 Merge-Rejected-64
As discussed over chat, xlai@ will propose client side workarounds. And we will target the fix for M65. 

Comment 17 by xlai@chromium.org, Feb 3 2018

liudi08@gmail.com:
There is a cost to merge a fix to stable release; so we
will merge to M65 which is currently in its Beta.

As a temporary workaround to this problem in Chrome 64, you could do this:
1. type "chrome://flags" in a Chrome tab and enter.
2. Among the list of Chrome flags, find "Accelerated 2D Canvas", disable this flag.
3. Relaunch browser.
Labels: OS-Windows
FWIW, I can also repro via the JSFiddle on a high-DPI Windows machine.

Is this the sort of thing we could get a layout test into Blink for future? (Do layout tests cover this case?) Looks like the unit test here wasn't quite of the final rendering and might get lost in a future refactor.
Project Member

Comment 19 by sheriffbot@chromium.org, Feb 4 2018

Labels: -Merge-Request-65 Hotlist-Merge-Approved Merge-Approved-65
Your change meets the bar and is auto-approved for M65. Please go ahead and merge the CL to branch 3325 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Pls merge your change to M65 branch 3325 before 2:00 PM PT tomorrow, Monday (02/25/18) so we can pick it up for last dev release on Tuesday. Thank you.
Project Member

Comment 21 by bugdroid1@chromium.org, Feb 4 2018

Labels: -merge-approved-65 merge-merged-3325
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/160b0172e48798827282c024d0e61c8abbf5edaa

commit 160b0172e48798827282c024d0e61c8abbf5edaa
Author: xlai <xlai@chromium.org>
Date: Sun Feb 04 22:36:47 2018

Set CanvasResourceHost when canvas replaces its canvas2d layer bridge


Bug:  807636 ,  807427 
Change-Id: Ic5fe5abbf5deaed883c005280fd8418cf49ca9d2
Reviewed-on: https://chromium-review.googlesource.com/899962
Commit-Queue: Olivia Lai <xlai@chromium.org>
Reviewed-by: Mohammad Reza Zakerinasab <zakerinasab@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#534152}
Reviewed-on: https://chromium-review.googlesource.com/900455
Reviewed-by: Olivia Lai <xlai@chromium.org>
Cr-Commit-Position: refs/branch-heads/3325@{#285}
Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369}
[modify] https://crrev.com/160b0172e48798827282c024d0e61c8abbf5edaa/third_party/WebKit/Source/core/html/canvas/HTMLCanvasElement.cpp

Labels: TE-Verified-M65 TE-Verified-65.0.3325.51
Verified the fix on Mac 10.12.6(retina) and Win-10 using Chrome dev version #65.0.3325.51 as per the comment #9.
Attaching screen cast for reference.
Observed that eyedropping did not affect what the canvas looks like.
Hence, the fix is working as expected. 
Adding the verified labels.

Thanks...!!
807427.mp4
1.6 MB View Download
Project Member

Comment 23 by bugdroid1@chromium.org, Feb 6 2018

Labels: merge-merged-3282
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/68f180c2597177c389641868a36c8c4e5208aa3e

commit 68f180c2597177c389641868a36c8c4e5208aa3e
Author: xlai <xlai@chromium.org>
Date: Tue Feb 06 18:05:24 2018

Set CanvasResourceHost when canvas replaces its canvas2d layer bridge

This CL is a hotfix to M64 branch based on
https://chromium-review.googlesource.com/c/chromium/src/+/899962. Due to code
structural change between M64 and M66, it is a bit different from the original
CL.

Bug:  807636 ,  807427 
Change-Id: I7f5826187e3deef2ceaeee796dc7fdf3baf71717
Reviewed-on: https://chromium-review.googlesource.com/899913
Reviewed-by: Justin Novosad <junov@chromium.org>
Cr-Commit-Position: refs/branch-heads/3282@{#652}
Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840}
[modify] https://crrev.com/68f180c2597177c389641868a36c8c4e5208aa3e/third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp

 Issue 809403  has been merged into this issue.

Comment 25 by xlai@chromium.org, Feb 6 2018

We have landed the fix patch, which is the same fix to https://bugs.chromium.org/p/chromium/issues/detail?id=807636, into Chrome 64.

Once the latest release with this fix goes out in the coming week, we will update this issue again.

Comment 26 by xlai@chromium.org, Feb 12 2018

I'm marking this as fixed because the fix CL is already waiting in the next update of Chrome 64. But I'll continue verifying it after the update is released.

Comment 27 by xlai@chromium.org, Feb 12 2018

Status: Fixed (was: Started)
Labels: TE-Verified-M64 TE-Verified-64.0.3282.167
Verified the fix on Mac 10.12.6(retina) and Win-10 using Chrome version #64.0.3282.167 as per the comment #9.
Attaching screen cast for reference.
Observed that eyedropping did not affect what the canvas looks like.
Hence, the fix is working as expected. 
Adding the verified labels.

Thanks...!!
807427@M64.mp4
2.2 MB View Download

Sign in to add a comment