Bug 71352 - [EFL] Port tiled backing store
Summary: [EFL] Port tiled backing store
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: Ryuan Choi
URL:
Keywords:
Depends on: 71350
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-02 02:05 PDT by Tomasz Morawski
Modified: 2012-11-14 22:45 PST (History)
10 users (show)

See Also:


Attachments
Initial implementation (10.77 KB, patch)
2011-11-02 03:58 PDT, Tomasz Morawski
gyuyoung.kim: commit-queue-
Details | Formatted Diff | Diff
Updated according to Grzegorz's suggestions (10.73 KB, patch)
2011-11-04 00:54 PDT, Tomasz Morawski
gyuyoung.kim: commit-queue-
Details | Formatted Diff | Diff
Updated according to suggestions (11.52 KB, patch)
2011-11-08 00:46 PST, Tomasz Morawski
t.morawski: commit-queue-
Details | Formatted Diff | Diff
Updated (12.74 KB, patch)
2011-11-16 00:14 PST, Tomasz Morawski
no flags Details | Formatted Diff | Diff
Rebased and updated according to KwangHyuk suggestions (12.53 KB, patch)
2011-12-20 02:03 PST, Tomasz Morawski
no flags Details | Formatted Diff | Diff
Updated (12.58 KB, patch)
2011-12-21 01:16 PST, Tomasz Morawski
no flags Details | Formatted Diff | Diff
Patch (14.94 KB, patch)
2012-11-14 18:18 PST, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (14.94 KB, patch)
2012-11-14 18:41 PST, Ryuan Choi
no flags Details | Formatted Diff | Diff
patch for land (14.97 KB, patch)
2012-11-14 21:58 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 Tomasz Morawski 2011-11-02 02:05:39 PDT
Ported tiled backing store to EFL WebKit port.
Comment 1 Tomasz Morawski 2011-11-02 03:58:06 PDT
Created attachment 113302 [details]
Initial implementation

Initial patch for discussion
Comment 2 Grzegorz Czajkowski 2011-11-02 04:19:27 PDT
View in context: https://bugs.webkit.org/attachment.cgi?id=113302&action=review

> Source/WebKit/efl/ewk/ewk_view.cpp:2286
> +#if USE(TILED_BACKING_STORE)

Please add an internal documentation for newly added functions.

> Source/WebKit/efl/ewk/ewk_view.cpp:2287
> +Eina_Bool ewk_view_setting_tiled_backingstore_enable_set(Evas_Object* o, Eina_Bool enable)

I would rather prefer using a new WebKit coding style, ewkView instaed of o, smartData instead of sd etc.

> Source/WebKit/efl/ewk/ewk_view.cpp:2650
> +void ewk_view_backingstore_invalidate(Evas_Object* o, const Eina_Rectangle *area)

Ditto.

> Source/WebKit/efl/ewk/ewk_view.cpp:2663
> +void ewk_view_backingstore_adjuct_visible_rect(Evas_Object* o)

Ditto.
Comment 3 Gyuyoung Kim 2011-11-02 07:06:47 PDT
Comment on attachment 113302 [details]
Initial implementation

Attachment 113302 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/10258254
Comment 4 Tomasz Morawski 2011-11-02 07:14:26 PDT
(In reply to comment #3)
> (From update of attachment 113302 [details])
> Attachment 113302 [details] did not pass efl-ews (efl):
> Output: http://queues.webkit.org/results/10258254
Compilation error due to that this patch depends on 71350. 

Anyway, please also check if EFL building bot has libcairo-1.10.2 or more recent.
Comment 5 KwangHyuk 2011-11-02 09:56:36 PDT
> Source/WebKit/efl/ewk/ewk_private.h:162
> +EAPI void ewk_view_backingstore_invalidate(Evas_Object* o, const Eina_Rectangle *area);

Would you check the location of '*' for parameter area ? and I prefer ewkView instead of one character 'o'. :)
In addition, backing_store looks more natural.

> Source/WebKit/efl/ewk/ewk_private.h:163
> +EAPI void ewk_view_backingstore_adjuct_visible_rect(Evas_Object* o);

May adjuct mean adjust ? and  prefer ewkView instead of one character 'o'.
In addition, Would you check the sequence of each word for API ?

> Source/WebKit/efl/ewk/ewk_private.h:164
> +EAPI Eina_Bool ewk_view_setting_tiled_backingstore_enable_set(Evas_Object* o, Eina_Bool enable);

As ewk_private.h is for internal usage, I think that EAPI won't be required in here.
Comment 6 Gyuyoung Kim 2011-11-02 18:02:48 PDT
Comment on attachment 113302 [details]
Initial implementation

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

> Source/WebCore/CMakeListsEfl.txt:109
> +        platform/graphics/cairo/TiledBackingStoreBackendCairo.cpp

This patch doesn't include "TiledBackingStoreBackendCairo.cpp" file. You need to add this file or remove this from CMakeListsEfl.txt.
Comment 7 Tomasz Morawski 2011-11-03 00:20:20 PDT
(In reply to comment #6)
> (From update of attachment 113302 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=113302&action=review
> 
> > Source/WebCore/CMakeListsEfl.txt:109
> > +        platform/graphics/cairo/TiledBackingStoreBackendCairo.cpp
> 
> This patch doesn't include "TiledBackingStoreBackendCairo.cpp" file. You need to add this file or remove this from CMakeListsEfl.txt.
Could you please tell me why? This patch depends on patch 71350 which added few new files. This means that missing files will revel when the 71350 patch will merged. The "depends on" also means that this patch can not be merged and builded without 71350. So, I don't think that I should remove anythink from CMakeListEfl.txt file.
Comment 8 Tomasz Morawski 2011-11-04 00:54:08 PDT
Created attachment 113631 [details]
Updated according to Grzegorz's suggestions
Comment 9 Gyuyoung Kim 2011-11-04 01:15:13 PDT
Comment on attachment 113631 [details]
Updated according to Grzegorz's suggestions

Attachment 113631 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/10318069
Comment 10 Raphael Kubo da Costa (:rakuco) 2011-11-04 07:48:39 PDT
Comment on attachment 113631 [details]
Updated according to Grzegorz's suggestions

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

I'd use "backing_store" instead of "backingstore" in the function names as they are separate words.

I'd also like to know how this is going to interact with the tiled backing store implementation in ewk (the API added to ewk_view looks weird if one is using it instead of the single backing store, to start with).

> ChangeLog:3
> +        [EFL] Port tiled backing store

This sounds a bit misleading: the EFL port already has a tiled backing store; I'd rephrase it as "Make it possible to use WebCore's tiled backing store in EFL" or something like that.

> Source/WebCore/CMakeListsEfl.txt:106
> +  IF (WTF_USE_TILED_BACKING_STORE)

Isn't it always on?

> Source/WebCore/CMakeListsEfl.txt:150
> +IF (WTF_USE_TILED_BACKING_STORE)

Ditto.

> Source/WebKit/efl/ChangeLog:8
> +        Ported tiled backing store to EFL WebKit port.

Same thing about the misleading phrase.

> Source/WebKit/efl/ChangeLog:14
> +        Major changes:
> +        - Added new functions to ChromeClientEfl
> +        - Added new functions to ewk_view
> +        - From ewk_view_single calls
> +          ewk_view_setting_tiled_backingstore_enable_set to enable/disable
> +          use of tiled backing store

This is already evident from the lines below; it'd be good to know the motivation and how this interacts with the tiled backing store implementation in ewk.

> Source/WebKit/efl/WebCoreSupport/ChromeClientEfl.cpp:67
> +#if USE(TILED_BACKING_STORE)

Isn't it always on?

> Source/WebKit/efl/WebCoreSupport/ChromeClientEfl.cpp:441
> +    WebCore::FloatRect rect = ewk_view_page_rect_get(m_view);
> +    Evas_Object* frame = ewk_view_frame_main_get(m_view);

These can be const.

> Source/WebKit/efl/WebCoreSupport/ChromeClientEfl.cpp:516
> +    Eina_Rectangle area;
> +    EINA_RECTANGLE_SET(&area, updateRect.x(), updateRect.y(), updateRect.width(), updateRect.height());
> +
> +    ewk_view_backingstore_invalidate(m_view, &area);

This is a private function, you could just pass the IntRect to tehe function.
Comment 11 Tomasz Morawski 2011-11-08 00:46:45 PST
Created attachment 114005 [details]
Updated according to suggestions
Comment 12 Gyuyoung Kim 2011-11-08 00:50:11 PST
Comment on attachment 114005 [details]
Updated according to suggestions 

Attachment 114005 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/10375021
Comment 13 Tomasz Morawski 2011-11-08 00:55:58 PST
> I'd also like to know how this is going to interact with the tiled backing store implementation in ewk (the API added to ewk_view looks weird if one is using it instead of the single backing store, to start with).
Hi, 
Thank you for your review. I have uploaded a new version of patch. 
No, this patch contains WebCore's tiled backing store implementation. This implementation should not interact with the old ewk implementation. I agree with you according to API names. They are not good. Do you have any proposition of API name changes to not mix backing stores implementations? Do you think that it is good idea to enabled this feature for ewk_view_single as it is currently done? I would like to discuss this. 

>> +#if USE(TILED_BACKING_STORE)
> Isn't it always on?
No, this flag is defined in CMakeList file so, it could be disabled.
Comment 14 Raphael Kubo da Costa (:rakuco) 2011-11-11 15:09:24 PST
Comment on attachment 114005 [details]
Updated according to suggestions 

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

> Source/WebKit/efl/ewk/ewk_private.h:165
> +void ewk_view_backing_store_invalidate(Evas_Object* o, const WebCore::IntRect& area);
> +void ewk_view_backing_store_adjuct_visible_rect(Evas_Object* o);
> +Eina_Bool ewk_view_setting_tiled_backing_store_enable_set(Evas_Object* o, Eina_Bool enable);

I think you can omit "o" as a parameter name, and the Eina_Bool can be a simple bool.

> Source/WebKit/efl/ewk/ewk_view.cpp:2288
> +#if USE(TILED_BACKING_STORE)

I think these #if checks should be moved to the function bodies, so the function symbols are always present.

> Source/WebKit/efl/ewk/ewk_view_single.cpp:56
> +    // use WebCore's backing store

Unneeded.

> Source/cmake/OptionsEfl.cmake:52
> +SET(WTF_USE_TILED_BACKING_STORE 1)
> +ADD_DEFINITIONS(-DWTF_USE_TILED_BACKING_STORE=1)

Thinking about this again, wouldn't it be better to make it an actual option? This way if one uses the build-webkit script the --tiled-backing-store can have an effect. It would also make the #if USE(TILED_BACKING_STORE) checks make sense, as right now they check for something that is always on.
Comment 15 Raphael Kubo da Costa (:rakuco) 2011-11-11 15:13:55 PST
(In reply to comment #13) 
> Do you think that it is good idea to enabled this feature for ewk_view_single as it is currently done? I would like to discuss this.

It all boils down to whether ewk's tiled backing store still works as expected and if there are any gains to enabling this in the single backing store. Running the layout tests would be very helpful to make sure there are no regressions related to the patch.

> >> +#if USE(TILED_BACKING_STORE)
> > Isn't it always on?
> No, this flag is defined in CMakeList file so, it could be disabled.

You set it with SET(...) and then you call ADD_DEFINITION(...). The only way this can be disabled is if one manually edits OptionsEfl.cmake; if one does so, it's not really our problem.
Comment 16 Tomasz Morawski 2011-11-14 05:43:30 PST
(In reply to comment #14)
> (From update of attachment 114005 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=114005&action=review
> 
> > Source/WebKit/efl/ewk/ewk_private.h:165
> > +void ewk_view_backing_store_invalidate(Evas_Object* o, const WebCore::IntRect& area);
> > +void ewk_view_backing_store_adjuct_visible_rect(Evas_Object* o);
> > +Eina_Bool ewk_view_setting_tiled_backing_store_enable_set(Evas_Object* o, Eina_Bool enable);
> 
> I think you can omit "o" as a parameter name, and the Eina_Bool can be a simple bool.
OK, I will change that. Anyway, removing only "o" parameter will look strange for me. Maybe I will raname "o" to "ewkView"? 

> > Source/WebKit/efl/ewk/ewk_view.cpp:2288
> > +#if USE(TILED_BACKING_STORE)
> 
> I think these #if checks should be moved to the function bodies, so the function symbols are always present.
Due to that the Tiled Backing Store support has to be compiled into the WebCore and these functions are not in public API. I think there is no need to move these #if checks to functions body. 

> > Source/cmake/OptionsEfl.cmake:52
> > +SET(WTF_USE_TILED_BACKING_STORE 1)
> > +ADD_DEFINITIONS(-DWTF_USE_TILED_BACKING_STORE=1)
> 
> Thinking about this again, wouldn't it be better to make it an actual option? This way if one uses the build-webkit script the --tiled-backing-store can have an effect. It would also make the #if USE(TILED_BACKING_STORE) checks make sense, as right now they check for something that is always on.
Yes, it can be done. 

> >> +#if USE(TILED_BACKING_STORE)
> > Isn't it always on?
> No, this flag is defined in CMakeList file so, it could be disabled.
You set it with SET(...) and then you call ADD_DEFINITION(...). The only way this can be disabled is if one manually edits OptionsEfl.cmake; if one does so, it's not really our problem.

I have make this like cairo option:
SET(WTF_USE_CAIRO 1)
ADD_DEFINITIONS(-DWTF_USE_CAIRO=1)
Comment 17 Tomasz Morawski 2011-11-16 00:14:52 PST
Created attachment 115339 [details]
Updated

Updated according to suggestions. Added additional description to ChangeLogs.
Comment 18 Raphael Kubo da Costa (:rakuco) 2011-11-16 06:03:59 PST
The code looks OK (you might also want to change the default option value in Tools/Scripts/build-webkit too).

However, I'm still waiting for an answer for the other part of my comment #15: are there performance gains with this feature? Have the layout tests been run so there are no regressions?
Comment 19 Tomasz Morawski 2011-11-16 06:32:11 PST
(In reply to comment #18)
> The code looks OK (you might also want to change the default option value in Tools/Scripts/build-webkit too).
I will change this.
 
> However, I'm still waiting for an answer for the other part of my comment #15: are there performance gains with this feature? Have the layout tests been run so there are no regressions?
I am sorry for lack of response for my side. 

Currently I don't have any performance test done yet. So, I can give you any answer for your question now. I have plan to do such tests till end of December. Would you like to get result of such tests?

No, there was no layout test run for this. This patch and 71350 patch doesn't change any code related to layout mechanism. There was not change in backing store class that fully controls all invalidation and drawing functionality.
It is a good idea to do this. Thank you. I will run layout tests and put result on this bugs.
Comment 20 Raphael Kubo da Costa (:rakuco) 2011-11-16 06:59:08 PST
(In reply to comment #19)
> Currently I don't have any performance test done yet. So, I can give you any answer for your question now. I have plan to do such tests till end of December. Would you like to get result of such tests?

It'd be good if you could paste them or attach them to this report. I just have something like "I am proposing this change because it improves X and Y by Z%" in mind.
Comment 21 Tomasz Morawski 2011-11-16 07:38:00 PST
(In reply to comment #20)
> (In reply to comment #19)
> > Currently I don't have any performance test done yet. So, I can give you any answer for your question now. I have plan to do such tests till end of December. Would you like to get result of such tests?
> 
> It'd be good if you could paste them or attach them to this report. I just have something like "I am proposing this change because it improves X and Y by Z%" in mind.

Yes, I understand that it could be a good. But I have not written this patch to prove that ewk's tbs is worse or better then WebCore's tbs. Anyway, what if the webcore's tbs will be worse? I can not write "I am proposing this change because it decrease X and Y by Z%" :) Should we reject this patch in that situation? I have only written this patch to get support of WebCore's backing store and allows to use the genernic WebCore's backing store in the EFL port. I thnik the end user of WebKit's EFL port should decided what backing store should use if any.
Comment 22 Raphael Kubo da Costa (:rakuco) 2011-11-16 08:06:33 PST
(In reply to comment #21)
> Yes, I understand that it could be a good. But I have not written this patch to prove that ewk's tbs is worse or better then WebCore's tbs. Anyway, what if the webcore's tbs will be worse? I can not write "I am proposing this change because it decrease X and Y by Z%" :) Should we reject this patch in that situation? I have only written this patch to get support of WebCore's backing store and allows to use the genernic WebCore's backing store in the EFL port. I thnik the end user of WebKit's EFL port should decided what backing store should use if any.

I see. Now that the latest patch version allows one to enable and disable WebCore's tiled backing store at will this is indeed true.

As for the build-webkit change, it might be better to leave it off by default until it can be shown that it performs better than the current implementation, IMO.
Comment 23 Tomasz Morawski 2011-11-16 23:15:51 PST
> As for the build-webkit change, it might be better to leave it off by default until it can be shown that it performs better than the current implementation, IMO.
Yes, it could be a good solution. Moreover, it will allow to continue work on WebCore's tbs with use of our EFL port in more convenient way. I have only two concerns about this. I don't even know if this patch can be compiled on EFL buildbot due to lack of knowelage about libcairo version installed on it. And the second one our port could has unnoticed build break if somebody change WebCore's tbs client API (due to the EFL buildbot will not compile disabled code).
Comment 24 Raphael Kubo da Costa (:rakuco) 2011-11-17 04:12:04 PST
(In reply to comment #23)
> I have only two concerns about this.

> I don't even know if this patch can be compiled on EFL buildbot due to lack of knowelage about libcairo version installed on it.

Does it require a very recent cairo version to work? Does it fail in older ones because it needs newer API or due to rendering failures?

We can ask the people who take care of the buildbot about that. Gyuyoung, are you there?

> And the second one our port could has unnoticed build break if somebody change WebCore's tbs client API (due to the EFL buildbot will not compile disabled code).

That's true to any code path that depends on features which are disabled by default. Until we think of a better way to deal with this, I can only recommend updating your checkout frequently and making sure the code still builds.
Comment 25 Tomasz Morawski 2011-11-17 04:48:25 PST
(In reply to comment #24)
> (In reply to comment #23)
> > I have only two concerns about this.
> 
> > I don't even know if this patch can be compiled on EFL buildbot due to lack of knowelage about libcairo version installed on it.
> 
> Does it require a very recent cairo version to work? Does it fail in older ones because it needs newer API or due to rendering failures?
No, no fail. The older version then 1.10 doesn't contain cairo_region_* API that is used in TileCairo. I have used recent version 1.10.2 that is almost one year old. 

Thank you.
Comment 26 Raphael Kubo da Costa (:rakuco) 2011-11-17 04:54:15 PST
AFAIR we raised the minimum version we required to 1.10 some time ago due to the cairo_region API, so there's no need to worry about that.
Comment 27 KwangHyuk 2011-12-19 22:42:45 PST
View in context: https://bugs.webkit.org/attachment.cgi?id=115339&action=review

> Source/WebKit/efl/ewk/ewk_private.h:164
> +void ewk_view_backing_store_adjuct_visible_rect(Evas_Object*);

May be adjuct replaced with adjust ?

> Source/WebKit/efl/ewk/ewk_view.cpp:2302
> +    EWK_VIEW_PRIV_GET_OR_RETURN(smartData, priv, EINA_FALSE);

Currently we are using false instead of EINA_FALSE.

> Source/WebKit/efl/ewk/ewk_view.cpp:2684
> +void ewk_view_backing_store_adjuct_visible_rect(Evas_Object* ewkView)

May be adjuct replaced with adjust ?

> Source/WebKit/efl/ewk/ewk_view_single.cpp:56
> +    ewk_view_setting_tiled_backing_store_enable_set(ewkView, EINA_TRUE);

Currently we are using true instead of EINA_TRUE.
Comment 28 Tomasz Morawski 2011-12-20 02:03:55 PST
Created attachment 119998 [details]
Rebased and updated according to KwangHyuk suggestions

Additional change:
in ewk_view_single.cpp:56
Call the ewk_view_setting_tiled_backing_store_enable_set function with true parameter (was false) in _ewk_view_single_smart_add function. Changed this due to that the feature can be only enabled when the --tiled-backing-store compilation flag is used.
Comment 29 KwangHyuk 2011-12-20 05:55:16 PST
> Source/WebCore/PlatformEfl.cmake:108
> +  IF (WTF_USE_TILED_BACKING_STORE)

Would you check whether WTF_USE_CAIRO would be required for this cairo related files ?

> Source/WebKit/efl/ewk/ewk_private.h:160
> +Eina_Bool ewk_view_setting_tiled_backing_store_enable_set(Evas_Object* ewkView, bool enable);

As it's containing word "tiled", ewk_view_setting_tiled_backing_store_enable_set looks little different from above two APIs
Was it intended ?

> Source/WebKit/efl/ewk/ewk_view.cpp:2296
> +    return EINA_TRUE;

one more EINA_TRUE. :)
Comment 30 Tomasz Morawski 2011-12-20 06:11:24 PST
(In reply to comment #29)
> > Source/WebCore/PlatformEfl.cmake:108
> > +  IF (WTF_USE_TILED_BACKING_STORE)
> 
> Would you check whether WTF_USE_CAIRO would be required for this cairo related files ?
This "if" is inside IF (WTF_USE_CAIRO) block. Please see: Source/WebCore/PlatformEfl.cmake:84. So, there is no need to add new flag check.

Other things will be fixed.
Comment 31 Tomasz Morawski 2011-12-21 01:16:32 PST
Created attachment 120159 [details]
Updated
Comment 32 KwangHyuk 2011-12-21 07:12:50 PST
I don't think that this patch is perfect.
But, it would be helpful for the ewk_single_view. :)

LGTM.
Comment 33 Tomasz Morawski 2011-12-22 01:57:13 PST
(In reply to comment #18)
> Have the layout tests been run so there are no regressions?
I have made the layout test for disabled and enabled feature. There are not diffrences in result: 8034 tests ran as expected, 500 didn't (19028 didn't run) for both cases. This patch doesn't change layout processing.
Comment 34 Gyuyoung Kim 2012-01-05 02:18:40 PST
It looks this patch adhere efl port coding style rule well. I think this patch needs to be reviewed by tiled backing store's reviewer as well. It seems Simon Hausmann reviewed tiled backing store.
Comment 35 KwangHyuk 2012-01-11 23:43:49 PST
I could notice small things.

> ChangeLog:3
> +        [EFL] Use WebCore's tiled backing store in WebKit EFL port

Add '.' into end of title.

> Source/WebCore/ChangeLog:3
> +        [EFL] Use WebCore's tiled backing store in WebKit EFL port

Ditto.

> Source/WebKit/efl/ChangeLog:3
> +        [EFL] Use WebCore's tiled backing store in WebKit EFL port

Ditto.

> Source/WebKit/efl/WebCoreSupport/ChromeClientEfl.cpp:440
> +    int x, y;

Move this close to line 443.
Comment 36 Gyuyoung Kim 2012-08-27 18:47:43 PDT
Comment on attachment 120159 [details]
Updated

Cleared review? from attachment 120159 [details] so that this bug does not appear in http://webkit.org/pending-review.  If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).
Comment 37 Ryuan Choi 2012-11-10 06:46:46 PST
Do you have any plan to update this?

Now, WK2/Efl has WebCore's tiled backingstore.

I hope that WK/Efl also use it instead of ewk_view_tiled.

If you have plan, I will try it.
Comment 38 Ryuan Choi 2012-11-10 06:47:32 PST
(In reply to comment #37)
> Do you have any plan to update this?
> 
> Now, WK2/Efl has WebCore's tiled backingstore.
> 
> I hope that WK/Efl also use it instead of ewk_view_tiled.
> 
> If you have plan, I will try it.

s/have/don't have/
Comment 39 Gyuyoung Kim 2012-11-11 18:04:01 PST
(In reply to comment #38)
> (In reply to comment #37)
> > Do you have any plan to update this?
> > 
> > Now, WK2/Efl has WebCore's tiled backingstore.
> > 
> > I hope that WK/Efl also use it instead of ewk_view_tiled.
> > 
> > If you have plan, I will try it.
> 
> s/have/don't have/

I heard that Tomasz had been focused on other project. If there is no response, you can take over this patch.
Comment 40 Ryuan Choi 2012-11-14 18:18:13 PST
Created attachment 174309 [details]
Patch
Comment 41 Gyuyoung Kim 2012-11-14 18:22:04 PST
Comment on attachment 174309 [details]
Patch

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

> Source/WebKit/efl/WebCoreSupport/ChromeClientEfl.cpp:664
> +    int x, y;

Please move this to 667 line.

> Source/WebKit/efl/ewk/ewk_view.h:2780
> + * Enables/disables the WebCore's tiled backing store

Missing . at the end of line.

> Source/WebKit/efl/ewk/ewk_view.h:2782
> + * @param o view object.

We don't use . in @ fields.
Comment 42 Ryuan Choi 2012-11-14 18:41:26 PST
Created attachment 174315 [details]
Patch
Comment 43 Ryuan Choi 2012-11-14 18:43:54 PST
(In reply to comment #41)
> (From update of attachment 174309 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=174309&action=review
> 
> > Source/WebKit/efl/WebCoreSupport/ChromeClientEfl.cpp:664
> > +    int x, y;
> 
> Please move this to 667 line.
> 
> > Source/WebKit/efl/ewk/ewk_view.h:2780
> > + * Enables/disables the WebCore's tiled backing store
> 
> Missing . at the end of line.
> 
> > Source/WebKit/efl/ewk/ewk_view.h:2782
> > + * @param o view object.
> 
> We don't use . in @ fields.

Done
Comment 44 WebKit Review Bot 2012-11-14 19:25:52 PST
Comment on attachment 174315 [details]
Patch

Attachment 174315 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14847217

New failing tests:
inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html
Comment 45 Ryuan Choi 2012-11-14 21:58:01 PST
Created attachment 174343 [details]
patch for land
Comment 46 Ryuan Choi 2012-11-14 22:06:45 PST
(In reply to comment #44)
> (From update of attachment 174315 [details])
> Attachment 174315 [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/14847217
> 
> New failing tests:
> inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html

I think that it is not related this bug because I jsut touched efl side.

Anyway, I rebased to see the green bot.
Comment 47 WebKit Review Bot 2012-11-14 22:45:06 PST
Comment on attachment 174343 [details]
patch for land

Clearing flags on attachment: 174343

Committed r134743: <http://trac.webkit.org/changeset/134743>
Comment 48 WebKit Review Bot 2012-11-14 22:45:13 PST
All reviewed patches have been landed.  Closing bug.