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

[label] Full-width hyphen in Japanese is accidently disappeared #5420

Closed
Kanahiro opened this issue Jan 28, 2025 · 8 comments
Closed

[label] Full-width hyphen in Japanese is accidently disappeared #5420

Kanahiro opened this issue Jan 28, 2025 · 8 comments
Labels
bug Something isn't working PR is more than welcomed Extra attention is needed

Comments

@Kanahiro
Copy link
Contributor

Kanahiro commented Jan 28, 2025

maplibre-gl-js version: latest

browser: Chrome

Steps to Trigger Behavior

  1. write style to show label, without "text-font" or with "text-font" not including
  2. show labels including full-width hyphen in Japanese
  3. is not shown.

Link to Demonstration

Following two are using same style.

GL JS v4

https://jsbin.com/wosopasika/edit?html,output

Image

GL JS v3

https://jsbin.com/qiduxehete/edit?html,output

Image

  • You can see A-BーC in v3 but in v4 A-BC.
  • It seems that there might be some regression from v3 to v4.

Expected Behavior

v3 is correct.

Actual Behavior

is missing. It occurs also on latest version.

@HarelM
Copy link
Collaborator

HarelM commented Jan 28, 2025

Thanks for taking the time to report this issue!
Any chance you could take a look into this issue?

@HarelM HarelM added bug Something isn't working PR is more than welcomed Extra attention is needed labels Jan 28, 2025
@Kanahiro
Copy link
Contributor Author

Yes, I'm researching this now.👍

@Kanahiro
Copy link
Contributor Author

Kanahiro commented Jan 28, 2025

When text-font isn't defined OR defined font doesn't exist in glyphs, getGlyphs returns object as result that result[stack][id] become null.
This seems to occur this problem.

async getGlyphs(glyphs: {[stack: string]: Array<number>}): Promise<GetGlyphsResponse> {

@HarelM
Copy link
Collaborator

HarelM commented Jan 28, 2025

That can be a result of my refactoring of the callback hell, including the glyphs code, which was a nightmare to read and understand...

@Kanahiro
Copy link
Contributor Author

The problem is here:

/\p{Ideo}|\p{sc=Hang}|\p{sc=Hira}|\p{sc=Kana}/u.test(String.fromCodePoint(id));

This regexp judges whether a character is CJK or not.

Japansese full-width dash is not included sc=Hira or sc=Kana. It is included in sc=Common.
sc=Common includes very broad characters: some Japanese symbols and some half-width symbols like whitespace.

sc=Common includes not only Japanese symbols but also some symbols used globally.
Simply adding sc=Common in the regexp break current testcase: code

IMHO, there are two ways:

  1. hardcode all characters not included in sc=Hira or sc=Kana
  2. revert changes in the regexp to v3 codes

@HarelM
Copy link
Collaborator

HarelM commented Jan 28, 2025

I think this was changed by @1ec5. @1ec5 can you take a look at this issue please?

@Kanahiro
Copy link
Contributor Author

IMO, we can use unicodeBlockLookup

https://github.com/maplibre/maplibre-gl-js/blob/main/src/util/is_char_in_unicode_block.ts

sc=Kana in Regexp may not be enough unicode classfying like above.

@Kanahiro Kanahiro mentioned this issue Jan 28, 2025
6 tasks
@Kanahiro
Copy link
Contributor Author

I made draft: #5421

IMO:

  • It is better to use unicodeBlockLookup.
  • It may be possible to broaden blocks to be judged as CJK (e.g. // symbol in pull request)

I'm not sure how much is a cost to broaden it but it looks useful.

@Kanahiro Kanahiro closed this as completed Feb 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working PR is more than welcomed Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants