This change based on discussion on D9533 , where @smohanty example shows that unnecessary calculation happened on textblock related to lay-outing
Where now _layout_setup will not calculate main format using (_format_fill) unless style has been changed.
expedite commit:
https://git.enlightenment.org/tools/expedite.git/commit/?id=dc6c931dc2e6c240d3e240f24578980c689ab7fc
src/bin/textblock_text_fill_format.c
#define EFL_EO_API_SUPPORT 1
#define EFL_BETA_API_SUPPORT 1
#include <Eina.h>
#include <Efl.h>
#include <Elementary.h>
EAPI_MAIN int
elm_main(int argc, char **argv)
{
Evas_Object *win, *textblock;
elm_policy_set(ELM_POLICY_QUIT, ELM_POLICY_QUIT_LAST_WINDOW_CLOSED);
win = elm_win_util_standard_add("Main", "App");
elm_win_autodel_set(win, EINA_TRUE);
int l, r, t, b;
textblock = evas_object_textblock_add(evas_object_evas_get(win));
Evas_Textblock_Style *st = evas_textblock_style_new();
evas_textblock_style_set (st, "DEFAULT='font=Sans font_size=10 color=#000000 wrap=word align=left outline_color=#000 shadow_color=#fff8 shadow_color=#0002 glow2_color=#fe87 glow_color=#f214 underline_color=#00f linesize=40'");
evas_object_textblock_style_set(textblock, st);
int sizes[] = {600, 700};
evas_object_textblock_text_markup_set(textblock, "This test resize text block and keep style (style parsed only once)");
clock_t start, end;
start = clock();
for (int i = 0; i < 10000; i++)
{
evas_object_resize(textblock, sizes[i % 2], sizes[i % 2]);
evas_object_textblock_style_insets_get(textblock, &l, &r, &t, &b);
}
end = clock();
double total_Time1 = ((double)(end - start)) / CLOCKS_PER_SEC;
printf("total time = %f\n", total_Time1);
evas_object_size_hint_weight_set(textblock, EVAS_HINT_EXPAND, EVAS_HINT_EXPAND);
evas_object_size_hint_align_set(textblock, EVAS_HINT_FILL, EVAS_HINT_FILL);
evas_object_show(textblock);
evas_object_move(textblock, 0, 0);
evas_object_resize(textblock, 640, 800);
evas_object_resize(win, 640, 800);
evas_object_show(win);
elm_run();
return 0;
}
ELM_MAIN()Old Code output : total time = 0.096900
New Code output : total time = 0.045580
| Automatic diff as part of commit; lint not applicable. |
| Automatic diff as part of commit; unit tests not applicable. |
It seems that this patch has no reviewers specified. If you are unsure who can review your patch, please check this wiki page and see if anyone can be added: https://phab.enlightenment.org/w/maintainers_reviewers/
Test plan are nice, but not very helpful. It would be best to land that test program somewhere in our own tree (Could be build as a benchmark for example).
@cedric I did not get exactly what you mean, can you please be clear how to add example in tree(what tree :) )
After thinking about this more, it would be best to add a new tests to our benchmark suite: https://git.enlightenment.org/tools/expedite.git/
this is weird I did not get any errors in test suite !
@cedric is it possible to share the error ?
I am getting the following failures when running evas suite :
EFL_RUN_IN_TREE='1' /home/cedric/work/enlightenment/core/efl/build/src/tests/evas/evas_suite "Object Textblock" Running suite(s): Evas ERR<10106>:evas_font_main ../src/lib/evas/common/evas_text_utils.c:991 evas_common_text_props_split() Couldn't find the cutoff position. Is it inside a cluster? ERR<10106>:evas_font_main ../src/lib/evas/common/evas_text_utils.c:991 evas_common_text_props_split() Couldn't find the cutoff position. Is it inside a cluster? ERR<10136>: ../src/lib/evas/canvas/evas_textblock_hyphenation.x:147 _layout_wrap_hyphens_get() Couldn't find matching dictionary and couldn't fallback to locale C 96%: Checks: 27, Failures: 1, Errors: 0 ../src/tests/evas/evas_test_textblock.c:4175:F:Object Textblock:evas_textblock_hyphenation:0: Assertion 'w == fw' failed: w == 54, fw == 49
I have harfbuzz and fontconfig turned on. Not to sure what else in our configuration would be different.
I still did not get the error, and both harfbuzz and fontconfig are enabled, so I try to rebase
@cedric I think you will get same error, even if you get latest.
the reason is you build latest harfbuzz library on your machine, and with it you will get error on evas.
We already raise an issue on Harfbuzz side https://github.com/harfbuzz/harfbuzz/issues/1910
For now I suggest moving back to harfbuzz version 2.5.3
Thanks for digging that. I still tested this patch with version 2.5.0 and it does still fail. I am not sure this is the same problem.
Nothing really. I am on holiday for the next two weeks. Try to get more people to run this patch and see if someone else is having the same issue.
I have no problem running the evas suite. (Before Patch & After Patch)
bowon@bowon:~/efl/storage/efl/build/src/tests/evas$ sudo EFL_RUN_IN_TREE='1' ./evas_suite "Object Textblock" Running suite(s): Evas ERR<27275>:evas_font_main ../src/lib/evas/common/evas_text_utils.c:991 evas_common_text_props_split() Couldn't find the cutoff position. Is it inside a cluster? ERR<27275>:evas_font_main ../src/lib/evas/common/evas_text_utils.c:991 evas_common_text_props_split() Couldn't find the cutoff position. Is it inside a cluster? 100%: Checks: 27, Failures: 0, Errors: 0
Below is my harfbuzz information.
But not sure if it is a harfbuzz problem.
bowon@bowon:~/efl/storage/efl/build/src/tests/evas$ hb-view --version hb-view (HarfBuzz) 2.6.1 Available shapers: ot,fallback
ninja test fails for me too in evas-suite.
I think the relevant lines are:
../src/tests/evas/evas_test_textblock.c:3725:F:Object Textblock:evas_textblock_style:0: Assertion 'nw == 0' failed: nw == 102, 0 == 0 ../src/tests/evas/evas_test_textblock.c:4154:F:Object Textblock:evas_textblock_fit:0: Failure 'fw_new == fw && fh_new == fh' occurred
My PC in the work is so sweet and always pass the test, I try it at home and fix the issue
This patch has been there for very long time, can someone provide any kind of feedback ?
This patch is setting here for long time, Anyone of the reviewers can you please let me know what you think ??
Whoops this one got away from me. The tests do pass now, but, as @cedric said previously, this is a significant optimization patch, and it should include at least some sort of expedite benchmark along with some results in the commit log here.
Once that's done, this should be good to go!
Oh great. Can you update the commit log with a reference to the name of the expedite test used at the bottom? I'll land after