Bug 126355

Summary: [CoordinatedGraphics] The changed scale of page should be applied to WebProcess directly when page is scaled.
Product: WebKit Reporter: Hunseop Jeong <hs85.jeong>
Component: WebKit EFLAssignee: Hunseop Jeong <hs85.jeong>
Status: NEW ---    
Severity: Normal CC: bunhere, cmarcelo, commit-queue, enmi.lee, gyuyoung.kim, lucas.de.marchi, luiz, noam, ryuan.choi, sergio, zeno
Priority: P3    
Version: 528+ (Nightly build)   
Hardware: All   
OS: Linux   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description Hunseop Jeong 2013-12-31 20:54:58 PST
Currently, don't send the changed scale of page to WebProcess directly when page is scaled.
Because scale of page in WebProcess isn't set in a moment, unnecessary calculation of tile is added.
Comment 1 Hunseop Jeong 2013-12-31 23:40:12 PST
Created attachment 220180 [details]
Patch
Comment 2 Hunseop Jeong 2013-12-31 23:43:18 PST
Created attachment 220181 [details]
Patch
Comment 3 Gyuyoung Kim 2014-01-02 17:32:31 PST
Comment on attachment 220181 [details]
Patch

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

Noam, could you take a look this ?

> Source/WebKit2/ChangeLog:8
> +        Because location of function for sending scale of page to WebProcess is wrong, scale of page in WebProcess isn't promptly set.

scale of page to WebProcess -> a scale of page to WebProcess ?

scale of page in WebProcess -> the scale of page in WebProcess ?

> Source/WebKit2/ChangeLog:9
> +        This patch moves function for sending scale of page to correct location.

I think this patch move a place of scalePage() invocation. This description seem a little unclear.
Comment 4 Hunseop Jeong 2014-01-09 23:03:41 PST
Created attachment 220805 [details]
Patch
Comment 5 Noam Rosenthal 2014-01-09 23:25:24 PST
Comment on attachment 220805 [details]
Patch

What is the implication of this? Is there a flicker? How is this tested?
Comment 6 Gyuyoung Kim 2014-03-04 22:28:13 PST
Comment on attachment 220805 [details]
Patch

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

> Source/WebKit2/UIProcess/CoordinatedGraphics/PageViewportController.cpp:324
> +    m_webPageProxy->scalePage(m_pageScaleFactor, roundedIntPoint(m_contentsPosition));

Why do you call scalePage() in applyPositionAfterRenderingContents() instead of applyScaleAfterRenderingContents() ?
Comment 7 Gyuyoung Kim 2014-03-04 22:28:37 PST
Comment on attachment 220805 [details]
Patch

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

> Source/WebKit2/ChangeLog:3
> +        [EFL] The changed scale of page should be applied to WebProcess directly when page is scaled.

[EFL] -> [CoordinatedGraphics]
Comment 8 Hunseop Jeong 2014-03-04 22:41:16 PST
When changed the scale of page, didn't apply the scale (In reply to comment #6)
> (From update of attachment 220805 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=220805&action=review
> > Source/WebKit2/UIProcess/CoordinatedGraphics/PageViewportController.cpp:324
> > +    m_webPageProxy->scalePage(m_pageScaleFactor, roundedIntPoint(m_contentsPosition));
> Why do you call scalePage() in applyPositionAfterRenderingContents() instead of applyScaleAfterRenderingContents() ?

You are right, it was my fault.
I will move the scalePage() to applyScaleAfterRenderingContents().
Comment 9 Hunseop Jeong 2014-03-04 22:57:51 PST
Created attachment 225860 [details]
Patch
Comment 10 Gyuyoung Kim 2014-03-04 23:06:37 PST
(In reply to comment #5)
> (From update of attachment 220805 [details])
> What is the implication of this? Is there a flicker? How is this tested?

As Noam said, you have to explain why we should move it. Any performance benefit ? I wonder it as well.
Comment 11 Gyuyoung Kim 2016-04-19 01:57:32 PDT
Comment on attachment 225860 [details]
Patch

Clear r? because there was no reply against last question for a long time.
Comment 12 Hunseop Jeong 2016-04-19 03:06:19 PDT
Sorry for the late reply.
I uploaded the patch for the benefit of scaling on the mobile phone.
But I don't know that this patch is still valid.