Bug 66229 – WebKit doesn't react to device scale factor changes

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

[Adam Roben (:aroben)]

A window's scale factor can change over time, in a couple of different ways:

(1) it could be dragged from one monitor to another monitor that has a different scale factor
(2) the scale factor of the entire display could be changed (which, I believe, ripples across the individual windows)

We need to make sure that when this happens, WebKit responds appropriately.

[Adam Roben (:aroben)]

<rdar://problem/9906269>

[Adam Roben (:aroben)]

Created attachment 103915 [details]
Update pages' style and content scale when the window's backing scale factor changes

[Simon Fraser (smfr)]

Comment on attachment 103915 [details]
Update pages' style and content scale when the window's backing scale factor changes

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

> Source/WebCore/page/Frame.h:172
> +#if USE(ACCELERATED_COMPOSITING)
> +        void deviceOrPageScaleFactorChanged();
> +#endif

I don't think this method should be #ifdeffed. In fact, it could be the one entry point that then calls setNeedsRecalcStyleInAllFrames().

[Adam Roben (:aroben)]

Comment on attachment 103915 [details]
Update pages' style and content scale when the window's backing scale factor changes

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

>> Source/WebCore/page/Frame.h:172
>> +#endif
> 
> I don't think this method should be #ifdeffed. In fact, it could be the one entry point that then calls setNeedsRecalcStyleInAllFrames().

I can un-ifdef it. But I don't think it should call setNeedsRecalcStyleInAllFrames. Hyatt told me we don't want to call that function when pinch-zooming, as it will be too slow, and the page scale factor doesn't affect style anyway.

[Adam Roben (:aroben)]

Simon and I decided that having a separate, public deviceScaleFactorChanged function that calls through to the private deviceOrPageScaleFactorChanged function would be clearest. Of course, all this logic should really be on Page, not Frame.

[Adam Roben (:aroben)]

Created attachment 103943 [details]
Update pages' style and content scale when the window's backing scale factor changes

[Simon Fraser (smfr)]

Comment on attachment 103943 [details]
Update pages' style and content scale when the window's backing scale factor changes

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

> Source/WebKit/mac/WebView/WebView.mm:3395
> +#if USE(ACCELERATED_COMPOSITING)
> +    _private->page->mainFrame()->deviceScaleFactorChanged();
> +#endif

This should not be inside #ifdefs now, right?

> Source/WebKit2/WebProcess/WebPage/ca/LayerTreeHostCA.cpp:153
> +void LayerTreeHostCA::deviceScaleFactorDidChange()

Seems odd to have methods called deviceScaleFactorChanged() and deviceScaleFactorDidChange() in various places. Can they share the same name?

[Adam Roben (:aroben)]

Comment on attachment 103943 [details]
Update pages' style and content scale when the window's backing scale factor changes

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

>> Source/WebKit/mac/WebView/WebView.mm:3395
>> +#endif
> 
> This should not be inside #ifdefs now, right?

Right! Removed.

>> Source/WebKit2/WebProcess/WebPage/ca/LayerTreeHostCA.cpp:153
>> +void LayerTreeHostCA::deviceScaleFactorDidChange()
> 
> Seems odd to have methods called deviceScaleFactorChanged() and deviceScaleFactorDidChange() in various places. Can they share the same name?

In each case I was trying to match other functions in the same class (specifically, Frame::deviceOrPageScaleFactorChanged and related functions in RenderLayerCompositor and GraphicsLayer, and LayerTreeHost::sizeDidChange). I agree it would be nice if we had a consistent "did change" vs. "changed" convention throughout our code, particularly for cases like this where the two clash.

[Adam Roben (:aroben)]

Committed r93058: <http://trac.webkit.org/changeset/93058>