Bug 110298

Summary: [EFL][QT][WK2] Turn on ApplyDeviceScaleFactorInCompositor always.
Product: WebKit Reporter: Dongseong Hwang <dongseong.hwang>
Component: Layout and RenderingAssignee: Dongseong Hwang <dongseong.hwang>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, benjamin, cmarcelo, gyuyoung.kim, jturcotte, kenneth, luiz, mikhail.pozdnyakov, noam, rakuco, simon.fraser, webkit.review.bot, zeno
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 110066, 110197, 110450, 110847, 111056    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
kenneth: review+, kenneth: commit-queue-
Patch for landing none

Dongseong Hwang
Reported 2013-02-19 21:21:09 PST
We turn on settings not related to useFixedLayout in CoordinatedLayerTreeHost. 1. setScrollingCoordinatorEnabled 2. setApplyDeviceScaleFactorInCompositor 3. setApplyPageScaleFactorInCompositor
Attachments
Patch (11.74 KB, patch)
2013-02-19 21:22 PST, Dongseong Hwang 		no flags
DetailsFormatted DiffDiff
Patch (15.20 KB, patch)
2013-02-25 21:54 PST, Dongseong Hwang 		no flags
DetailsFormatted DiffDiff
Patch (15.22 KB, patch)
2013-02-25 22:40 PST, Dongseong Hwang 		no flags
DetailsFormatted DiffDiff
Patch (6.24 KB, patch)
2013-02-28 01:41 PST, Dongseong Hwang 		kenneth: review+
kenneth: commit-queue-
DetailsFormatted DiffDiff
Patch for landing (6.31 KB, patch)
2013-03-03 14:53 PST, Dongseong Hwang 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Dongseong Hwang
Comment 1 2013-02-19 21:22:48 PST
Created attachment 189233 [details] Patch
Dongseong Hwang
Comment 2 2013-02-25 21:54:47 PST
Created attachment 190201 [details] Patch
EFL EWS Bot
Comment 3 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
Dongseong Hwang
Comment 4 2013-02-25 22:40:11 PST
Created attachment 190208 [details] Patch
Dongseong Hwang
Comment 5 2013-02-28 01:41:13 PST
Created attachment 190679 [details] Patch
Kenneth Rohde Christiansen
Comment 6 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.
Dongseong Hwang
Comment 7 2013-03-03 14:53:54 PST
Created attachment 191149 [details] Patch for landing
Dongseong Hwang
Comment 8 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 :)
WebKit Review Bot
Comment 9 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>
WebKit Review Bot
Comment 10 2013-03-03 15:56:18 PST
All reviewed patches have been landed. Closing bug.
Kenneth Rohde Christiansen
Comment 11 2013-03-03 16:01:24 PST
I forgot to say that this should be looked over by a WK2 owner :-(
Dongseong Hwang
Comment 12 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.
Benjamin Poulain
Comment 13 2013-03-03 17:41:20 PST
Kenneth, please do not directly r+ patches for WebKit2.
Dongseong Hwang
Comment 14 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?
Benjamin Poulain
Comment 15 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).
Dongseong Hwang
Comment 16 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
Kenneth Rohde Christiansen
Comment 17 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 :-)
Note You need to log in before you can comment on or make changes to this bug.