[WK2][EFL] REGRESSION(r139189): MiniBrowser renders abnormally with -r option.

[Dongseong Hwang]

If running MiniBrowser with -r option to set deviceScaleFactor, MiniBrowser renders abnormally.

[Dongseong Hwang]

Created attachment 182034 [details]
Patch

[Kenneth Rohde Christiansen]

Comment on attachment 182034 [details]
Patch

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

> Source/WebKit2/UIProcess/API/efl/EwkViewImpl.h:203
> +    void setContentsScaleFactor(float scaleFactor) { m_contentsScaleFactor = scaleFactor; }
> +    float contentsScaleFactor() const { return m_contentsScaleFactor; }

isnt it better to separate it into two here? like it having page scale factor as well as device scale factor.

[Dongseong Hwang]

Created attachment 182095 [details]
Patch

[Dongseong Hwang]

(In reply to comment #2)
> (From update of attachment 182034 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=182034&action=review
> 
> > Source/WebKit2/UIProcess/API/efl/EwkViewImpl.h:203
> > +    void setContentsScaleFactor(float scaleFactor) { m_contentsScaleFactor = scaleFactor; }
> > +    float contentsScaleFactor() const { return m_contentsScaleFactor; }
> 
> isnt it better to separate it into two here? like it having page scale factor as well as device scale factor.

The second patch changed from setContentsScaleFactor to setPageScaleFactor, and did not add setDeviceScaleFactor because it already exists.
It means PageViewportControllerClientEFL uses EwkViewImpl API differently to the previous patch.

[Kenneth Rohde Christiansen]

Comment on attachment 182095 [details]
Patch

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

> Source/WebKit2/UIProcess/API/efl/EwkViewImpl.cpp:313
> -    transform.scale(1 / m_scaleFactor);
> +    transform.scale(1 / effectiveScaleFactor());

I would prefer

transform.scale(1 / m_pageScaleFactor)
transform.scale(1 / deviceScaleFactor())

That is much more clear

[Dongseong Hwang]

Created attachment 182247 [details]
Patch

[Dongseong Hwang]

(In reply to comment #5)
> (From update of attachment 182095 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=182095&action=review
> I would prefer
> 
> transform.scale(1 / m_pageScaleFactor)
> transform.scale(1 / deviceScaleFactor())
> 
> That is much more clear

Yeah, done.

[Dongseong Hwang]

Created attachment 182248 [details]
Patch

[EFL EWS Bot]

Comment on attachment 182248 [details]
Patch

Attachment 182248 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15802256

[Dongseong Hwang]

Created attachment 182252 [details]
Patch

[Kenneth Rohde Christiansen]

Comment on attachment 182252 [details]
Patch

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

> Source/WebKit2/UIProcess/API/efl/EwkViewImpl.cpp:415
> -        cairo_scale(graphicsContext.get(), m_scaleFactor, m_scaleFactor);
> +        cairo_scale(graphicsContext.get(), effectiveScaleFactor(), effectiveScaleFactor());

I would do the same here and get rid of that method

[Dongseong Hwang]

Created attachment 182508 [details]
Patch

[Dongseong Hwang]

(In reply to comment #11)
> I would do the same here and get rid of that method

Done. I removed effectiveScaleFactor().

[WebKit Review Bot]

Comment on attachment 182508 [details]
Patch

Clearing flags on attachment: 182508

Committed r139675: <http://trac.webkit.org/changeset/139675>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.