Bug 131586 - Keep secondary tile grid for zoomed-out scale
Summary: Keep secondary tile grid for zoomed-out scale
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: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-04-12 22:21 PDT by Antti Koivisto
Modified: 2014-04-14 12:20 PDT (History)
2 users (show)

See Also:


Attachments
patch (15.20 KB, patch)
2014-04-12 22:45 PDT, Antti Koivisto
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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)

:(