Bug 97214

Summary: [Qt] Abstract as much devicePixelRatio logic as possible behind PageViewportController
Product: WebKit Reporter: Jocelyn Turcotte <jturcotte>
Component: New BugsAssignee: Jocelyn Turcotte <jturcotte>
Status: RESOLVED FIXED    
Severity: Normal CC: abecsi, cmarcelo, hausmann, kenneth, menard, webkit.review.bot, zoltan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Jocelyn Turcotte 2012-09-20 07:51:56 PDT
[Qt] Abstract as much devicePixelRatio logic as possible behind PageViewportController
Comment 1 Jocelyn Turcotte 2012-09-20 07:59:05 PDT
Created attachment 164918 [details]
Patch
Comment 2 Andras Becsi 2012-09-20 09:16:07 PDT
Comment on attachment 164918 [details]
Patch

Nice improvement, LGTM.
Comment 3 Kenneth Rohde Christiansen 2012-09-20 23:38:39 PDT
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?
Comment 4 Jocelyn Turcotte 2012-09-21 03:11:40 PDT
(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.
Comment 5 Jocelyn Turcotte 2012-09-25 07:05:21 PDT
Created attachment 165606 [details]
Patch

Patch to commit
Comment 6 WebKit Review Bot 2012-09-25 08:05:40 PDT
Comment on attachment 165606 [details]
Patch

Clearing flags on attachment: 165606

Committed r129508: <http://trac.webkit.org/changeset/129508>
Comment 7 WebKit Review Bot 2012-09-25 08:05:44 PDT
All reviewed patches have been landed.  Closing bug.