Bug 131521 – Pipe initial scale factor to TileController

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Antti Koivisto]

We need it to support caching multiple scales.

[Antti Koivisto]

Created attachment 229097 [details]
patch

[Simon Fraser (smfr)]

Comment on attachment 229097 [details]
patch

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

> Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:2989
> +    if (tiledBacking()) {
> +        float initialScale = (m_client ? m_client->initialPageScaleFactor() : 1) * deviceScaleFactor();
> +        tiledBacking()->setInitialContentsScale(initialScale);
> +    }

This seems like an odd way to get initial scale, and could easily have timing issues. I think we should explicitly get the initial scale from viewport-related information.

[Antti Koivisto]

> This seems like an odd way to get initial scale, and could easily have timing issues. I think we should explicitly get the initial scale from viewport-related information.

It is done the same way as the content scale is passed and is fully analogous. Are you a suggesting we refactor that too?

[Antti Koivisto]

What do you mean by timing issues? This is specifically done on layer commit to avoid timing issues.

[Antti Koivisto]

Created attachment 229143 [details]
renamed to zoomedOutScale

[Tim Horton]

Comment on attachment 229143 [details]
renamed to zoomedOutScale

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

> Source/WebCore/platform/graphics/GraphicsLayerClient.h:89
> +    virtual float zoomedOutPageScaleFactor() const { return 0; }

Would getting a zero cause trouble anywhere? (divide-by-zero, etc.)

> Source/WebCore/platform/graphics/TiledBacking.h:92
> +    virtual void setZoomedOutContentsScale(float) = 0;
> +    virtual float zoomedOutContentsScale() const = 0;

Every 'push' property we've had on TileController has caused trouble (and we have since made many 'pull' via PlatformCALayerClient), but I think this one is OK because it's called from updateContentsScale. That said, I would not be opposed to inverting it.

> Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:75
> +typedef int AVPlayerViewControllerExitFullScreenReason;

?

[Antti Koivisto]

(In reply to comment #6)
> (From update of attachment 229143 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=229143&action=review
> 
> > Source/WebCore/platform/graphics/GraphicsLayerClient.h:89
> > +    virtual float zoomedOutPageScaleFactor() const { return 0; }
> 
> Would getting a zero cause trouble anywhere? (divide-by-zero, etc.)

The plan is to use it to mean "disabled". Might change it later.

> Every 'push' property we've had on TileController has caused trouble (and we have since made many 'pull' via PlatformCALayerClient), but I think this one is OK because it's called from updateContentsScale. That said, I would not be opposed to inverting it.

Yeah, push is right thing to do for things that go via layer commits

> > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:75
> > +typedef int AVPlayerViewControllerExitFullScreenReason;
> 
> ?

Oops, build fix.

[Antti Koivisto]

https://trac.webkit.org/r167138