Repository navigation
Conversation
| break | ||
|
|
||
| if best_font is not None: | ||
| if abs(map_weight_name_to_score(prop.get_weight()) |
There was a problem hiding this comment.
pep8 is pretty unhappy about the formatting here.
|
Could you also add a test that warnings are actually raised when we miss a weight? We will always have dejavu installed so maybe one of the very high or low weights? |
|
Thanks @colathro ! It was great working with you today! |
|
I think we should warn anytime there's no exact match, rather than with a threshold of 100. |
| 'black': 900, | ||
| } | ||
|
|
||
| def map_weight_name_to_score(weight): |
There was a problem hiding this comment.
When you say private, I think of other language access modifiers. In Python would that be just making this a class method?
There was a problem hiding this comment.
Just prepend an underscore to its name.
I'm not quite able to figure out the best way to do this. I can't quite get the output to appear in the warnings no matter what I try. Is this due to the logger not actually implementing Warnings? |
|
The font weight dict looks incomplete, tested some fonts on Win10:
CJK fonts may not have same weight value compared to Latin fonts |
|
The CSS font weight names are mentioned in https://www.w3.org/TR/css-fonts-4/#font-weight-prop https://developer.mozilla.org/en-US/docs/Web/CSS/font-weight#Common_weight_name_mapping so the intermediate values should not be included, Windows likely just maps them to the closest name. It actually looks like our weights name table is wrong... The full table in the CSS spec is 100 thin (aka hairline in MozDev) plus the vendor-specific ones fontconfig (https://gitlab.freedesktop.org/fontconfig/fontconfig/blob/master/src/fcweight.c) MozDev, and Windows (https://docs.microsoft.com/en-us/dotnet/api/system.windows.fontweights?view=netframework-4.8#remarks) |
|
Are those font weight inconsistencies handled by #16203? Where would this PR stand after that one? |
|
Users can still request e.g. Foobar Thin font and if that doesn't exist but Foobar Regular does then we will return that, and I think it still makes sense to warn in that case? |
|
I'm pushing this to 3.4 as there seems to be some discussion about when we should warn (on a non-perfect match or any miss-match). |
PR Summary
#15529
We check if the chosen font is +-100 from the desired user outcome. If so, we log a warning documenting the desired and the actual used.
Test is added to make sure new function for string mappings are correct.
PR Checklist