search

rougier / freetype-py

Python binding for the freetype library
Other
304 stars 88 forks source link

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

Open moi15moi opened 1 year ago

moi15moi commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 10 months ago

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