Be able to load a Face from a non-ASCII filename on Windows

[moi15moi]

This PR close #157

In brief, on Windows, FreeType doesn't support to create a Face with an non-ASCII filename.
To work around this problem, we load the font from memory.

Pillow use a similar approch.

[HinTak]

Looking at the code now, I think the problem is really that except UnicodeError is wrong. I think a python-side file existence test and a c-side file-not-exist test might be a better idea. (I think freetype actually returns "resource not available" or something quite specific when file not found).

[moi15moi]

I think the problem is really that except UnicodeError is wrong.

Honestly, I have no idea why this part of code is there. It seems to be for python 2 only and from what I can see, freetype-py need an python 3.7 or more

I think a python-side file existence test and a c-side file-not-exist test might be a better idea.

That's an alternative. I will implement it.

[HinTak]

Hmm, I think I see how that might come - freetype takes char *, or in python 3 terms, a byte array b'path', as input. So from python path to unix path, there is, or there should be, an explicit or implicit conversion from python unicode path to byte array utf8 path. So the unicode error is for failure in this case.

It is separate and different from the windows failure.

[moi15moi]

So the unicode error is for failure in this case

If a conversion fail, encode will raise an UnicodeEncodeError, so I don't see why we have an special case handling for python 2.

I updated my branch with your recommandation.

[rougier]

I'm not too fond of the constant you defined in ft_errors.py. I'd prefer to leave it as it was and use a comment when using erros 0x01.

[moi15moi]

I'm not too fond of the constant you defined in ft_errors.py. I'd prefer to leave it as it was and use a comment when using erros 0x01.

I applied the requested corrections.

[HinTak]

I'm not too fond of the constant you defined in ft_errors.py. I'd prefer to leave it as it was and use a comment when using erros 0x01.

You mean FT_Err_Ok ? It is defined upstream:
https://gitlab.freedesktop.org/freetype/freetype/-/blob/master/include/freetype/fterrdef.h?ref_type=heads#L58

FT_Err_Cannot_Open_Resource is 0x01. The numbers are not arbitrary - they have meanings. I think I only use FT_Err_Ok recently, so only imported one. If other errors need to be dealt with in different cases (like some errors are not fatal and can retry), they need to be added to ft_errors.py.

[HinTak]

If you need to add more values from upstream:
https://gitlab.freedesktop.org/freetype/freetype/-/blame/master/include/freetype/fterrdef.h#L61

Add more.

[HinTak]

Most of the time you only care whether it is FT_Err_Ok or not (or just ignore the return value). But sometimes you want to check what failure it is, and re-try. I think this is probably a good reason of adding
FT_Err_Cannot_Open_Resource, actually.

[moi15moi]

Could you let me know if there's anything I need to do for this PR to be accepted?

[NCBM]

I've just tried another strange solution for this:

from ctypes import CDLL, c_ssize_t, c_char_p, c_wchar_p, create_string_buffer
# `c_ssize_t` is not in actual declaration, I use it to help express (size_t)(-1) in C.

crt = CDLL("msvcrt")

# size_t wcstombs( char *dst, const wchar_t *src, size_t len );
wcstombs = crt.wcstombs
wcstombs.restype = c_ssize_t
wcstombs.argtypes = (c_char_p, c_wchar_p, c_ssize_t)

# char* setlocale( int category, const char* locale );
setlocale = crt.setlocale
setlocale.restype = c_char_p

# replace `_init_from_file` with this function
def _init_from_file(self, library, face, index, path):
    orig_lc = bytes(setlocale(0, None))
    setlocale(0, b"")
    u_filename = create_string_buffer(1024)  # change the size if needed, `len(path) * 4` for example.
    if wcstombs(u_filename, path, 1024) == -1:
        raise Exception()  # replace it with a better exception with message
    error = FT_New_Face(library, u_filename, index, byref(face))
    setlocale(0, orig_lc)  # restore original locale
    return error

Currently I tested it with WINE 9.20 in Chinese locale and it works. I'm not sure if this works in other cases.

[HinTak]

Locale-related matter and file-system processing on wine are significantly different from genuine windows . While doing so is useful, testing against wine especially in those two areas can give essentially unrelated results compared to genuine windows, so does not indicate anything...

[NCBM]

Locale-related matter and file-system processing on wine are significantly different from genuine windows . While doing so is useful, testing against wine especially in those two areas can give essentially unrelated results compared to genuine windows, so does not indicate anything...

I've just tried on my dual-boot Windows 11 after your reply. At least on my machine my patch made it instead of an error cannot open resource.

[moi15moi]

@HinTak Is there something I can do for this PR?

[HinTak]

Since we touched on testing, one useful (but quite substantial) task to do is to drive this new code though github's ci windows hosts, if it is possible to configure github's ci hosts to run in every (non-english) locale. I think that might be possible - or one can persuade github's ci system that having host configurable in every locale is a good thing.

I am not too keen on adding large block of rarely(?) used code which is also difficult to test and verify.

[moi15moi]

configure github's ci hosts to run in every (non-english) locale

Is your goal to test this PR on every codepage?

[HinTak]

Not necessaily needed on every code page - I don't think every code page has a different windows locale? But if the test is automated, N code pages is really the same work as 2N code pages.

[moi15moi]

I don't think every code page has a different windows locale?

A codepage can have many windows locale. See this stackoverflow post or even what I found out.

From what I know, it isn't possible to change directly the ANSI Code page with a command or with a API.
To change the ANSI Code page, we need to change the windows locale.
To change the Windows locale, I think dism /online /set-syslocale:<locale_name> would work, but to have effect, we need to reboot the computer, so I don't think we can use that in GitHub Actions.

I also tried to use Set-WinSystemLocale with the command Set-WinSystemLocale -SystemLocale ja-JP, but it didn't change the ACP key in HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\Nls\CodePage and GetACP didn't returned 932 which was the expected result, so it isn't a solution.
Edit: If I launch the powershell has admin, it actually set the ACP key in HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\Nls\CodePage to 932, but GetACP still doesn't give me 932.

[HinTak]

Should ping / re-activate this, or there abouts: https://github.com/orgs/community/discussions/68929

[HinTak]

I should quantify my comment further:

I am not too keen on adding large block of rarely(?) used code which affects one of the most frequently used code path (opening a file), which is also difficult to test and verify.

I.e. it makes one of the most frequently used part more complicated, and penalises the English users (which is the majority of the users). You can argue that the slow-down is negligible; but nonetheless, the suggested change - even when it is completely bug-free and bullet-proof - is about slowing the majority of users down for a minority.

[moi15moi]

I don't see how it penalises the English users. In term of performance, for ASCII filename, it is the same has before.