| Summary: | Pipe initial scale factor to TileController | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Antti Koivisto <koivisto> | ||||||
| Component: | Layout and Rendering | Assignee: | Nobody <webkit-unassigned> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | commit-queue, eric.carlson, esprehn+autocc, glenn, jer.noble, kondapallykalyan, philipj, sergio, simon.fraser, thorton | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Antti Koivisto
2014-04-10 18:02:16 PDT
Created attachment 229097 [details]
patch
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. > 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?
What do you mean by timing issues? This is specifically done on layer commit to avoid timing issues. Created attachment 229143 [details]
renamed to zoomedOutScale
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; ? (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. |