Bug 106355 – [Qt][EFL][WK2] Remove redundant device pixel ratio adjustment from PageViewportController

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Andras Becsi]

[WK2] Remove redundant device pixel ratio adjustment from PageViewportController

[Andras Becsi]

Created attachment 181717 [details]
Patch

[Andras Becsi]

(In reply to comment #1)
> Created an attachment (id=181717) [details]
> Patch

A follow-up could remove the device pixel ratio adjustment logic from the ViewportAttributes also, but I'm not sure how dependent other ports are on that code.

[Kenneth Rohde Christiansen]

Comment on attachment 181717 [details]
Patch

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

> Source/WebKit2/ChangeLog:13
> +        As a result we can follow the same aproach as QtQuick and all the

spelling

> Source/WebKit2/ChangeLog:15
> +        coordinates in PageViewportController need to be in device independent
> +        pixels (dips) thus we do no longer need to adjust with the device pixel

AKA UI pixels

> Source/WebKit2/UIProcess/efl/PageViewportControllerClientEfl.cpp:66
> +    // The viewport controller expects sizes in device independent coordinates.

Maybe it is more understandable saying that

// The viewport controller expects sizes in UI units, and not raw device units.

> Source/WebKit2/UIProcess/efl/PageViewportControllerClientEfl.cpp:67
> +    size.scale(1 / m_controller->devicePixelRatio());

devicePixelRatio is called deviceScaleFactor most places now, maybe we should rename

[Andras Becsi]

Committed r139189: <http://trac.webkit.org/changeset/139189>

[Andras Becsi]

Comment on attachment 181717 [details]
Patch

Clearing flags from attachment.

[Csaba Osztrogonác]

(In reply to comment #4)
> Committed r139189: <http://trac.webkit.org/changeset/139189>

It broke many Qt-WK2 API tests:

FAIL!  : qmltests::DoubleTapToZoom::test_basic_zoomInAndBack() Compared values are not the same
   Actual   (): 2.5
   Expected (): 1.3333333333333333
   Loc: [/home/webkitbuildbot/slaves/release64bitWebKit2_EC2/buildslave/qt-linux-64-release-webkit2/build/Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_doubleTapToZoom.qml(95)]
FAIL!  : qmltests::DoubleTapToZoom::test_double_zoomInAndBack() Compared values are not the same
   Actual   (): 128x96
   Expected (): 320x240
   Loc: [/home/webkitbuildbot/slaves/release64bitWebKit2_EC2/buildslave/qt-linux-64-release-webkit2/build/Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_doubleTapToZoom.qml(106)]
FAIL!  : qmltests::DoubleTapToZoom::test_double_zoomInAndBack2() Compared values are not the same
   Actual   (): 2.5
   Expected (): 1.3333333333333333
   Loc: [/home/webkitbuildbot/slaves/release64bitWebKit2_EC2/buildslave/qt-linux-64-release-webkit2/build/Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_doubleTapToZoom.qml(146)]
FAIL!  : qmltests::DoubleTapToZoom::test_double_zoomInOutAndBack() Compared values are not the same
   Actual   (): 128x96
   Expected (): 320x240
   Loc: [/home/webkitbuildbot/slaves/release64bitWebKit2_EC2/buildslave/qt-linux-64-release-webkit2/build/Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_doubleTapToZoom.qml(166)]
FAIL!  : qmltests::DoubleTapToZoom::test_double_zoomInOutAndBack2() Compared values are not the same
   Actual   (): 2.5
   Expected (): 2
   Loc: [/home/webkitbuildbot/slaves/release64bitWebKit2_EC2/buildslave/qt-linux-64-release-webkit2/build/Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_doubleTapToZoom.qml(207)]
FAIL!  : qmltests::Resize::test_resizeAfterNeutralZoom() Compared values are not the same
   Actual   (): 2.5
   Expected (): 1.2307692307692308
   Loc: [/home/webkitbuildbot/slaves/release64bitWebKit2_EC2/buildslave/qt-linux-64-release-webkit2/build/Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_resize.qml(120)]
FAIL!  : qmltests::Resize::test_resizeZoomedIn() Compared values are not the same
   Actual   (): 2.5
   Expected (): 1
   Loc: [/home/webkitbuildbot/slaves/release64bitWebKit2_EC2/buildslave/qt-linux-64-release-webkit2/build/Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_resize.qml(143)]

[Andras Becsi]

(In reply to comment #6)
> (In reply to comment #4)
> > Committed r139189: <http://trac.webkit.org/changeset/139189>
> 
> It broke many Qt-WK2 API tests:

Looking into it, sorry for the breakage.

[Andras Becsi]

(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #4)
> > > Committed r139189: <http://trac.webkit.org/changeset/139189>
> > 
> > It broke many Qt-WK2 API tests:
> 
> Looking into it, sorry for the breakage.

Fixed in http://trac.webkit.org/changeset/139215.

[Dongseong Hwang]

Great. Code became more succinct.

I fixed some nits in Bug 106499 and Bug 106500.
And I fixed blurring regression in Bug 106504.

[Dongseong Hwang]

I fixed regression that MiniBrowser renders abnormally with -r option in Bug 106512.

[Andras Becsi]

Thanks for fixing these issues.