Bug 106355 - [Qt][EFL][WK2] Remove redundant device pixel ratio adjustment from PageViewportController
Summary: [Qt][EFL][WK2] Remove redundant device pixel ratio adjustment from PageViewpo...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andras Becsi
URL:
Keywords:
Depends on:
Blocks: 106499 106500 106504 106512
  Show dependency treegraph
 
Reported: 2013-01-08 11:32 PST by Andras Becsi
Modified: 2013-01-10 10:53 PST (History)
10 users (show)

See Also:


Attachments
Patch (18.35 KB, patch)
2013-01-08 11:37 PST, Andras Becsi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andras Becsi 2013-01-08 11:32:39 PST
[WK2] Remove redundant device pixel ratio adjustment from PageViewportController
Comment 1 Andras Becsi 2013-01-08 11:37:14 PST
Created attachment 181717 [details]
Patch
Comment 2 Andras Becsi 2013-01-08 11:41:35 PST
(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.
Comment 3 Kenneth Rohde Christiansen 2013-01-08 12:37:47 PST
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
Comment 4 Andras Becsi 2013-01-09 06:01:29 PST
Committed r139189: <http://trac.webkit.org/changeset/139189>
Comment 5 Andras Becsi 2013-01-09 06:10:38 PST
Comment on attachment 181717 [details]
Patch

Clearing flags from attachment.
Comment 6 Csaba Osztrogonác 2013-01-09 08:56:09 PST
(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)]
Comment 7 Andras Becsi 2013-01-09 09:38:34 PST
(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.
Comment 8 Andras Becsi 2013-01-09 11:53:42 PST
(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.
Comment 9 Dongseong Hwang 2013-01-09 16:55:30 PST
Great. Code became more succinct.

I fixed some nits in Bug 106499 and Bug 106500.
And I fixed blurring regression in Bug 106504.
Comment 10 Dongseong Hwang 2013-01-09 18:00:30 PST
I fixed regression that MiniBrowser renders abnormally with -r option in Bug 106512.
Comment 11 Andras Becsi 2013-01-10 10:53:19 PST
Thanks for fixing these issues.