Page MenuHomePhabricator

evas_object_textblock: add support for variation sequences
Needs ReviewPublic

Authored by ali.alzyod on May 29 2019, 11:42 PM.

Details

Summary

update font processing to handle variation sequences unicodes to select proper glypg in respect to variation seqences

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", "");
   elm_win_autodel_set(win, EINA_TRUE);
   textblock = evas_object_textblock_add(win);
   efl_canvas_text_style_set(textblock,NULL,"DEFAULT='font=DejaVuSans font_fallbacks=SamsungColorEmoji color=#000 font_size=20'");
   evas_object_textblock_text_markup_set(textblock, "8&#xfe0f;&#x20E3;&#x262a;&#xfe0f;AAA&#x262a;&#xfe0E;1234567&#xfe0f;&#x20E3;");



   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, 320, 320);
   evas_object_resize(win, 320, 320);
   evas_object_show(win);
   elm_run();
   return 0;

}

ELM_MAIN()

Diff Detail

Repository
rEFL core/efl
Branch
arcpatch-D9053_1
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 13178
Build 9358: arc lint + arc unit
ali.alzyod created this revision.May 29 2019, 11:42 PM
ali.alzyod requested review of this revision.May 29 2019, 11:42 PM
ali.alzyod edited the test plan for this revision. (Show Details)May 29 2019, 11:55 PM

This Compare Old Vs New Vs Android

old (EFL)

Android

New (EFL)

segfaultxavi requested changes to this revision.May 30 2019, 1:40 AM
segfaultxavi added subscribers: zmike, segfaultxavi.

I am no expert in Unicode so I cannot comment on the code correctness. The end result looks like very good progress, though.

However, doesn't this patch break API compatibility? The signature for two public (EAPI) methods is being changed: evas_common_get_char_index and evas_common_font_glyph_search.

I think you might need to create new versions of these methods (something like evas_common_get_char_index_with_variant) so apps using the old signature do not break. What do you think @zmike?

This revision now requires changes to proceed.May 30 2019, 1:40 AM

@segfaultxavi are not these internal APIs ?

segfaultxavi resigned from this revision.May 30 2019, 1:50 AM

@segfaultxavi are not these internal APIs ?

Oh, I always assume that all EAPI symbols are public, but you are right, this file only seems to be included from _private headers.

I'm sorry for the noise :)

ali.alzyod added a comment.EditedMay 30 2019, 1:54 AM

@segfaultxavi You are right, it is confusing to use same signature,
Internal API should have there own signatures to avoid confusing

ali.alzyod requested review of this revision.May 30 2019, 2:36 AM
cedric requested changes to this revision.May 30 2019, 9:39 AM

Indeed that seems like a nice improvement. Could you provide us with a patch for our test suite too?

This revision now requires changes to proceed.May 30 2019, 9:39 AM
ali.alzyod updated this revision to Diff 22625.May 30 2019, 6:53 PM
  • fix compilation error if harfbuzz was not enabled
ali.alzyod updated this revision to Diff 22628.EditedMay 31 2019, 6:21 AM

update test suite with variation sequences
@cedric

This seems good for me. If there would be no more comments in 2 days ~ I'll accept this :)

ali.alzyod updated this revision to Diff 22641.Jun 2 2019, 5:31 AM
  • Add function comment decleration,Fix caching Issue, Fix searching fonts issue
ali.alzyod updated this revision to Diff 22708.Jun 12 2019, 6:59 AM

evas_textblock: add support for caching variation sequence

fix binary insertion

@herdsman are you there?

I don't think @herdsman can check this comment in a short time. He is not working here these days :)

@ali.alzyod
Small query here,
why do you need separate caching logic for variation sequence? Can't it be accommodated in the current logic?

@ali.alzyod
Small query here,
why do you need separate caching logic for variation sequence? Can't it be accommodated in the current logic?

-because variation sequences can vary from ( 0xFE00-0xFE0F , 0xE0100-0xE01EF, 0x180B-0x180D), So I think create hashing tables for these values for each character (or characters that could have variation sequences) will consume more memory, and logic will become more complicated where one Unicode value could have many variation sequences.

-So I thought of binary search as good solution where we will keep old logic as it is, and if character has variation sequence it will be added to variation sequence sorted list, and accessing will be fast specially list will be small for each unicode value.

@ali.alzyod
Small query here,
why do you need separate caching logic for variation sequence? Can't it be accommodated in the current logic?

-because variation sequences can vary from ( 0xFE00-0xFE0F , 0xE0100-0xE01EF, 0x180B-0x180D), So I think create hashing tables for these values for each character (or characters that could have variation sequences) will consume more memory, and logic will become more complicated where one Unicode value could have many variation sequences.

-So I thought of binary search as good solution where we will keep old logic as it is, and if character has variation sequence it will be added to variation sequence sorted list, and accessing will be fast specially list will be small for each unicode value.

Making myself clear,
Memory will not be issue here, only issue I can see is that single Unicode can have multiple variation sequence. So if same unicode with different variation sequence belonging to different font file will fetch you the incorrect font in some cases, if current logic is used.
Am I correct?
If so, then it looks good to me.

@ali.alzyod
Small query here,
why do you need separate caching logic for variation sequence? Can't it be accommodated in the current logic?

-because variation sequences can vary from ( 0xFE00-0xFE0F , 0xE0100-0xE01EF, 0x180B-0x180D), So I think create hashing tables for these values for each character (or characters that could have variation sequences) will consume more memory, and logic will become more complicated where one Unicode value could have many variation sequences.

-So I thought of binary search as good solution where we will keep old logic as it is, and if character has variation sequence it will be added to variation sequence sorted list, and accessing will be fast specially list will be small for each unicode value.

Making myself clear,
Memory will not be issue here, only issue I can see is that single Unicode can have multiple variation sequence. So if same unicode with different variation sequence belonging to different font file will fetch you the incorrect font in some cases, if current logic is used.
Am I correct?
If so, then it looks good to me.

"if same unicode with different variation sequence belonging to different font file will fetch you the incorrect font in some cases, if current logic is used.
Am I correct?
If so, then it looks good to me."

Yes you are

"only issue I can see is that single Unicode can have multiple variation sequence" this is the reason why "Memory will be an issue"
We will need to allocate more memory if we want to create hash-tables (we do not need to allocate all of them at once for each glyph), and it will be more than what we need to allocate for searching small subset of variations

Sorry, I wasn't precise enough. I mean evas unit test in src/tests/evas/evas_test_textblock.c, so that it get integrated inside ninja test properly.

Sorry, I wasn't precise enough. I mean evas unit test in src/tests/evas/evas_test_textblock.c, so that it get integrated inside ninja test properly.

For variation sequences test I could not think of unit testing for this feature, you can detect this feature visually only.

On the one side you *could* check it also visually in a test. On the other side, a test that just uses the feature, checks that the getter returns the same as the setter, checks that there are no errors, checks that there are no crashes, would also be a good start.

On the one side you *could* check it also visually in a test. On the other side, a test that just uses the feature, checks that the getter returns the same as the setter, checks that there are no errors, checks that there are no crashes, would also be a good start.

there are no getter and setter for this feature, it uses same as markup get/set.
It is about how to render text

there are no getter and setter for this feature, it uses same as markup get/set.
It is about how to render text

I am talking about the getter and setter of markup. If the rendering would fail or some parser stuff permits this feature, then there should likely either be a error, or a different returned string. So this can be tested in this way.

there are no getter and setter for this feature, it uses same as markup get/set.
It is about how to render text

I am talking about the getter and setter of markup. If the rendering would fail or some parser stuff permits this feature, then there should likely either be a error, or a different returned string. So this can be tested in this way.

First : I think getter and setter of markup must have error code I am 100% with this option, and it should be used before rendering (check parsed string).

Second: returning error for failed rendering is not convenient (My opinion ), for example when you do not have glyph and render '?' this is not real error, it is that your system does not know how to render it.

For unit test visual option automated way (just an Idea)
render Textblock on Image, save it. with each new build XOR new image with saved one to check if there are change

I am not talking about a return code.
I am talking about a *printed* error, you can detect those errors, examples can be found in the elementary (efl_ui_suite) testsuite.
(For the visual option, talk to @cedric, there are a few things that are getting difficult here)

@bu5hm4n I will check (efl_ui_suite) testsuite.
Thank you

ali.alzyod updated this revision to Diff 22991.Jun 26 2019, 8:15 AM

change way of finding substitute glyphs for Text Emojis

Exactly what @bu5hm4n said. As for the visual test, the easy way with text is just to ask for the object size once the markup is set. If all the characters are properly processed, you will always have the same size. There are already test like that in evas_test_textblock. It isn't perfect, but it ensure that we do not break to much the parsing and sizing of this characters that might not otherwise be seen by anyone during a release cycle.

ali.alzyod updated this revision to Diff 23046.Jun 28 2019, 8:42 AM
  • skip caching saved values
ali.alzyod updated this revision to Diff 23494.Jul 18 2019, 1:51 AM

Add test cases, update substitute color or text presentation

cedric accepted this revision.Jul 29 2019, 11:02 AM
This revision is now accepted and ready to land.Jul 29 2019, 11:02 AM
This revision was automatically updated to reflect the committed changes.
bu5hm4n reopened this revision.Jul 30 2019, 10:12 AM

I reverted this for now, as it fails on travis.

This revision is now accepted and ready to land.Jul 30 2019, 10:12 AM
ali.alzyod updated this revision to Diff 23774.Jul 30 2019, 2:24 PM

add font file for test

@bu5hm4n @zmike
Font files for test have been added

@zmike can you please close this one

bu5hm4n requested changes to this revision.Sep 5 2019, 2:17 AM

Still does not pass tests.

This revision now requires changes to proceed.Sep 5 2019, 2:17 AM

Still does not pass tests.

I uploaded 6MB file (NotoColorEmoji.ttf), but when I apply the patch its size is 0, do you know what may cause this issue ?

ali.alzyod updated this revision to Diff 24833.Sep 9 2019, 3:15 AM
  • upload new font file

@ali.alzyod

Could you check whether this patch still passes all the tests now ?
If there is no problem, I'll close soon.

Yes it passes