Bug 86886 - [chromium] Don't force the visibleLayerRect to be empty for animating layers whose front face is not visible
Summary: [chromium] Don't force the visibleLayerRect to be empty for animating layers ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dana Jansens
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-18 12:51 PDT by Dana Jansens
Modified: 2012-05-22 18:01 PDT (History)
7 users (show)

See Also:


Attachments
Patch (5.53 KB, patch)
2012-05-18 12:58 PDT, Dana Jansens
no flags Details | Formatted Diff | Diff
Patch (4.65 KB, patch)
2012-05-22 15:22 PDT, Dana Jansens
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dana Jansens 2012-05-18 12:51:11 PDT
[chromium] Don't force the visibleLayerRect to be empty for animating layers whose front face is not visible
Comment 1 Dana Jansens 2012-05-18 12:57:48 PDT
Some context discussion on https://bugs.webkit.org/show_bug.cgi?id=82571
Comment 2 Dana Jansens 2012-05-18 12:58:02 PDT
Created attachment 142770 [details]
Patch
Comment 3 Adrienne Walker 2012-05-21 09:57:11 PDT
Comment on attachment 142770 [details]
Patch

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

Hmm.  This assumes that the current visible rect (flipped) is a good proxy for the final visible rect when the animation is done.  There are certainly edge cases where that's not true, but I guess this is just as much as heuristic as "paint everything only if it's a small layer".  I think overall that this is an improvement, and I'm happy to get rid of the empty IntRect special case.

> Source/WebKit/chromium/tests/CCLayerTreeHostCommonTest.cpp:-1325
> -
> -    // But if the back face is visible, then the visibleLayerRect should be empty.
> -    EXPECT_TRUE(animatingChild->visibleLayerRect().isEmpty());
> -    EXPECT_TRUE(animatingSurface->visibleLayerRect().isEmpty());
> -    // And any layers in the subtree should not be considered visible either.
> -    EXPECT_TRUE(childOfAnimatingSurface->visibleLayerRect().isEmpty());

Can you verify that the visible layer rect for these animating layers is the whole layer (assuming it's not clipped)?
Comment 4 Dana Jansens 2012-05-22 15:20:28 PDT
Comment on attachment 142770 [details]
Patch

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

>> Source/WebKit/chromium/tests/CCLayerTreeHostCommonTest.cpp:-1325
>> -    EXPECT_TRUE(childOfAnimatingSurface->visibleLayerRect().isEmpty());
> 
> Can you verify that the visible layer rect for these animating layers is the whole layer (assuming it's not clipped)?

yup!
Comment 5 Dana Jansens 2012-05-22 15:22:48 PDT
Created attachment 143376 [details]
Patch

I removed the change in the prepainting code. While the flipped case we should now have a non-empty rect, for animating layers that are not flipped but transitioning into the viewport from outside, we should still prepaint them when they are small.
Comment 6 Adrienne Walker 2012-05-22 17:20:56 PDT
Comment on attachment 143376 [details]
Patch

R=me.  Leaving the prepainting code makes sense.
Comment 7 WebKit Review Bot 2012-05-22 18:01:35 PDT
Comment on attachment 143376 [details]
Patch

Clearing flags on attachment: 143376

Committed r118090: <http://trac.webkit.org/changeset/118090>
Comment 8 WebKit Review Bot 2012-05-22 18:01:43 PDT
All reviewed patches have been landed.  Closing bug.