Kentzo / ShortcutRecorder

The best control to record shortcuts on macOS, written in ObjC with Swift in mind
Other
571 stars 114 forks source link

Warning about fonts on Catalina #120

Closed lwouis closed 4 years ago

lwouis commented 4 years ago

I'm getting this warning:

CoreText note: Client requested name ".LucidaGrandeUI", it will get Times-Roman rather than the intended font. All system UI font access should be through proper APIs such as CTFontCreateUIFontForLanguage() or +[NSFont systemFontOfSize:].
CoreText note: Set a breakpoint on CTFontLogSystemFontNameRequest to debug.

I then put a symbolic breakpoint as instructed, and discovered that it is coming from ShortcutRecorder. Here is the stack-trace:

image

It stems from this call: https://github.com/Kentzo/ShortcutRecorder/blob/master/Library/SRRecorderControl.m#L709

Interestingly, my code from branch issue-114 is slightly different:

if (labelFrame.size.width >= minWidth)
    {
        if (!self.isOpaque && self.style.isLabelDrawingFrameOpaque)
            CGContextSetShouldSmoothFonts(NSGraphicsContext.currentContext.CGContext, true);

        [label drawWithRect:labelFrame options:0 attributes:labelAttributes context:nil];
    }

Checking the codebase, I also found this interesting line: https://github.com/Kentzo/ShortcutRecorder/blob/061f5ca1e22696165b6e8a4956aef086b82e75c9/Resources/Images.xcassets/sr-mojave-info.dataset/info.json#L50

I don't see an asset catalog for Catalina. Maybe Catalina changed fonts a bit, and a new asset catalog is needed? Not sure.

Also strangely, if I compare the actual font displayed:

image

It seems that on the fonts are not the same. The shift arrow for instance differs. Also strangely, if I try to change the font in TextEdit to Times or Times Roman, it instantly switches back to Lucida Grande. It may be that something is wrong on my system.

I see here and here that .AppleSystemUIFont may cause problems on Catalina. I think it's the font used currently, based on: https://github.com/Kentzo/ShortcutRecorder/blob/8e2b5a5792ffc8c6c75834e74ddca7ebf6c460a5/Library/SRRecorderControlStyle.m#L939

Last interesting bit, I ran this and got these results:

Based on that I checked in TextEdit with SF Pro Display and it looks like it's using it:

image

Overall I'm opening this ticket to document/discuss. Feel free to close it if you think nothing needs to be done here :)

Kentzo commented 4 years ago

Should be fixed in the issue-114 branch.

lwouis commented 4 years ago

The warnings are still there unfortunately, from this line:

https://github.com/Kentzo/ShortcutRecorder/blob/issue-114/Library/SRRecorderControl.m#L747

Kentzo commented 4 years ago

Hmm, can you change https://github.com/Kentzo/ShortcutRecorder/blob/issue-114/Library/SRRecorderControlStyle.m#L689 to unconditionally use -systemFontOfSize: and see if it helps with the warning?

lwouis commented 4 years ago

I followed your instructions, but the exception is still thrown. Line 689 you linked is executed 3 times, everytime with the value for font being:

".AppleSystemUIFont 13.00 pt. P [] (0x10058f220) fobj=0x10058f220, spc=3.59"

Then after those 3 execution, the exception is thrown on line 747 I linked

Kentzo commented 4 years ago

Interesting. Can you reproduce the issue in a demo app, just drawing any string with the same label attributes?

Kentzo commented 4 years ago

Another thing to try: try to remove these lines that enable font smoothing.

lwouis commented 4 years ago

Another thing to try: try to remove these lines that enable font smoothing.

Still the exception on the next line

Interesting. Can you reproduce the issue in a demo app, just drawing any string with the same label attributes?

Ok let me try that

lwouis commented 4 years ago

When creating a demo app with minimal code to create a font as in the exception, I get no exception:

image

Here is the one-file app:

import Cocoa
import Darwin

class App: NSApplication, NSApplicationDelegate {
    override init() {
        super.init()
        delegate = self
    }

    required init?(coder: NSCoder) {
        super.init(coder: coder)
    }

    func applicationDidFinishLaunching(_ aNotification: Notification) {
        let window = NSWindow()
        let textView = NSTextView()
        let font = NSFont.systemFont(ofSize: 13)
        debugPrint(font)
        debugPrint(".AppleSystemUIFont 13.00 pt. P [] (0x10058f220) fobj=0x10058f220, spc=3.59")
        textView.font = font
        window.contentView = textView
        window.makeKeyAndOrderFront(nil)
    }
}
lwouis commented 4 years ago

Constructing fonts by name doesn't warn either:

image

Kentzo commented 4 years ago

What if use the same API to draw the string as SR does? With the same attributes.

Kentzo commented 4 years ago

@lwouis Any luck with reproducing the issue?

lwouis commented 4 years ago

@Kentzo sorry for the delays. I'm trying to push important tickets for AltTab. I think this ticket being just a warning without any actual consequences, I will revisit it later, as soon as I have some bandwidth :)

lwouis commented 4 years ago

I found the issue!

On Catalina, the new system font is SF Pro. That font doesn't contain the unicode glyph for some reason.

First character is only in Lucida Grande UI All other characters are in SF Pro Text
image image

This means that to render a RecorderControl which contains the , NSFont.systemFont fails and there is a fallback on .LucidaGrandeUI.

I'm not sure how to go forwards with this. I see 2 roads:

  1. Use another font on Catalina. I have seen tickets in other big project such as OpenJDK, where people conservatively decided to use another system font than SF Pro on Catalina.

  2. Replace the glyph with 􀅆 on Catalina. That way all rendering happens within the same font, which is the official font for Catalina.

Kentzo commented 4 years ago

I wonder how TextEdit suppresses this warning .

I think warning is something the project can live with. This character is used throughout the system and I’d like to keep it as is.

Otherwise, great investigation! Have you filed a bug, I’d like to dupe it.

lwouis commented 4 years ago

So you're not interested in any of the 2 workarounds? I'm fine with having a warning, but it's not just a false warning. There is an actual issue as there will be multiple fonts used to render the UI.

image

Here for example, the shift is SF Pro, and the tab is Lucida Grande. Not ideal as they have different artistic styles. It's not dramatic, but I suggested some workarounds, and they both may be quite easy to implement.

I've opened a bug report to Apple, to make them aware of SF Pro shortcoming:

There seems to have been an oversight in Catalina's new system font SF Pro: the key tab unicode glyph is missing. It falls-back to Lucida Grande UI to be able to display it, but I get a warning in the console:

2020-05-06 03:29:22.764603+0900 AltTab[5802:84236] CoreText note: Client requested name ".LucidaGrandeUI", it will get Times-Roman rather than the intended font. All system UI font access should be through proper APIs such as CTFontCreateUIFontForLanguage() or +[NSFont systemFontOfSize:].

Please find the full context with more details on this github issue: https://github.com/Kentzo/ShortcutRecorder/issues/120#issuecomment-624219059

Kentzo commented 4 years ago

I'm not sure workaround is necessary: I have checked builtin Catalina apps, like Keyboard Viewer, and they seem to use Lucida Grande's font for that particular character. I believe it's important to match look'n'feel of the OS when it comes to this. As long as metrics are reported properly and rendering is not clipped I'm ok with this mix.

That said, right now it's not easy to customize code -> glyph transformation behavior. I'm looking to move transformers from being hardcoded in the control onto SRRecorderControlStyle.

lwouis commented 4 years ago

I have checked builtin Catalina apps, like Keyboard Viewer, and they seem to use Lucida Grande's font for that particular character

I wonder how Apple did their non-regression QA after switching fonts.

Anyway, thanks for sharing your thoughts. This was very interesting!

Please feel free to close this ticket 👍

Kentzo commented 4 years ago

Fingers crossed the bug will resolve itself in June :) Please let me know if you hear anything from Apple.