Page MenuHomePhabricator

evas_textblock: optimize calculate main format once in layout setup stage
Needs ReviewPublic

Authored by ali.alzyod on Aug 9 2019, 5:08 AM.

Details

Summary

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.

Test Plan
#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.035342

Diff Detail

Repository
rEFL core/efl
Branch
arcpatch-D9536_1
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 12990
Build 9253: arc lint + arc unit
ali.alzyod created this revision.Aug 9 2019, 5:08 AM

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/

ali.alzyod requested review of this revision.Aug 9 2019, 5:08 AM
ali.alzyod edited the summary of this revision. (Show Details)Aug 9 2019, 5:13 AM
ali.alzyod edited the test plan for this revision. (Show Details)
ali.alzyod added a subscriber: smohanty.
cedric requested changes to this revision.Aug 9 2019, 9:27 AM

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).

This revision now requires changes to proceed.Aug 9 2019, 9:27 AM

@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/

ali.alzyod edited the test plan for this revision. (Show Details)Aug 9 2019, 3:38 PM

update test plan code

cedric added a comment.Aug 9 2019, 3:52 PM

I would really appreciate a new test in expedite.

I would really appreciate a new test in expedite.

D9541 is this ok ?

ali.alzyod edited the test plan for this revision. (Show Details)Aug 10 2019, 4:37 AM
cedric accepted this revision.Aug 12 2019, 9:18 AM
This revision is now accepted and ready to land.Aug 12 2019, 9:18 AM

This actually break Evas tests suite. Please run ninja test :-)

cedric requested changes to this revision.Aug 12 2019, 10:07 AM
This revision now requires changes to proceed.Aug 12 2019, 10:07 AM
ali.alzyod added a comment.EditedAug 12 2019, 2:11 PM

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.

ali.alzyod updated this revision to Diff 24128.Aug 16 2019, 2:21 PM
ali.alzyod edited the summary of this revision. (Show Details)

rebase

I still did not get the error, and both harfbuzz and fontconfig are enabled, so I try to rebase

@cedric ,

Could you please review it again. We need this patch in Tizen :)
ali.alzyod added a comment.EditedAug 20 2019, 8:40 AM

@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

@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.

@cedric just small thing, If you build without this patch, do you still get error ?

@cedric just small thing, If you build without this patch, do you still get error ?

Nop, no error without this patch (and the older version of harfbuzz).

@cedric what is your meson/ninja args do you use for building ?

@cedric what is your meson/ninja args do you use for building ?

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.

@cedric

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
ali.alzyod updated this revision to Diff 24552.Wed, Aug 28, 2:01 AM
  • pushing main format in stack
ali.alzyod retitled this revision from evas_textblock: optimize calculate main format form style to evas_textblock: optimize calculate main format once in layout setup stage.Thu, Sep 5, 12:12 AM