Fix for loading custom fonts on iOS when referenced from a CCB file #2102
Conversation
…er CCB file Fix up how fonts are loaded in Cocos2dx for iOS to work transparently with custom font filenames/paths such as 'MyCustomFont.ttf' or 'fonts/MyCustomFont.ttf'. Referring to fonts by these paths would be OK on Android but can't be used on iOS since fonts must be referred to by family name on iOS; we must use 'MyCustomFont' when referring to that particular font in both examples on iOS. Hence add the code to strip out the folder path and extension on iOS so that the font path 'MyCustomFont.ttf' and 'fonts/MyCustomFont.ttf' can be used transparently across all platforms. This is required especially in the case where we have custom fonts being referenced from CCB files, since CCB files store the font names with the .ttf extension included.
| else | ||
| { | ||
| fntName = _isValidFontName(pFontName) ? fntName : @"MarkerFelt-Wide"; |
There was a problem hiding this comment.
Why comment this code?
It should check if it is a system font name. Because the passed in font name may be a system font name.
There was a problem hiding this comment.
That is true, the font could be a system font. The reason I removed these statements:
fntName = _isValidFontName(pFontName) ? fntName : @"MarkerFelt-Wide";
font = [UIFont fontWithName:fntName size:nSize];
And replaced with just:
font = [UIFont fontWithName: @"MarkerFelt-Wide" size:nSize];
Was because they would cause the code to attempt to load the same font again if '_isValidFontName(pFontName)' returned true, or in other words if the font was a system font. Since it failed to load the first time it would almost certainly fail to load again on the second attempt- therefore I figured it would not make much sense in re-trying this failed operation again. Instead, the code was changed to go straight to loading the fallback 'MarkerFelt-Wide' font when an error occurred.
Was there a reason why the failed font load was being re-attempted a second time when the font was a system font?
There was a problem hiding this comment.
But if it is a valid system font name, then it can not load the correct system font.
Yep, if it isn't a valid system font, it will be always wrong. But we can not prevent developers from getting a valid system font.
There was a problem hiding this comment.
Ah ok, I think I finally understand now- apologies! So if the user is trying to load a system font (perhaps for reasons of localisation?) then we should only ever give back the default system font on failure to load? Yes... This makes sense because the default system font might support the character set that the user is looking for.
What was confusing me about this code was why it was trying the same failed operation twice but I think I understand the rationale behind it now. The only change that needs to be made I think is perhaps not to do the failed operation twice (when loading a system font) and go straight instead to the default system font.
Do the following additional changes below make sense?
There was a problem hiding this comment.
I think it is not so correct.
_isValidFontName() will check if the font name is valid, it doesn't load the font if it is valid.
So if it is a valid font name, such as "Arial", then it should load "Arial".
But the following code doesn't load "Arial", it just skip it because it is valid.
There was a problem hiding this comment.
That is true- it will not attempt to reload system fonts such as "Arial" again if the initial load fails. The reasoning for this however is that "Arial" would have already been attempted to load on the initial call to [UIFont fontWithName] below:
// create the font
id font = [UIFont fontWithName:fntName size:nSize];
Therefore would we want to re-attempt the same load again, even though the initial load (of the same system font) fails?
There was a problem hiding this comment.
And _isValidFontName() should not be invoked?
There was a problem hiding this comment.
I'm happy to continue to calling _isValidFontName() as per the 2nd commit in the pull request because we can use that function to determine how to handle errors and keep the error handling consistent with the past behaviour in cocos2dx. We can use _isValidFontName() to determine whether to load the 'MarkerFelt-wide' font when an error occurs.
There was a problem hiding this comment.
I'm also happy to delegate on the error handling aspect of this change if you have any suggestions to improve further? The most important thing is that we can support custom fonts referenced from CCB files and make custom font support more transparent across iOS and Android (and other platforms) so that we no longer need the #if blocks with different font names per platform, as per this example: http://www.cocos2d-x.org/projects/cocos2d-x/wiki/How_to_Use_Custom_TTF_Font_on_iOS
We should only load the 'MarkerFelt-Wide' fallback font in 'CCImage.mm, _initWithString()' if the user is attempting to load a custom font, not when the user is attempting to load a system font. If the user is trying to load a system font then give back the default system font instead.
|
Thank you. |
Fix for loading custom fonts on iOS when referenced from a CCB file
|
Thanks minggo! |
|
It's my pleasure. |
We should only load the 'MarkerFelt-Wide' fallback font in 'CCImage.mm, _initWithString()' if the user is attempting to load a custom font, not when the user is attempting to load a system font. If the user is trying to load a system font then give back the default system font instead.
…ng_fix Fix for loading custom fonts on iOS when referenced from a CCB file
We should only load the 'MarkerFelt-Wide' fallback font in 'CCImage.mm, _initWithString()' if the user is attempting to load a custom font, not when the user is attempting to load a system font. If the user is trying to load a system font then give back the default system font instead.
…ng_fix Fix for loading custom fonts on iOS when referenced from a CCB file
We should only load the 'MarkerFelt-Wide' fallback font in 'CCImage.mm, _initWithString()' if the user is attempting to load a custom font, not when the user is attempting to load a system font. If the user is trying to load a system font then give back the default system font instead.
Fix for loading custom fonts on iOS when referenced from a CCB file
Fix up how fonts are loaded in Cocos2dx for iOS to work transparently with custom font filenames/paths such as 'MyCustomFont.ttf' or 'fonts/MyCustomFont.ttf'. Referring to fonts by these paths would be OK on Android but can't be used on iOS since fonts must be referred to by family name on iOS; we must use 'MyCustomFont' when referring to that particular font in both examples on iOS. Hence add the code to strip out the folder path and extension on iOS so that the font path 'MyCustomFont.ttf' and 'fonts/MyCustomFont.ttf' can be used transparently across all platforms. This is required especially in the case where we have custom fonts being referenced from CCB files, since CCB files store the font names with the .ttf extension included.