New issue
Advanced search Search tips

Issue 865415 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 25
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Image is not appearing if overlapped by other image after transform animation

Reported by diiiima...@gmail.com, Jul 19

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/69.0.3495.0 Safari/537.36

Steps to reproduce the problem:
Issue on Chrome Canary 69.0.3495.0. Mac.

To reproduce:

1. Visit photoswipe.com
2. Click an image to open gallery
3. Larger image will be invisible after it's loaded.

How PhotoSwipe loads/adds image:

1. Stretches thumbnail to the size of the large image.
2. Animates transform:scale of its parent container. 
3. After animation is finished - large image is appended on top of stretched thumbnail.
4. After large image is fully loaded, stretched thumbnail is hidden.

(on steps #3 and #4 large image is not visible in Canary, but visible in Stable and other browsers)

Adding will-change:transform to the "large" image fixes the issue, but I want to avoid this, since it disables progressive jpeg loading and it makes Safari render maximum quality.

Permanently hiding "stretched thumbnail" image just via "display:none" also fixes the issue. Were there any hitstate/overlap optimizations in v69?

What is the expected behavior?

What went wrong?
.

Did this work before? Yes 67.0.3396.99

Does this work in other browsers? Yes

Chrome version: 69.0.3495.0  Channel: n/a
OS Version: OS X 10.13.0
Flash Version:
 
Components: -Blink>Image Blink>Compositing
Labels: -Pri-2 RegressedIn-69 ReleaseBlock-Beta Target-69 FoundIn-69 Pri-1
Owner: chrishtr@chromium.org
Status: Assigned (was: Unconfirmed)
diiiimaaaa@, if at all possible could you produce a reduced test case for us? IN particular, you seem to have some insight into the overlap conditions necessary to reproduce.

Bisects to https://chromium.googlesource.com/chromium/src/+/a9be49beca9893711aad3ffac2e00132d38c3bf8 "Optimize out work in CompositingLayerAssigner"

Note that it doesn't always go wrong. It seems to require that the animation finish before the image is fully decoded; that is, before the progressive jpeg has fully loaded. When the progressive loading completes, the image disappears.

If I had to guess I would say that the completion of the progressive load reloads and changes the compositing status of the image but the compositor doesn't update the layer assignment.
Here is the reduced test case: https://jsfiddle.net/Ln5mx2v4/

Click once anywhere to load the larger image on top of the thumbnail.
Project Member

Comment 3 by sheriffbot@chromium.org, Jul 23

Cc: schenney@chromium.org
This issue is marked as a release blocker with no milestone associated. Please add an appropriate milestone.

All release blocking issues should have milestones associated to it, so that the issue can tracked and the fixes can be pushed promptly.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: M-69
Added M-69 as the merge target label. If it seems like a beta merge might be impossible we can revise the plan.
Labels: -ReleaseBlock-Beta ReleaseBlock-Stable
Fix is in the CQ.
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 24

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

commit afd0623808c7cb2cb2f0f4e4fbf470f7bae2b2e5
Author: Chris Harrelson <chrishtr@chromium.org>
Date: Tue Jul 24 01:53:44 2018

Properly invalidate backings of squashed content when it loses its squashing layer.

Previously, in such situations we failed to mark the layer as needing layer requirements
update, which already has code to issue an invalidation on the new backing of the content
now that it lost squashing.

Bug:  865415 

Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I807bf1426e45ce71ed42024512e9c77481f018cc
Reviewed-on: https://chromium-review.googlesource.com/1145896
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Tien-Ren Chen <trchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577404}
[modify] https://crrev.com/afd0623808c7cb2cb2f0f4e4fbf470f7bae2b2e5/third_party/blink/renderer/core/paint/compositing/compositing_requirements_updater.cc
[modify] https://crrev.com/afd0623808c7cb2cb2f0f4e4fbf470f7bae2b2e5/third_party/blink/renderer/core/paint/compositing/compositing_requirements_updater_test.cc

Labels: Merge-Request-69
Project Member

Comment 9 by sheriffbot@chromium.org, Jul 25

Labels: -Merge-Request-69 Hotlist-Merge-Approved Merge-Approved-69
Your change meets the bar and is auto-approved for M69. Please go ahead and merge the CL to branch 3497 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 10 by bugdroid1@chromium.org, Jul 25

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3e92c6f571e5735a550ceca858d3c7f101d1a261

commit 3e92c6f571e5735a550ceca858d3c7f101d1a261
Author: Chris Harrelson <chrishtr@chromium.org>
Date: Wed Jul 25 05:31:56 2018

Properly invalidate backings of squashed content when it loses its squashing layer.

Previously, in such situations we failed to mark the layer as needing layer requirements
update, which already has code to issue an invalidation on the new backing of the content
now that it lost squashing.

Bug:  865415 

TBR=chrishtr@chromium.org

(cherry picked from commit afd0623808c7cb2cb2f0f4e4fbf470f7bae2b2e5)

Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I807bf1426e45ce71ed42024512e9c77481f018cc
Reviewed-on: https://chromium-review.googlesource.com/1145896
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Tien-Ren Chen <trchen@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#577404}
Reviewed-on: https://chromium-review.googlesource.com/1149615
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#62}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/3e92c6f571e5735a550ceca858d3c7f101d1a261/third_party/blink/renderer/core/paint/compositing/compositing_requirements_updater.cc
[modify] https://crrev.com/3e92c6f571e5735a550ceca858d3c7f101d1a261/third_party/blink/renderer/core/paint/compositing/compositing_requirements_updater_test.cc

Status: Fixed (was: Assigned)

Sign in to add a comment