Bug 131586

Summary: Keep secondary tile grid for zoomed-out scale
Product: WebKit Reporter: Antti Koivisto <koivisto>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: simon.fraser, thorton
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch darin: review+

Description Antti Koivisto 2014-04-12 22:21:51 PDT
Better zooming
Comment 1 Antti Koivisto 2014-04-12 22:45:52 PDT
Created attachment 229217 [details]
patch
Comment 2 Antti Koivisto 2014-04-12 22:48:04 PDT
<rdar://problem/16383851>
Comment 3 Darin Adler 2014-04-13 23:25:24 PDT
Comment on attachment 229217 [details]
patch

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

> Source/WebCore/platform/graphics/ca/mac/TileController.mm:520
> +        count +=  m_zoomedOutTileGrid->numberOfUnparentedTiles();

extra space here after "+="
Comment 4 Tim Horton 2014-04-14 11:27:44 PDT
Comment on attachment 229217 [details]
patch

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

> Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:2999
> +        if (m_isPageTiledBackingLayer)
> +            m_uncommittedChanges |= ChildrenChanged;

Do we need to do this always? Is it bad if we do?
Comment 5 Antti Koivisto 2014-04-14 11:31:37 PDT
> Do we need to do this always? Is it bad if we do?

We don't but I don't think it ends up doing significant amount of actual work if nothing changes (it just re-adds the layers that are already there).
Comment 6 Antti Koivisto 2014-04-14 11:40:48 PDT
https://trac.webkit.org/r167256
Comment 7 Simon Fraser (smfr) 2014-04-14 12:20:52 PDT
Comment on attachment 229217 [details]
patch

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

>> Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:2999
>> +            m_uncommittedChanges |= ChildrenChanged;
> 
> Do we need to do this always? Is it bad if we do?

Yeah, I would prefer that we don't always do this. It will result in extra remote layer tree transaction thrash.

tiledBacking() should have a:
bool contentsScaleChangeRequiresSublayerUpdate(float) or something.

> Source/WebCore/platform/graphics/ca/mac/TileController.mm:135
> +    return tileGrid().scale() * m_deviceScaleFactor;

I hope the scale /= deviceScaleFactor; and this math don't result in floating-point rounding issues that cause us to compare the contentsScale as different somewhere.

> Source/WebCore/platform/graphics/ca/mac/TileGrid.h:-62
> -    typedef unsigned TileValidationPolicyFlags;

Why did you remove this? I prefer strongly typed bitfield types.

> Source/WebCore/platform/graphics/ca/mac/TileGrid.mm:340
> +void TileGrid::revalidateTiles(unsigned validationPolicy)

:(