Bug 138305 - [EFL] Improve previous temporary fix against drawing (0,0) position before rendering content of next web page
Summary: [EFL] Improve previous temporary fix against drawing (0,0) position before re...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gyuyoung Kim
URL:
Keywords:
Depends on: 133300
Blocks:
  Show dependency treegraph
 
Reported: 2014-11-03 03:32 PST by Gyuyoung Kim
Modified: 2014-11-05 21:35 PST (History)
6 users (show)

See Also:


Attachments
WIP (7.99 KB, patch)
2014-11-03 03:42 PST, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
Patch (6.67 KB, patch)
2014-11-03 18:30 PST, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
another_approach (8.51 KB, patch)
2014-11-03 20:28 PST, Ryuan Choi
no flags Details | Formatted Diff | Diff
another_approach2 (8.59 KB, patch)
2014-11-03 20:47 PST, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (7.50 KB, patch)
2014-11-04 02:38 PST, Ryuan Choi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gyuyoung Kim 2014-11-03 03:32:20 PST
In EFL port, there was to draw current content at (0, 0) scroll position before drawing next page. r170243 fixed it using a flag for the occasion though, we should fix it with more correct way.
Comment 1 Gyuyoung Kim 2014-11-03 03:42:47 PST
Created attachment 240838 [details]
WIP
Comment 2 Gyuyoung Kim 2014-11-03 18:30:36 PST
Created attachment 240896 [details]
Patch
Comment 3 Gyuyoung Kim 2014-11-03 18:38:41 PST
CC'ing Ossy.
Comment 4 Ryuan Choi 2014-11-03 20:28:04 PST
Created attachment 240905 [details]
another_approach
Comment 5 Ryuan Choi 2014-11-03 20:30:22 PST
(In reply to comment #4)
> Created attachment 240905 [details]
> another_approach

Dear Gyuyoung,
As mentioned in IRC,

I uplodated my approach based on analysis.

Feel free to check this.
I hope that this is helpfull for you
Comment 6 Ryuan Choi 2014-11-03 20:47:50 PST
Created attachment 240907 [details]
another_approach2
Comment 7 Gyuyoung Kim 2014-11-03 23:49:21 PST
Comment on attachment 240907 [details]
another_approach2

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

> Source/WebKit2/ChangeLog:13
> +        rendered the results of first visually not empty layout.

I modify this description a little.

"WebKit2 has freezed layer tree until frame load completion since r101838. In EFL port, we unfreeze it when PageViewportController::pageTransitionViewportReady() is called though, UIProcess on EFL port updates cairo surface during the freezing time of layer tree. Thus UIProcess should not update the layer trees until the layerTreeState is unfrozen."

> Source/WebKit2/UIProcess/CoordinatedGraphics/PageViewportController.h:107
> +    enum class PageState {

I think that it would be nicer if we use variable or enum name based on "layerTreeStateIsFrozen" to improve our code readability. Under my understanding, this state is depended on m_layerTreeStateIsFrozen state.
Comment 8 Ryuan Choi 2014-11-04 02:38:16 PST
Created attachment 240919 [details]
Patch
Comment 9 Ryuan Choi 2014-11-04 02:50:19 PST
Comment on attachment 240907 [details]
another_approach2

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

>> Source/WebKit2/UIProcess/CoordinatedGraphics/PageViewportController.h:107
>> +    enum class PageState {
> 
> I think that it would be nicer if we use variable or enum name based on "layerTreeStateIsFrozen" to improve our code readability. Under my understanding, this state is depended on m_layerTreeStateIsFrozen state.

After considered more, boolean looks enough like your original approach.

I changed this as boolean member, m_layerTreeStateIsFrozen
Comment 10 Gyuyoung Kim 2014-11-05 00:08:53 PST
Comment on attachment 240919 [details]
Patch

LGTM, r=me.
Comment 11 Gyuyoung Kim 2014-11-05 00:56:22 PST
Before landing, could you check if there is any side effect ? It looks black screen is drawn as soon as the MiniBrowser is shown.
Comment 12 Gyuyoung Kim 2014-11-05 00:58:03 PST
Comment on attachment 240919 [details]
Patch

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

> Source/WebKit2/UIProcess/CoordinatedGraphics/PageViewportController.cpp:273
> +    if (!m_layerTreeStateIsFrozen)

I'm not sure if we can block during the m_layerTreeStateIsFrozen is true. If we block to update display, can black screen be shown during the period ?
Comment 13 Ryuan Choi 2014-11-05 05:54:05 PST
(In reply to comment #12)
> Comment on attachment 240919 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=240919&action=review
> 
> > Source/WebKit2/UIProcess/CoordinatedGraphics/PageViewportController.cpp:273
> > +    if (!m_layerTreeStateIsFrozen)
> 
> I'm not sure if we can block during the m_layerTreeStateIsFrozen is true. If
> we block to update display, can black screen be shown during the period ?

Well, I can't imagine that scenario.
Are you considering with direct rendering feature?

In that scenario, previous screen will be used because we didn't remove it.
Comment 14 Gyuyoung Kim 2014-11-05 20:48:44 PST
(In reply to comment #13)
> (In reply to comment #12)
> > Comment on attachment 240919 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=240919&action=review
> > 
> > > Source/WebKit2/UIProcess/CoordinatedGraphics/PageViewportController.cpp:273
> > > +    if (!m_layerTreeStateIsFrozen)
> > 
> > I'm not sure if we can block during the m_layerTreeStateIsFrozen is true. If
> > we block to update display, can black screen be shown during the period ?
> 
> Well, I can't imagine that scenario.
> Are you considering with direct rendering feature?
> 
> In that scenario, previous screen will be used because we didn't remove it.

This problem occurs on current WebKit EFL MiniBrowser. Other previous commit looks like to made this problem.
Comment 15 WebKit Commit Bot 2014-11-05 21:34:57 PST
Comment on attachment 240919 [details]
Patch

Clearing flags on attachment: 240919

Committed r175669: <http://trac.webkit.org/changeset/175669>
Comment 16 WebKit Commit Bot 2014-11-05 21:35:02 PST
All reviewed patches have been landed.  Closing bug.