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.
Created attachment 98333 [details] proposed patch upload patch
LGTM. Rafael, how do you think ?
This patch would reduce the number of forceful layout from ewk and I agree that layout better to be called close to painting time.
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.
Comment on attachment 98333 [details] proposed patch Informal r+.
Comment on attachment 98333 [details] proposed patch Eunsol, could you say what problem the patch fixes?
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.
It was just strange that a XXXIfNeeded was being executed needlessly.
When paintsEntireContents() is TRUE, the layout is needed more frequently.
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.
Comment on attachment 98333 [details] 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...
Created attachment 102612 [details] proposed patch
Comment on attachment 102612 [details] proposed patch I modified the changelog.
You also need to mark the previous patch as obsolete.
Comment on attachment 102612 [details] 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
Comment on attachment 102612 [details] proposed patch Clearing flags on attachment: 102612 Committed r92190: <http://trac.webkit.org/changeset/92190>
All reviewed patches have been landed. Closing bug.