[Mac] Buttons with font sizes>23pt are not correctly rendered #3085

[xpomul]

Fix issue #3085 by handling the case when a button uses a custom font with size > 16pt. In this case adjust the button height in cellSizeForBounds(), similar to the other height adjustment fixes already present.

[github-actions]

Test Results (macos)

   54 files  ±0     54 suites  ±0   6m 22s ⏱️ +8s
4 528 tests ±0  4 284 ✅ ±0  244 💤 ±0  0 ❌ ±0 
2 124 runs  ±0  2 062 ✅ ±0   62 💤 ±0  0 ❌ ±0 

Results for commit 0d65473. ± Comparison against base commit db2adff.

♻️ This comment has been updated with latest results.

[vogella]

@HeikoKlare can you review?

[HeikoKlare]

I have to admit that I am probably not the right person to review this. I am not a common Mac user and not an expert in the SWT implementation for Cocoa.

What I can contribute here is some testing and some considerations. Let's start with the testing:
With the proposed change, the behavior improves in the sense that the button increases its size for larger fonts. However, the behavior I experience feels a bit inconsistent. For example, when I increase the font size from 14 to 20, I get this:

When I reduce the font size from 24 to 20, I get this:

While already feels strange when manually changing the font size, it will be even be more difficult to achieve an expected result when automatically setting a fixed (but high) font size if the result will depend on which font size was set before.

With respect to the idea of adapting the size of the button at all, it seems as if this is not intended by Apple. From what I find in their documentation, it seems as if they assume the standard font (13 pt?) to be used for buttons. And if buttons with variable height content shall be used, there seem to be specific controls or control configurations available: https://developer.apple.com/design/human-interface-guidelines/buttons
We of course sometime need to overrule the intents of the operating systems and their guidelines to achieve consistent behavior across the operating systems, so it might be okay to do that here, but I still wanted to make that point.

[xpomul]

Thanks @HeikoKlare for having a look.
I am not a MacOS/Cocoa developer myself and have just tried to piece together what seemed to work better than what we currently have (which is buttons heights stuck at what is fine for the 13pt font size the system uses).

I was hoping to get in touch with someone who knows their way around Cocoa and whether there is a way to improve/work around the quirky behavior you have experienced (setting the same height leads to different results depending on whether a smaller or larger height was set before...). So far, I was not able to find one.
What I experienced is that, actually, the 20pt size you have experimented with is the size with the most unpleasant behavior. Other sizes, both smaller and larger that around 20pt seem to work much better... Still, I don't know why... :-(

Maybe it is a more general discussion how we deal with the strict implications from Apple.

• strictly accepting them would mean that Button.setFontSize() should have no effect and we stick to the Apple way of keeping a 13pt font for all buttons
• supporting fonts with bigger sizes (as we are currently doing), but then I'd say we should try to improve on the consequences (scaling of buttons) as well
• the radical approach might even be something like ditching the NSButton completely and doing it completely in SWT (at the cost of visual differences). But that is quite some effort and maybe not worth it...

That said, in the meantime, I have come across more parts where larger font sizes lead to issues with Apple's Cocoa implementation (e.g. Combo boxes). So there might be similar fixes needed (and maybe more strange behavior in Apple API to work around...)

[xpomul]

@vogella @HeikoKlare do you know if there is anyone else who could contribute to this discussion?

[vogella]

@Phillipus can you help here?

[Phillipus]

@Phillipus can you help here?

I share @HeikoKlare's concerns, but on the other hand this PR doesn't necessarily make things too worse. Apple's Human Interface Guidelines recommend standard button styles with 13pt text. Without this PR the font size is changed and the button doesn't change size.

Hard to say one way or the other.