UNICODE fixes (clipboard, window title, file dialogs, paths, font glyphs, ...)

[nixbody]

Adds UNICODE support for CFFI related things on HXCPP and HashLink targets.

[player-03]

Hey @restorer, can you take a look at this one and tell us how it compares to #1391?

[player-03]

@joshtynjala Can you resolve the conflicts in ExternalInterface.cpp? It looks like you were both trying to solve the same problems but solved them in slightly different ways.

[nixbody]

I'd like to discourage from using std::wcstombs even on Windows because the encoding of the resulting string is locale-dependent and while some more recent versions of Windows 10 (and newer) allow to set locale to UTF-8, it's probably never the default system locale. Windows API uses UTF-16 (UCS-2 before Windows 2000) for file names and it's quite possible to have files with names that cannot be translated to a narrow string using the default system locale.

[joshtynjala]

. @joshtynjala Can you resolve the conflicts in ExternalInterface.cpp? It looks like you were both trying to solve the same problems but solved them in slightly different ways.

[nixbody]

In order to resolve the conflicts some maintainer needs to decide which one of those two similar functions (hl_wstring_to_utf8_bytes() vs wstring_to_vbytes()) they wish to keep. Mine is locale-independent so I think it's better but I really wish for this PR to get merged so I go either way.

[tobil4sk]

which one of those two similar functions (hl_wstring_to_utf8_bytes() vs wstring_to_vbytes()) they wish to keep

I've opened up a another PR #1665 to clean things up so hopefully we can avoid these conversions altogether.

[tobil4sk]

Actually, it looks like there are a few other places where these conversions are still required, so might need to keep one of these functions around anyway, unless those wstrings can be avoided as well.

[tobil4sk]

In order to resolve the conflicts some maintainer needs to decide which one of those two similar functions (hl_wstring_to_utf8_bytes() vs wstring_to_vbytes()) they wish to keep.

Would it be possible to decide on one of these so that these unicode issues can be fixed? Maybe @nixbody's one because it is locale independent?

(For reference, the commit that introduced the conflicts was 95411ac)

[tobil4sk]

I've forked and resolved the merge conflicts in a separate branch, preferring hl_wstring_to_utf8_bytes over wstring_to_vbytes: https://github.com/tobil4sk/lime/tree/cffi-unicode-fixes

This is quite an important fix. If there are no objections, these changes can be pushed here and this could be merged? Then, once things are working finally, in future we can look at cleaning up the conversions in #1665.

[player-03]

I didn't have permission to push your branch from my machine, so I redid the changes in the web editor. Let me know if I got anything wrong.

[tobil4sk]

I added a commit here to put it in sync with my branch.

[tobil4sk]

There is a conflict now in src/lime/text/Font.hx because of #1742. I think it might be fine to keep that file as it currently is in develop, it seems to me that both changes deal with the same issue? @nixbody

[player-03]

I think I'd prefer the version here, without the unsafe cast.

[tobil4sk]

I think I'd prefer the version here, without the unsafe cast.

You're right, looks like this is the preferred way to handle hl.NativeArray -> Array conversions with hashlink, e.g. in hlsdl:

https://github.com/HaxeFoundation/hashlink/blob/783cd2a785b2e6f38e6b01e1b3bf5bdc19fa1173/libs/sdl/sdl/Sdl.hx#L141-L144

[tobil4sk]

Since all the conflicts and issues seem to be resolved here, is this something that could be merged soon? This is quite a useful fix.

[player-03]

I haven't moved towards merging this because I don't know how to test it. I can tell it doesn't break the TextRendering sample on Linux, but it also doesn't fix it.

But if you've gone over the changes, you can go ahead and merge it yourself. (I'm a fan of PRs being merged by whoever understands them best. Then if there's a problem it's clear who to contact.)

[tobil4sk]

I can tell it doesn't break the TextRendering sample on Linux, but it also doesn't fix it.

I've mostly been dealing with the file dialog and file system fixes here, which I've tested and they work fine. I don't think these changes would be enough to get unicode text rendering working, I think a separate PR like #1391 is needed to deal with that.

I haven't myself run into the issues with the other parts of the api this PR fixes (particularly the font related bits). However, the changes are pretty regular and simple (ensuring that necessary string conversions are performed before passing to native apis). From what I know about hxcpp and hashlink string encoding, these changes look correct to me.

I'd prefer to just go ahead and merge this, but if you think it's better I could also try to create some test projects to test all these api calls, I don't know when I'll have time for this though.

[player-03]

However, the changes are pretty regular and simple (ensuring that necessary string conversions are performed before passing to native apis). From what I know about hxcpp and hashlink string encoding, these changes look correct to me.

That sounds like a good level of confidence. I think you can go ahead, then.