Bug 126355 - [CoordinatedGraphics] The changed scale of page should be applied to WebProcess directly when page is scaled.
Summary: [CoordinatedGraphics] The changed scale of page should be applied to WebProce...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: All Linux
: P3 Normal
Assignee: Hunseop Jeong
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-12-31 20:54 PST by Hunseop Jeong
Modified: 2016-04-19 03:06 PDT (History)
11 users (show)

See Also:


Attachments
Patch (2.59 KB, patch)
2013-12-31 23:40 PST, Hunseop Jeong
no flags Details | Formatted Diff | Diff
Patch (2.33 KB, patch)
2013-12-31 23:43 PST, Hunseop Jeong
no flags Details | Formatted Diff | Diff
Patch (2.46 KB, patch)
2014-01-09 23:03 PST, Hunseop Jeong
no flags Details | Formatted Diff | Diff
Patch (2.43 KB, patch)
2014-03-04 22:57 PST, Hunseop Jeong
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.