Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Labels incorrect for right-to-left languages #2543

Closed
mramato opened this issue Mar 3, 2015 · 9 comments
Closed

Labels incorrect for right-to-left languages #2543

mramato opened this issue Mar 3, 2015 · 9 comments

Comments

@mramato
Copy link
Contributor

mramato commented Mar 3, 2015

Similar to the problem in #2521, because we render each character individually, we are always rendering left to write, where some languages are right to left. It should be easy to render in the opposite direction, the question is whether we can automatically detect this or if we have to rely on the user telling us.

@mramato mramato changed the title Labels do not work for right-to-left languages Labels incorrect for right-to-left languages Mar 3, 2015
@mramato
Copy link
Contributor Author

mramato commented Mar 3, 2015

forum post

@EladShechter
Copy link

Maybe this could help:
https://github.com/twitter/twitter-cldr-js#handling-bidirectional-text

It's important to notice that there can be complex sentences with both rtl and ltr languages and numbers that are ltr. It's seems that the libary in the link above handle it.

@mramato
Copy link
Contributor Author

mramato commented Mar 26, 2015

Thanks for the link. The size of the library makes it unlikely that we will be able to use it directly (it does way more stuff than we need), but it definitely might help point us in the right direction.

@mramato
Copy link
Contributor Author

mramato commented Mar 26, 2015

Actually, it looks like that library still requires you to tell it what direction the text is, it doesn't detect it automatically (which is part of the problem), it still may be useful for RTL support overall.

@tonpo
Copy link

tonpo commented Apr 7, 2015

Hi,
One of my customers has need for RTL support in kmls (Arabic and Hebrew).

As a quick and dirty hack I used the TwitterCldr linked by @goergeBerg in this issue.

I'm writing it here because I think it could help you with the final solution...

I downloaded the npm package from
https://github.com/twitter/twitter-cldr-npm
then got the ar.js from 'full' directory and converted it to require.js module (for now, as is: https://gist.github.com/tonpo/59b63ac532be935914c4#file-ar-js).
Note that it's not must be the 'ar.js' as we need only the bidi functionality. actually 'he.js' is also works .

In KmlDataSource I injected the ar.js as 'TwitterCldr'.

Then, in the two places where 'label.text' is computed I replaced it with:

if (TwitterCldr && defined(entity.name)){
    var bidi_str = TwitterCldr.Bidi.from_string(entity.name, {"direction": "RTL"});
    bidi_str.reorder_visually();
    label.text = bidi_str.toString();
} else {
        label.text = entity.name;
    }

and

if (TwitterCldr && defined(targetEntity.name)){
    var bidi_str = TwitterCldr.Bidi.from_string(targetEntity.name, {"direction": "RTL"});
    bidi_str.reorder_visually();
    label.text = bidi_str.toString();
} else {
    label.text = targetEntity.name;
}

before:
reversed_label

after:
reversed_fixed_label

It's seems to work (including mixed sentences with english, numbers, arabic and question marks) with one caveat I noticed so far:
In arabic (and other languages) there is the cursive scripts issue: characters look different based on their relative position.

for arabic I have found this:
https://gist.github.com/thesnarky1/10012004

which is linked from here:
ondras/rot.js#31

So I converted the linked function to require.js module (https://gist.github.com/tonpo/59b63ac532be935914c4#file-cursive_scripts-js), inject it as 'cursive' to the KmlDataSource and now the relevant code looks like:

if (TwitterCldr && defined(entity.name)){
    var bidi_str = TwitterCldr.Bidi.from_string(cursive.getRealCharCodes(entity.name), {"direction": "RTL"});
    bidi_str.reorder_visually();
    label.text = bidi_str.toString();
} else {
    label.text = entity.name;
}

and

if (TwitterCldr && defined(targetEntity.name)){
    var bidi_str = TwitterCldr.Bidi.from_string(cursive.getRealCharCodes(targetEntity.name), {"direction": "RTL"});
    bidi_str.reorder_visually();
    label.text = bidi_str.toString();
} else {
    label.text = targetEntity.name;
}

reversed_and_cursive_fixed_label

Now it's looks far better and it's good enough for now, but in final solution it should looks (connected) like :
should_looks_like

This is still not perfect but I think it could help for your full solution...

I think there is a need to distinguish between 2 cases:

  1. the whole label should be transformed to RTL. this is the case I described above.
    rtl
  2. the label should be aligned LTR but still support rtl languages words in the correct characters order (not reversed and the cursive issue is treated). this is the the rest.
    ltr
    (The above two images are the same paragraph in chrome with different 'dir' attribute. be aware of the position of the words when adding word in a different language to the expression)

All browsers support the above cases: for case 1 the browser gives you the possibility to describe the direction as RTL (in 'dir' attribute or css 'direction' property).
If you don't give the direction the browser behavior is as described in case 2.

If one chooses option No.1 the name in the description (in kml it's displayed in the balloon) is also need to be aligned to rtl.

BTW: Is it really necessary to render the label char by char? Isn't there a simpler method to achieve the same goals?

@mramato
Copy link
Contributor Author

mramato commented Apr 7, 2015

Thanks for the write up @tonpo. I believe #2521 is the cause of "disconnected" characters.

BTW: Is it really necessary to render the label char by char? Isn't there a simpler method to achieve the same goals?

The problem is performance and memory. If all of your text is static and you don't have a lot of it, then you can get away with rendering whole words, I actually posted a small example of how to do that here. But if your text is dynamically changing or you have a lot of labels, then creating a unique texture for each word becomes a huge problem. So for Cesium as a whole rendering characters is the only solution.

That being said, I have a branch, single-billboard-labels, which adds the ability to tell the label collection to use whole words instead of individual characters. If you use that branch, I believe the disconnected problem would go away, but that branch isn't official or actively maintained (though I just merged in master). I basically use it for debugging and it will go away once we fix this issue and #2521.

@tonpo
Copy link

tonpo commented Apr 8, 2015

Thanks @mramato for the reply, and for Cesium. It's really great library.

After some reading in the links you gave in the forum post, I'm not sure #2521 is the cause for the disconnected letters here, because it seems that the range of the relevant arabic letters is not in the astral code point range.

Maybe there is an additional arbitrary space/padding/border between the rendered letters that causes it?

@hpinkos
Copy link
Contributor

hpinkos commented Dec 1, 2017

Fixed in #5771

@hpinkos hpinkos closed this as completed Dec 1, 2017
@cesium-concierge
Copy link

Congratulations on closing the issue! I found these Cesium forum links in the comments above:

https://groups.google.com/d/msg/cesium-dev/VbAwCV2PWTU/PO_7JDBuQ2UJ
https://groups.google.com/d/msg/cesium-dev/6EA78tUxGRY/iEmkET1kYQ0J

If this issue affects any of these threads, please post a comment like the following:

The issue at #2543 has just been closed and may resolve your issue. Look for the change in the next stable release of Cesium or get it now in the master branch on GitHub https://github.com/AnalyticalGraphicsInc/cesium.


I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome.

🌍 🌎 🌏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants