[Qt] Abstract as much devicePixelRatio logic as possible behind PageViewportController

[Jocelyn Turcotte]

[Qt] Abstract as much devicePixelRatio logic as possible behind PageViewportController

[Jocelyn Turcotte]

Created attachment 164918 [details]
Patch

[Andras Becsi]

Comment on attachment 164918 [details]
Patch

Nice improvement, LGTM.

[Kenneth Rohde Christiansen]

Comment on attachment 164918 [details]
Patch

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

> Source/WebKit2/ChangeLog:21
> +        Other related changes:
> +        - Make sure that the controller and it's client are only exchanging effective scales
> +        (cssScale * devicePixelRatio)
> +        - Remove members duplicating m_rawAttributes values
> +        - Use a separate member for the minimum scale to fit the page, separate from the viewport arguments
> +        - Make sure that the minimum scale to fit is adjusted if the contents size isn't
> +          updated after the viewport attributes changed
> +        - Make the scale conversion functions private to discourage this logic from spreading in the client
> +

nice!

> Source/WebKit2/UIProcess/PageViewportController.h:87
> +    float devicePixelRatio() const { return m_rawAttributes.devicePixelRatio; }

FYI devicePixelRatio should be removed from ViewportAttributes and gotten from elsewhere, it just havent been done yet

> Source/WebKit2/UIProcess/PageViewportController.h:115
> +    float m_minimumScaleToFit;

so this is viewort scale? m_minimumViewportScaleToFit?

[Jocelyn Turcotte]

(In reply to comment #3)
> > Source/WebKit2/UIProcess/PageViewportController.h:87
> > +    float devicePixelRatio() const { return m_rawAttributes.devicePixelRatio; }
> 
> FYI devicePixelRatio should be removed from ViewportAttributes and gotten from elsewhere, it just havent been done yet
Ok I'll have PageViewportController::devicePixelRatio fetch m_webPageProxy->pageScaleFactor() instead then.
> 
> > Source/WebKit2/UIProcess/PageViewportController.h:115
> > +    float m_minimumScaleToFit;
> 
> so this is viewort scale? m_minimumViewportScaleToFit?
No this is the same as m_rawAttributes, "css/document" scale.

[Jocelyn Turcotte]

Created attachment 165606 [details]
Patch

Patch to commit

[WebKit Review Bot]

Comment on attachment 165606 [details]
Patch

Clearing flags on attachment: 165606

Committed r129508: <http://trac.webkit.org/changeset/129508>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.