Bug 92569 - Ignore visibility:hidden elements when computing compositing layer bounds
Summary: Ignore visibility:hidden elements when computing compositing layer bounds
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: InRadar
Depends on:
Blocks:
 
Reported: 2012-07-27 21:55 PDT by Simon Fraser (smfr)
Modified: 2012-07-28 14:39 PDT (History)
10 users (show)

See Also:


Attachments
Patch (6.04 KB, patch)
2012-07-27 22:24 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (15.83 KB, patch)
2012-07-28 11:32 PDT, Simon Fraser (smfr)
mitz: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from gce-cr-linux-01 (597.62 KB, application/zip)
2012-07-28 12:22 PDT, WebKit Review Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2012-07-27 21:55:56 PDT
Google maps has some elements with crazy offsets:

<img style="position: absolute; left: 44940px; top: 15013px; width: 19px; height: 19px; -webkit-user-select: none; border: 0px; padding: 0px; margin: 0px; z-index: 0; visibility: hidden; " crossorigin="" src="//maps.gstatic.com/mapfiles/markers2/dd-via-transparent.png" class="gmnoprint" log="miw" id="mtgt_ddwnew">

<img style="width: 11px; height: 11px; -webkit-user-select: none; border: 0px; padding: 0px; margin: 0px; position: absolute; left: 44944px; top: 15017px; z-index: 0; visibility: hidden; " crossorigin="" src="//maps.gstatic.com/mapfiles/markers2/dd-via.png">

These cause us to create crazy-big layers when getting directions (at least on iOS).We should ignore visibility:hidden layers when computing composited layer bounds.
Comment 1 Simon Fraser (smfr) 2012-07-27 22:24:03 PDT
Created attachment 155109 [details]
Patch
Comment 2 Simon Fraser (smfr) 2012-07-27 22:25:15 PDT
I'll add a test later.
Comment 3 Dean Jackson 2012-07-28 05:25:47 PDT
Comment on attachment 155109 [details]
Patch

Did you mean to "r?" this patch, or will you upload another? Otherwise all looks good.
Comment 4 Simon Fraser (smfr) 2012-07-28 08:28:11 PDT
Comment on attachment 155109 [details]
Patch

I'll do one with some tests.
Comment 5 Simon Fraser (smfr) 2012-07-28 10:55:00 PDT
Filed bug 92579 about filter extents.
Comment 6 Simon Fraser (smfr) 2012-07-28 11:32:17 PDT
Created attachment 155136 [details]
Patch
Comment 7 mitz 2012-07-28 11:39:46 PDT
Comment on attachment 155136 [details]
Patch

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

> Source/WebCore/rendering/RenderLayer.cpp:4251
> +    // FIXME: should probably just pass 'flags' down to descendants. This is a defensive change.

“This is a defensive change” is an appropriate comment for the change log. It doesn’t make much sense to someone reading the code (what is the change?).

> Source/WebCore/rendering/RenderLayer.cpp:4254
> +    CalculateLayerBoundsFlags descendantFlags = DefaultCalculateLayerBoundsFlags;
> +    if (flags & ExcludeHiddenDescendants)
> +        descendantFlags |= ExcludeHiddenDescendants;

There are many ways to write this. I’d go for the one-liner
    CalculateLayerBoundsFlags descendantFlags = DefaultCalculateLayerBoundsFlags | (flags & ExcludeHiddenDescendants);
Comment 8 WebKit Review Bot 2012-07-28 12:22:14 PDT
Comment on attachment 155136 [details]
Patch

Attachment 155136 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13378886

New failing tests:
compositing/geometry/bounds-ignores-hidden-composited-descendant.html
compositing/geometry/bounds-ignores-hidden-dynamic.html
Comment 9 WebKit Review Bot 2012-07-28 12:22:18 PDT
Created attachment 155137 [details]
Archive of layout-test-results from gce-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-01  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Comment 10 Simon Fraser (smfr) 2012-07-28 12:37:29 PDT
Chromium linux needs to stop making layers 1px bigger on all sides.
Comment 11 Simon Fraser (smfr) 2012-07-28 12:44:52 PDT
http://trac.webkit.org/changeset/123971
Comment 12 Radar WebKit Bug Importer 2012-07-28 14:39:01 PDT
<rdar://problem/11981260>