Summary: | WebKit doesn't react to device scale factor changes | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adam Roben (:aroben) <aroben> | ||||||
Component: | Layout and Rendering | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | andersca, hyatt, jamesr, sam, simon.fraser, sullivan, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Mac (Intel) | ||||||||
OS: | OS X 10.7 | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 66244 | ||||||||
Attachments: |
|
Description
Adam Roben (:aroben)
2011-08-15 08:08:20 PDT
Created attachment 103915 [details]
Update pages' style and content scale when the window's backing scale factor changes
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(). 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. 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. Created attachment 103943 [details]
Update pages' style and content scale when the window's backing scale factor changes
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? 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. Committed r93058: <http://trac.webkit.org/changeset/93058> |