New issue
Advanced search Search tips

Issue 918752 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 18
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 836890



Sign in to add a comment

[BlinkGenPropertyTrees] Visual viewport constantly update paint properties

Project Member Reported by wangxianzhu@chromium.org, Jan 3

Issue description

For the following test case (from  crbug.com/918276 ):

<img src="https://placeholder.pics/svg/300" />
<img style="height: 30vh" src="https://placeholder.pics/svg/300" />

run
content_shell test-svg-image.html --enable-blink-features=BlinkGenPropertyTrees --trace-to-console=blink

we'll see endless traces containing the following:

VisualViewport::mainFrameDidChangeSize[blink]
VisualViewport::setSize[blink], {width:180, height:180}
VisualViewport::mainFrameDidChangeSize[blink]
VisualViewport::setSize[blink], {width:300, height:300}
...

VisualViewport::UpdatePaintProertyNodesIfNeeded() is also called constantly with needs_paint_property_update_ == true.

Assigning to bokan@ because the related code in svg_image.cc was added in https://chromium.googlesource.com/chromium/src/+/72648da9df0395c2b753ed85964b34b061bde37f. 


 


 
Status: Assigned (was: Untriaged)
Blocking: 836890

Comment 3 by pdr@chromium.org, Jan 17 (5 days ago)

Cc: wangxianzhu@chromium.org
Hi Xianzhu, do you have time to look at this while David is OOO?

Comment 4 by wangxianzhu@chromium.org, Jan 17 (5 days ago)

Cc: -wangxianzhu@chromium.org bokan@chromium.org
Owner: wangxianzhu@chromium.org
Sure.

Comment 5 by wangxianzhu@chromium.org, Jan 17 (5 days ago)

It's actually caused by my code in SVGImage::ServiceAnimations().
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 18 (4 days ago)

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

commit f55ea7a02f3aa08ce57225d658d4e94c081247ba
Author: Xianzhu Wang <wangxianzhu@chromium.org>
Date: Fri Jan 18 11:27:42 2019

[BlinkGenPropertyTrees] Avoid visual update loop of two svg images

When two images referencing the same SVGImage, and they have different
sizes, previously setting the different viewport size for the same
SVGImage schedules visual update and the next frame update still do
the same causing a loop.

Now remove the ImageObserver::Changed() call for BGPT/CAP. The call
has been unnecessary since crrev.com/c/1393558, and was unnecessary
for BGPT even before the CL.

Bug:  918752 
Change-Id: I13ca36a7d41143d0e639a880ebba37868b31ac49
Reviewed-on: https://chromium-review.googlesource.com/c/1419498
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624076}
[modify] https://crrev.com/f55ea7a02f3aa08ce57225d658d4e94c081247ba/third_party/blink/renderer/core/paint/pre_paint_tree_walk.cc
[modify] https://crrev.com/f55ea7a02f3aa08ce57225d658d4e94c081247ba/third_party/blink/renderer/core/svg/graphics/svg_image.cc
[modify] https://crrev.com/f55ea7a02f3aa08ce57225d658d4e94c081247ba/third_party/blink/renderer/core/svg/graphics/svg_image_chrome_client.h
[modify] https://crrev.com/f55ea7a02f3aa08ce57225d658d4e94c081247ba/third_party/blink/renderer/core/svg/graphics/svg_image_test.cc

Comment 7 by wangxianzhu@chromium.org, Jan 18 (4 days ago)

Status: Fixed (was: Assigned)

Comment 8 by bokan@chromium.org, Jan 18 (4 days ago)

Thank you Xianzhu!

Sign in to add a comment