[webkit-reviews] review granted: [Bug 123279] Make TileController manipulate PlatformCALayers instead of CALayers : [Attachment 215221] missed one fixme (removing stale repaint counts)

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Oct 28 11:37:24 PDT 2013


Simon Fraser (smfr) <simon.fraser at apple.com> has granted Tim Horton
<thorton at apple.com>'s request for review:
Bug 123279: Make TileController manipulate PlatformCALayers instead of CALayers
https://bugs.webkit.org/show_bug.cgi?id=123279

Attachment 215221: missed one fixme (removing stale repaint counts)
https://bugs.webkit.org/attachment.cgi?id=215221&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=215221&action=review


> Source/WebCore/platform/graphics/ca/mac/TileController.h:142
> +    virtual void platformCALayerLayoutSublayersOfLayer(PlatformCALayer*)
OVERRIDE { }
> +    virtual bool platformCALayerRespondsToLayoutChanges() const OVERRIDE {
return false; }
> +    virtual void platformCALayerAnimationStarted(CFTimeInterval) OVERRIDE {
}
> +    virtual GraphicsLayer::CompositingCoordinatesOrientation
platformCALayerContentsOrientation() const OVERRIDE { return
GraphicsLayer::CompositingCoordinatesTopDown; }
> +    virtual void platformCALayerPaintContents(PlatformCALayer*,
GraphicsContext&, const IntRect&) OVERRIDE;
> +    virtual bool platformCALayerShowDebugBorders() const OVERRIDE;
> +    virtual bool platformCALayerShowRepaintCounter(PlatformCALayer*) const
OVERRIDE;
> +    virtual int platformCALayerIncrementRepaintCount(PlatformCALayer*)
OVERRIDE;
> +
> +    virtual bool platformCALayerContentsOpaque() const OVERRIDE { return
m_tilesAreOpaque; }
> +    virtual bool platformCALayerDrawsContent() const OVERRIDE { return true;
}
> +    virtual void platformCALayerLayerDidDisplay(PlatformLayer*) OVERRIDE { }

> +
> +    virtual void platformCALayerSetNeedsToRevalidateTiles() OVERRIDE { }
> +    virtual float platformCALayerDeviceScaleFactor() OVERRIDE;

It's weird that these client functions don't all take a PlatformCALayer*

Not sure why platformCALayerDeviceScaleFactor isnt' const.

> Source/WebCore/platform/graphics/ca/mac/TileController.mm:-120
> -    [CATransaction commit];

You should explain why it's OK to remove the transaction.

> Source/WebCore/platform/graphics/ca/mac/TileController.mm:193
> +    return m_tileCacheLayer->owner()->platformCALayerDeviceScaleFactor();

These redirections are pretty confusing.


More information about the webkit-reviews mailing list