Bug 107802

Created attachment 184436 [details] Patch

Comment on attachment 184436 [details] Patch Attachment 184436 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/16067789

Currently, only DrawingAreaImpl::updateBackingStoreState calls LayerTreeHost::deviceOrPageScaleFactorChanged. void DrawingAreaImpl::updateBackingStoreState(uint64_t stateID, bool respondImmediately, float deviceScaleFactor, const WebCore::IntSize& size, const WebCore::IntSize& scrollOffset) { ... if (m_layerTreeHost) { m_layerTreeHost->deviceOrPageScaleFactorChanged(); ... } I think this patch is covered by geometry/fixed-position-composited-page-scale-scroll.html geometry/fixed-position-composited-page-scale-smaller-than-viewport.html geometry/fixed-position-iframe-composited-page-scale.html geometry/fixed-position-transform-composited-page-scale-down.html geometry/fixed-position-composited-page-scale-down.html geometry/fixed-position-composited-page-scale-smaller-than-viewport-expected.html geometry/fixed-position-transform-composited-page-scale.html geometry/fixed-position-iframe-composited-page-scale-down.html geometry/fixed-position-composited-page-scale.html layer-creation/fixed-position-out-of-view-scaled-iframe.html layer-creation/fixed-position-out-of-view-scaled-iframe-scroll.html layer-creation/fixed-position-out-of-view-scaled-scroll.html layer-creation/fixed-position-out-of-view-scaled.html overflow/overflow-scaled-descendant-overlapping.html repaint/page-scale-repaint.html scaling/tiled-layer-recursion.html Gtk WK2 does not perform pixel test yet, and Gtk does not use page scale factor in WebCore. So I can not verify this patch in Gtk. I wonder how mac WK2 passes above pixel test until now. I guess even if mac WK2 enables AC, DrawingAreaImpl::updateBackingStoreState is called. I need feedback.

Created attachment 184443 [details] Patch

Comment on attachment 184443 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=184443&action=review > Source/WebKit2/ChangeLog:10 > + Currently, LayerTreeHostMac and LayerTreeHostGtk override > + LayerTreeHost::deviceScaleFactorDidChange(), but two implementations just call > + GraphicsLayer::deviceOrPageScaleFactorChanged(). It means we must call Currently LayerTreeHostMac and *Gtk overrides LayerTreeHost::deviceScaleFactorDidChange(), but the override just calls GraphicsLayer::deviceOrPageScaleFactorChanged(). OK, that seems right. The device scale changed, so it calls a functions which should be called when device scale (or page scale) changes. > Source/WebKit2/ChangeLog:12 > + LayerTreeHost::deviceScaleFactorDidChange() when a page scale factor as well as a > + device scale factor are changed. Why does it mean that? Why does that mean you must call ::deviceScaleFactorChange() when the page scale changed? How do you deduct that? > Source/WebKit2/ChangeLog:16 > + So this patch makes scalePage() and setDeviceScaleFactor() in WebPage call > + LayerTreeHost::deviceScaleFactorDidChange(), and renames from > + deviceScaleFactorDidChange() to deviceOrPageScaleFactorChanged(). I dont get this. You need to explain it better. scalePage doesn't include the device scale right? so then why do we need to call deviceScaleFactorDidChange()? Anyway you are actually renaming that to deviceOrPage... which clear up the naming, but you still don't explain why it must be called.

(In reply to comment #5) > (From update of attachment 184443 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=184443&action=review Thank you for your review! I'll refine changelog. > Anyway you are actually renaming that to deviceOrPage... which clear up the naming, but you still don't explain why it must be called. Yeah, you are right. It is because I don't fully understand Mac's rendering routine yet. Although Gtk override it, Gtk does not use both device and page scale factor AFAIK. So I must explain using Mac but I could not do that yet. btw, this is preparation of Bug 105978 that makes EFL and Qt use page scale factor. Without this patch, EFL and Qt have problem. I can verify this patch's justice with Bug 105978 on EFL and Qt.

Created attachment 184688 [details] Patch

Bug 66229 created deviceScaleChanged callback for LayerTreeHost.

thank you for your review!

Comment on attachment 184688 [details] Patch Clearing flags on attachment: 184688 Committed r141171: <http://trac.webkit.org/changeset/141171>

All reviewed patches have been landed. Closing bug.