Bug 110298 - [EFL][QT][WK2] Turn on ApplyDeviceScaleFactorInCompositor always.
Summary: [EFL][QT][WK2] Turn on ApplyDeviceScaleFactorInCompositor always.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dongseong Hwang
URL:
Keywords:
Depends on:
Blocks: 110066 110197 110450 110847 111056
  Show dependency treegraph
 
Reported: 2013-02-19 21:21 PST by Dongseong Hwang
Modified: 2013-03-04 00:54 PST (History)
13 users (show)

See Also:


Attachments
Patch (11.74 KB, patch)
2013-02-19 21:22 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff
Patch (15.20 KB, patch)
2013-02-25 21:54 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff
Patch (15.22 KB, patch)
2013-02-25 22:40 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff
Patch (6.24 KB, patch)
2013-02-28 01:41 PST, Dongseong Hwang
kenneth: review+
kenneth: commit-queue-
Details | Formatted Diff | Diff
Patch for landing (6.31 KB, patch)
2013-03-03 14:53 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dongseong Hwang 2013-02-19 21:21:09 PST
We turn on settings not related to useFixedLayout in CoordinatedLayerTreeHost.
1. setScrollingCoordinatorEnabled
2. setApplyDeviceScaleFactorInCompositor
3. setApplyPageScaleFactorInCompositor
Comment 1 Dongseong Hwang 2013-02-19 21:22:48 PST
Created attachment 189233 [details]
Patch
Comment 2 Dongseong Hwang 2013-02-25 21:54:47 PST
Created attachment 190201 [details]
Patch
Comment 3 EFL EWS Bot 2013-02-25 22:02:57 PST
Comment on attachment 190201 [details]
Patch

Attachment 190201 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/16744776
Comment 4 Dongseong Hwang 2013-02-25 22:40:11 PST
Created attachment 190208 [details]
Patch
Comment 5 Dongseong Hwang 2013-02-28 01:41:13 PST
Created attachment 190679 [details]
Patch
Comment 6 Kenneth Rohde Christiansen 2013-03-02 08:02:27 PST
Comment on attachment 190679 [details]
Patch

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

> Source/WebKit2/UIProcess/API/efl/EwkView.cpp:457
>  {
>      page()->setIntrinsicDeviceScaleFactor(scale);
> +    setSize(m_size);
>  }

It is not obvious that this actually changes the size.

I would definitely add a comment. // Update internal viewport size after device-scale change.
Comment 7 Dongseong Hwang 2013-03-03 14:53:54 PST
Created attachment 191149 [details]
Patch for landing
Comment 8 Dongseong Hwang 2013-03-03 14:54:38 PST
(In reply to comment #6)
> (From update of attachment 190679 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=190679&action=review
> 
> > Source/WebKit2/UIProcess/API/efl/EwkView.cpp:457
> >  {
> >      page()->setIntrinsicDeviceScaleFactor(scale);
> > +    setSize(m_size);
> >  }
> 
> It is not obvious that this actually changes the size.
> 
> I would definitely add a comment. // Update internal viewport size after device-scale change.

Yes, absolutely. Thanks for review :)
Comment 9 WebKit Review Bot 2013-03-03 15:56:13 PST
Comment on attachment 191149 [details]
Patch for landing

Clearing flags on attachment: 191149

Committed r144585: <http://trac.webkit.org/changeset/144585>
Comment 10 WebKit Review Bot 2013-03-03 15:56:18 PST
All reviewed patches have been landed.  Closing bug.
Comment 11 Kenneth Rohde Christiansen 2013-03-03 16:01:24 PST
I forgot to say that this should be looked over by a WK2 owner :-(
Comment 12 Dongseong Hwang 2013-03-03 17:05:42 PST
(In reply to comment #11)
> I forgot to say that this should be looked over by a WK2 owner :-(

Oops. I forgot too. It's Monday morning. I forget how I work... I'm sorry.

How can I resolve this conflict? First of all, I confess this to one of WK2 owners.
Comment 13 Benjamin Poulain 2013-03-03 17:41:20 PST
Kenneth, please do not directly r+ patches for WebKit2.
Comment 14 Dongseong Hwang 2013-03-03 17:58:10 PST
(In reply to comment #13)
> Kenneth, please do not directly r+ patches for WebKit2.

I'm sorry. It is my fault. I'll keep the rule from now.
Benjamin, could you advice how I resolve it? rollout and re-land?
And could you sign off this patch?
Comment 15 Benjamin Poulain 2013-03-03 18:12:42 PST
> Benjamin, could you advice how I resolve it? rollout and re-land?

No sure :)

I don't know anything about the plans regarding DeviceScaleFactorInCompositor so I cannot really sign off on this.

(But I think the acronym dip should have been avoided, following our practice of not using acronyms unless they are very widely used e.g. HTTP, FTP, etc).
Comment 16 Dongseong Hwang 2013-03-03 18:46:01 PST
(In reply to comment #15)
> > Benjamin, could you advice how I resolve it? rollout and re-land?
> 
> No sure :)
Thanks for kind replying :)

> I don't know anything about the plans regarding DeviceScaleFactorInCompositor so I cannot really sign off on this.

yes. I'm waiting for smfr and andersca's review.
Currently, EFL and Qt don't query applyDeviceScaleFactorInCompositor() while setting setApplyDeviceScaleFactorInCompositor to true, because EFL and Qt don't have code related to applyDeviceScaleFactorInCompositor being false.

> (But I think the acronym dip should have been avoided, following our practice of not using acronyms unless they are very widely used e.g. HTTP, FTP, etc).
aha, I'll avoid acronym. I assumed dip is acceptable because our wiki use dip. http://trac.webkit.org/wiki/ScalesAndZooms
Comment 17 Kenneth Rohde Christiansen 2013-03-04 00:54:22 PST
(In reply to comment #13)
> Kenneth, please do not directly r+ patches for WebKit2.

Yeah it was a mistake. I first realized that I actually r+ed if when it got landed. Sorry about that :-)