Bug 65030 - Avoiding painting backgrounds if they are fully obscures by an object's foreground
Summary: Avoiding painting backgrounds if they are fully obscures by an object's foreg...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-22 10:20 PDT by Simon Fraser (smfr)
Modified: 2012-05-14 11:10 PDT (History)
5 users (show)

See Also:


Attachments
Patch (11.90 KB, patch)
2011-07-22 15:13 PDT, Simon Fraser (smfr)
mitz: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2011-07-22 10:20:33 PDT
We shouldn't waste time painting backgrounds for things like images, for which it's easy to determine that the foreground fully obscures that background. This helps particularly in cases where the background might contain an animated "loading" image; that currently causes continual repaints, and it should not.
Comment 1 Simon Fraser (smfr) 2011-07-22 10:56:53 PDT
This is only going to work for JPEG initially: bug 65033.
Comment 2 Simon Fraser (smfr) 2011-07-22 15:13:23 PDT
Created attachment 101774 [details]
Patch
Comment 3 mitz 2011-07-22 17:02:42 PDT
Comment on attachment 101774 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=101774&action=review

> Source/WebCore/ChangeLog:8
> +        Some pages used animated loading GIFs as the background of <img>,

Confused by the use of “used” in the past tense.

> Source/WebCore/rendering/RenderImage.cpp:411
> +    if (!image->isBitmapImage())

I think you need to null-check image.
Comment 4 Simon Fraser (smfr) 2011-07-22 17:40:45 PDT
http://trac.webkit.org/changeset/91628
Comment 5 Ryosuke Niwa 2011-07-22 18:52:33 PDT
This patch seems to have caused a crash on Chromium Linux:
http://build.webkit.org/results/Chromium%20Linux%20Release%20(Tests)/r91628%20(21410)/results.html
http://build.webkit.org/results/Chromium%20Linux%20Release%20(Flexbox)/r91628%20(1508)/results.html

Given these are two different bots, I highly doubt that this is a flake.  Unfortunately, these are release builds so we can't get stack trace out of them.

There's also one failing test on Chromium Mac:
http://build.webkit.org/results/Chromium%20Mac%20Release%20(Tests)/r91628%20(9716)/results.html

But I'm not certain if this is a real regression or just a flakiness.
Comment 6 Simon Fraser (smfr) 2011-07-29 16:02:22 PDT
Do we have any more information on whether those crashes persisted?
Comment 7 Ryosuke Niwa 2011-07-29 16:40:19 PDT
(In reply to comment #6)
> Do we have any more information on whether those crashes persisted?

They have been crashing quite consistently: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Fcanvas%2Fwebgl%2Fdrawingbuffer-test.html%2Cfast%2Fcanvas%2Fcanvas-bg-multiple-removal.html%2Cfast%2Fcanvas%2Fcanvas-as-image-incremental-repaint.html
Comment 8 Simon Fraser (smfr) 2011-07-30 08:56:25 PDT
Odd that it's just Linux that's crashing.
Comment 9 Ryosuke Niwa 2011-07-30 10:16:50 PDT
(In reply to comment #8)
> Odd that it's just Linux that's crashing.

It's possible that there's a cr-linux or skia bug; or that only cr-linux hit some specific timing.
Comment 10 Simon Fraser (smfr) 2011-08-01 10:36:09 PDT
Let's continue discussing the crashes in bug 65063.