63241 2011-06-23 03:10:31 -0700 [EFL] Remove overlapping recursive layout function 2011-08-02 08:18:50 -0700 1 1 1 Unclassified WebKit WebKit EFL 528+ (Nightly build) Other Linux RESOLVED FIXED P2 Normal --- 0 eunsol47.park webkit-unassigned antognolli+webkit eunsol47.park gyuyoung.kim gyuyoung.kim hyuki.kim kenneth leandro lucas.de.marchi rakuco ryuan.choi tonikitoo webkit.review.bot oldest_to_newest 426058 0 eunsol47.park 2011-06-23 03:10:31 -0700 In tiled backing store status, the recursive layout function was called twice in _ewk_view_smart_calculate and _ewk_view_tiled_updates_process_pre. It made unnecessary layouting repeatedly, so I think the call in _ewk_view_smart_calculate have to be removed. But in single backing store, the call have to be added before repainting. So, the call will be added in _ewk_view_single_smart_repaints_process. 426065 1 98333 eunsol47.park 2011-06-23 03:34:45 -0700 Created attachment 98333 proposed patch upload patch 426066 2 gyuyoung.kim 2011-06-23 03:41:59 -0700 LGTM. Rafael, how do you think ? 426093 3 hyuki.kim 2011-06-23 05:17:14 -0700 This patch would reduce the number of forceful layout from ewk and I agree that layout better to be called close to painting time. 427823 4 ryuan.choi 2011-06-27 02:53:55 -0700 Now, FrameView::layout is called every _ewk_view_smart_calculate called. but I believe that it should be called when it really required. Especially, it will be called frequently although we suspend rendering for faster scrolling. So, I think that this patch is way to go. 428081 5 98333 leandro 2011-06-27 10:41:30 -0700 Comment on attachment 98333 proposed patch Informal r+. 428403 6 98333 tonikitoo 2011-06-27 15:51:30 -0700 Comment on attachment 98333 proposed patch Eunsol, could you say what problem the patch fixes? 428516 7 eunsol47.park 2011-06-27 17:57:18 -0700 As I commented, the function was called twice without the need. It can make scrolling slow. And it also can make other operations slow especially in heavy page. It is critical problem to use. 428523 8 tonikitoo 2011-06-27 18:02:54 -0700 It was just strange that a XXXIfNeeded was being executed needlessly. 428547 9 eunsol47.park 2011-06-27 18:28:05 -0700 When paintsEntireContents() is TRUE, the layout is needed more frequently. 438396 10 eunsol47.park 2011-07-18 03:49:57 -0700 Hello, I explained more specifically. First, The recursive layout is called twice in _ewk_view_smart_calculate and _ewk_view_tiled_updates_process_pre, so it has no problem removing one of them. Second, while panning, it is in the rendering suspend status to be fast in EFL port. In the status, it doesn't want to be repainted newly. _ewk_view_smart_calculate can be called from "webcore- invalidate" when changing the page despite the suspend status, because it is a ewk's status, webcore doesn't know it. (Especially, _ewk_view_smart_calculate is called more frequently when paintsEntireContents() is TRUE.) So, I think the recursive layout is unnecessary in _ewk_view_smart_calculate. Thank you. 444522 11 98333 tonikitoo 2011-07-31 22:13:46 -0700 Comment on attachment 98333 proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=98333&action=review Eunsol, could you say what problem the patch fixes? > Source/WebKit/efl/ChangeLog:9 > + ewk_view_layout_if_needed_recursive was called twice in tiled backing store, > + so it was removed. And in single backing store, it was needed and added. It does not describe what is the problem or what it fixes in practice. Could you be more wordy? There are some unanswered questions as well... 445083 12 102612 eunsol47.park 2011-08-01 19:09:41 -0700 Created attachment 102612 proposed patch 445084 13 102612 eunsol47.park 2011-08-01 19:10:53 -0700 Comment on attachment 102612 proposed patch I modified the changelog. 445212 14 rakuco 2011-08-02 05:33:54 -0700 You also need to mark the previous patch as obsolete. 445265 15 102612 tonikitoo 2011-08-02 07:59:39 -0700 Comment on attachment 102612 proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=102612&action=review > Source/WebKit/efl/ChangeLog:7 > + and _ewk_view_tiled_updates_process_pre,so it is no problem removing one of them. s/,so/, so/ > Source/WebKit/efl/ChangeLog:8 > + Second, In tiled backingstore configuration, _ewk_view_layout_if_needed_recursive called s/In/in 445275 16 102612 webkit.review.bot 2011-08-02 08:18:44 -0700 Comment on attachment 102612 proposed patch Clearing flags on attachment: 102612 Committed r92190: <http://trac.webkit.org/changeset/92190> 445276 17 webkit.review.bot 2011-08-02 08:18:50 -0700 All reviewed patches have been landed. Closing bug. 98333 2011-06-23 03:34:45 -0700 2011-08-02 05:39:18 -0700 proposed patch recursiveLayout.patch text/plain 1840 eunsol47.park ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvZWZsL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQv ZWZsL0NoYW5nZUxvZwppbmRleCA4ODA0MDY1Li44ZmY2NWEyIDEwMDc1NQotLS0gYS9Tb3VyY2Uv V2ViS2l0L2VmbC9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdC9lZmwvQ2hhbmdlTG9nCkBA IC0xLDMgKzEsMTggQEAKKzIwMTEtMDYtMjMgIEV1bnNvbCBQYXJrICA8ZXVuc29sNDcucGFya0Bz YW1zdW5nLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAg ICAgICBbRUZMXSBSZW1vdmUgb3ZlcmxhcHBpbmcgcmVjdXJzaXZlIGxheW91dCBmdW5jdGlvbgor ICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NjMyNDEKKwor ICAgICAgICBld2tfdmlld19sYXlvdXRfaWZfbmVlZGVkX3JlY3Vyc2l2ZSB3YXMgY2FsbGVkIHR3 aWNlIGluIHRpbGVkIGJhY2tpbmcgc3RvcmUsIAorICAgICAgICBzbyBpdCB3YXMgcmVtb3ZlZC4g QW5kIGluIHNpbmdsZSBiYWNraW5nIHN0b3JlLCBpdCB3YXMgbmVlZGVkIGFuZCBhZGRlZC4KKwor ICAgICAgICAqIGV3ay9ld2tfdmlldy5jcHA6CisgICAgICAgIChfZXdrX3ZpZXdfc21hcnRfY2Fs Y3VsYXRlKTpSZW1vdmUgZXdrX3ZpZXdfbGF5b3V0X2lmX25lZWRlZF9yZWN1cnNpdmUKKyAgICAg ICAgKiBld2svZXdrX3ZpZXdfc2luZ2xlLmM6CisgICAgICAgIChfZXdrX3ZpZXdfc2luZ2xlX3Nt YXJ0X3JlcGFpbnRzX3Byb2Nlc3MpOkFkZCBld2tfdmlld19sYXlvdXRfaWZfbmVlZGVkX3JlY3Vy c2l2ZQorCiAyMDExLTA2LTIyICBSeXVhbiBDaG9pICA8cnl1YW4uY2hvaUBzYW1zdW5nLmNvbT4K IAogICAgICAgICBSZXZpZXdlZCBieSBBbnRvbmlvIEdvbWVzLgpkaWZmIC0tZ2l0IGEvU291cmNl L1dlYktpdC9lZmwvZXdrL2V3a192aWV3LmNwcCBiL1NvdXJjZS9XZWJLaXQvZWZsL2V3ay9ld2tf dmlldy5jcHAKaW5kZXggMGZmMWQ0Ni4uYThkNzU3ZCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktp dC9lZmwvZXdrL2V3a192aWV3LmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0L2VmbC9ld2svZXdrX3Zp ZXcuY3BwCkBAIC04NTIsNyArODUyLDYgQEAgc3RhdGljIHZvaWQgX2V3a192aWV3X3NtYXJ0X2Nh bGN1bGF0ZShFdmFzX09iamVjdCogbykKICAgICB9CiAgICAgc2QtPmNoYW5nZWQucG9zaXRpb24g PSBFSU5BX0ZBTFNFOwogCi0gICAgZXdrX3ZpZXdfbGF5b3V0X2lmX25lZWRlZF9yZWN1cnNpdmUo c2QtPl9wcml2KTsKIAogICAgIGlmICghc2QtPmFwaS0+c2Nyb2xsc19wcm9jZXNzKHNkKSkKICAg ICAgICAgRVJSKCJmYWlsZWQgdG8gcHJvY2VzcyBzY3JvbGxzLiIpOwpkaWZmIC0tZ2l0IGEvU291 cmNlL1dlYktpdC9lZmwvZXdrL2V3a192aWV3X3NpbmdsZS5jIGIvU291cmNlL1dlYktpdC9lZmwv ZXdrL2V3a192aWV3X3NpbmdsZS5jCmluZGV4IDIxOTc0YmMuLjU1OWQyNjYgMTAwNzU1Ci0tLSBh L1NvdXJjZS9XZWJLaXQvZWZsL2V3ay9ld2tfdmlld19zaW5nbGUuYworKysgYi9Tb3VyY2UvV2Vi S2l0L2VmbC9ld2svZXdrX3ZpZXdfc2luZ2xlLmMKQEAgLTQzNyw2ICs0MzcsOCBAQCBzdGF0aWMg RWluYV9Cb29sIF9ld2tfdmlld19zaW5nbGVfc21hcnRfcmVwYWludHNfcHJvY2VzcyhFd2tfVmll d19TbWFydF9EYXRhICpzZAogICAgICAgICBnb3RvIGVycm9yX2l0ZXJhdG9yOwogICAgIH0KIAor ICAgIGV3a192aWV3X2xheW91dF9pZl9uZWVkZWRfcmVjdXJzaXZlKHNkLT5fcHJpdik7CisKICAg ICBpbnQgc3gsIHN5OwogICAgIGV3a19mcmFtZV9zY3JvbGxfcG9zX2dldChzZC0+bWFpbl9mcmFt ZSwgJnN4LCAmc3kpOwogCg== 102612 2011-08-01 19:09:41 -0700 2011-08-02 08:18:44 -0700 proposed patch recursive_layout.patch text/plain 2474 eunsol47.park ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvZWZsL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQv ZWZsL0NoYW5nZUxvZwppbmRleCA1MTliNzllLi4yNzA1ODVmIDEwMDc1NQotLS0gYS9Tb3VyY2Uv V2ViS2l0L2VmbC9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdC9lZmwvQ2hhbmdlTG9nCkBA IC0xLDMgKzEsMjUgQEAKKzIwMTEtMDgtMDEgIEV1bnNvbCBQYXJrICA8ZXVuc29sNDcucGFya0Bz YW1zdW5nLmNvbT4KKworICAgICAgICBbRUZMXSBSZW1vdmUgb3ZlcmxhcHBpbmcgcmVjdXJzaXZl IGxheW91dCBmdW5jdGlvbiAKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTYzMjQxIAorCisgICAgICAgIEZpcnN0LCBfZXdrX3ZpZXdfbGF5b3V0X2lmX25l ZWRlZF9yZWN1cnNpdmUgaXMgY2FsbGVkIHR3aWNlIGluIF9ld2tfdmlld19zbWFydF9jYWxjdWxh dGUgCisgICAgICAgIGFuZCBfZXdrX3ZpZXdfdGlsZWRfdXBkYXRlc19wcm9jZXNzX3ByZSxzbyBp dCBpcyBubyBwcm9ibGVtIHJlbW92aW5nIG9uZSBvZiB0aGVtLgorICAgICAgICBTZWNvbmQsIElu IHRpbGVkIGJhY2tpbmdzdG9yZSBjb25maWd1cmF0aW9uLCBfZXdrX3ZpZXdfbGF5b3V0X2lmX25l ZWRlZF9yZWN1cnNpdmUgY2FsbGVkIAorICAgICAgICBpbiBfZXdrX3ZpZXdfc21hcnRfY2FsY3Vs YXRlIHdpbGwgcmVzdWx0IGluIGdlbmVyYXRpbmcgcmVwYWludHMgZm9yIGRpcnR5IGFyZWFzIAor ICAgICAgICBsb2NhdGVkIG91dHNpZGUgb2Ygdmlld3BvcnQgc2luY2UgbV9wYWludEVudGlyZUNv bnRlbnRzIGlzIHNldC4KKyAgICAgICAgQnV0LCB0aGUgYXJlYXMgd2lsbCBub3QgYmUgYWN0dWFs bHkgcGFpbnRlZC4gVGh1cywgdGhlIGxheW91dCBpcyB1bm5lY2Vzc2FyeSBleGNlcHQKKyAgICAg ICAgd2hlbiB0aGVyZSBpcyBzb21ldGhpbmcgdG8gcGFpbnQgaW5zaWRlIG9mIHZpZXdwb3J0LCB3 aGljaCB3aWxsIGNvbnN1bWUgQ1BVIHVubmVjZXNzYXJpbHkuCisgICAgICAgIFNvLHRoZSBsYXlv dXQgZnVuYyB3YXMgcmVtb3ZlZCBmcm9tIF9ld2tfdmlld19zbWFydF9jYWxjdWxhdGUgCisgICAg ICAgIGFuZCBhZGRlZCB0byBfZXdrX3ZpZXdfc2luZ2xlX3NtYXJ0X3JlcGFpbnRzX3Byb2Nlc3Mg Zm9yIHNpbmdsZSBiYWNraW5nc3RvcmUuCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChP T1BTISkuCisKKyAgICAgICAgKiBld2svZXdrX3ZpZXcuY3BwOgorICAgICAgICAoX2V3a192aWV3 X3NtYXJ0X2NhbGN1bGF0ZSk6CisgICAgICAgICogZXdrL2V3a192aWV3X3NpbmdsZS5jOgorICAg ICAgICAoX2V3a192aWV3X3NpbmdsZV9zbWFydF9yZXBhaW50c19wcm9jZXNzKToKKwogMjAxMS0w Ny0yOSAgTWljaGFsIFBha3VsYSB2ZWwgUnV0a2EgIDxtLnBha3VsYUBzYW1zdW5nLmNvbT4KIAog ICAgICAgICBbRUZMXSBJbXBsZW1lbnQgRWRpdG9yQ2xpZW50RWZsOjpyZXNwb25kVG9DaGFuZ2Vk Q29udGVudHMKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvZWZsL2V3ay9ld2tfdmlldy5jcHAg Yi9Tb3VyY2UvV2ViS2l0L2VmbC9ld2svZXdrX3ZpZXcuY3BwCmluZGV4IDUwNmI0M2IuLjA5OTRi ZDAgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvZWZsL2V3ay9ld2tfdmlldy5jcHAKKysrIGIv U291cmNlL1dlYktpdC9lZmwvZXdrL2V3a192aWV3LmNwcApAQCAtODUzLDggKzg1Myw2IEBAIHN0 YXRpYyB2b2lkIF9ld2tfdmlld19zbWFydF9jYWxjdWxhdGUoRXZhc19PYmplY3QgKm8pCiAgICAg fQogICAgIHNkLT5jaGFuZ2VkLnBvc2l0aW9uID0gRUlOQV9GQUxTRTsKIAotICAgIGV3a192aWV3 X2xheW91dF9pZl9uZWVkZWRfcmVjdXJzaXZlKHNkLT5fcHJpdik7Ci0KICAgICBpZiAoIXNkLT5h cGktPnNjcm9sbHNfcHJvY2VzcyhzZCkpCiAgICAgICAgIEVSUigiZmFpbGVkIHRvIHByb2Nlc3Mg c2Nyb2xscy4iKTsKICAgICBfZXdrX3ZpZXdfc2Nyb2xsc19mbHVzaChwcml2KTsKZGlmZiAtLWdp dCBhL1NvdXJjZS9XZWJLaXQvZWZsL2V3ay9ld2tfdmlld19zaW5nbGUuYyBiL1NvdXJjZS9XZWJL aXQvZWZsL2V3ay9ld2tfdmlld19zaW5nbGUuYwppbmRleCBkOTZkNWU3Li4wZTU1MTgxIDEwMDY0 NAotLS0gYS9Tb3VyY2UvV2ViS2l0L2VmbC9ld2svZXdrX3ZpZXdfc2luZ2xlLmMKKysrIGIvU291 cmNlL1dlYktpdC9lZmwvZXdrL2V3a192aWV3X3NpbmdsZS5jCkBAIC00MzcsNiArNDM3LDggQEAg c3RhdGljIEVpbmFfQm9vbCBfZXdrX3ZpZXdfc2luZ2xlX3NtYXJ0X3JlcGFpbnRzX3Byb2Nlc3Mo RXdrX1ZpZXdfU21hcnRfRGF0YSAqc2QKICAgICAgICAgZ290byBlcnJvcl9pdGVyYXRvcjsKICAg ICB9CiAKKyAgICBld2tfdmlld19sYXlvdXRfaWZfbmVlZGVkX3JlY3Vyc2l2ZShzZC0+X3ByaXYp OworCiAgICAgaW50IHN4LCBzeTsKICAgICBld2tfZnJhbWVfc2Nyb2xsX3Bvc19nZXQoc2QtPm1h aW5fZnJhbWUsICZzeCwgJnN5KTsKIAo=