RFR: 6734168: BasicButtonUI uses wrong FontMetrics to Layout JButtons text

Fri Mar 21 16:38:15 UTC 2025

On Fri, 21 Mar 2025 11:26:34 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:

> javax.swing.plaf.basic.BasicButtonUI uses wrong FontMetrics object to layout the text on a JButton. 
> The paint(Graphics, JComponent) method of BasicButtonUI calculates the [FontMetrics](https://github.com/openjdk/jdk/blob/6656254c346ef505a48652fdf4dedd6edc020e33/src/java.desktop/share/classes/javax/swing/plaf/basic/BasicButtonUI.java#L331) using the passed Graphics object without setting the buttons font before. If the buttons font varies from the currently set font of the passed graphics object, the font metrics do not fit the metrics of the expected font leading to truncated text on the button.
> In WindowsLookAndFeel, the font in passed graphics object differs from currently set font as in 
> 
> button font javax.swing.plaf.FontUIResource[family=Tahoma,name=Tahoma,style=plain,size=10] 
> graphics font java.awt.Font[family=Dialog,name=Dialog,style=plain,size=12]
> 
> whereas in Metal and other L&F it is 
> `font javax.swing.plaf.FontUIResource[family=Dialog,name=Dialog,style=bold,size=12]`
> 
> so the fix is made in Windows L&F to ensure to set the font of the button to the passed graphics object font before calculating the current FontMetrics.

I'm unsure if this bug needs fixing at all.

When a button is painted, the fonts are correctly set. If one wants to cache an image of a button, it is their responsibility to configure the `Graphics` object with the correct attributes, failure to do so results in unpredictable behaviour.

“Caching” a painted button doesn't make much sense either: buttons may react to mouse hover…

The correct way to paint a button would be `button.paint(imageGraphics)` where `imageGraphics` is the `Graphics` object returned by `image.getGraphics`.

The test case doesn't play nicely with High DPI environments, or rather the approach demonstrated by the test case.

src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsButtonUI.java line 177:

> 175:         if (!(b.getFont().equals(g.getFont()))) {
> 176:             b.setFont(g.getFont());
> 177:         }

This looks wrong to me: you persistently change the font of the button to the font set in passed in `Graphics`.

Instead, `g.setFont(b.getFont())` should always be called — that is the font set in the button must be set to `Graphics` before painting.

The problem is reproducible with Metal too, add the following line after the button is created:

        button.setFont(new Font("Cambria", Font.PLAIN, 20));

Initially, the frame in the button looks this way:

![JButton in Windows 10 using Cambria font of size 20](https://github.com/user-attachments/assets/ee134452-39d5-485f-a838-faedf3a5ba1a)

Then, after clicking the button, the button text renders with the default font:

![After disabling the button, it renders with Dialog instead of Cambria](https://github.com/user-attachments/assets/fe1c1383-82e4-4ff6-82f5-8fb6d0bf910d)

If there were a way to re-enable the button again, it would render with `Font.DIALOG` instead of the custom font that I set when the button was created.

test/jdk/javax/swing/JButton/TestButtonFontMetrics.java line 61:

> 59: 
> 60:     public static void main(String[] args) throws Exception {
> 61:         UIManager.setLookAndFeel("com.sun.java.swing.plaf.windows.WindowsLookAndFeel");

Suggestion:

        UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName());

test/jdk/javax/swing/JButton/TestButtonFontMetrics.java line 86:

> 84:         Graphics imgGraphics = image.getGraphics();
> 85: 
> 86:         super.paintComponent(imgGraphics);

Suggestion:

        super.paintComponent(imgGraphics);

To separate the painting to image from further processing.

test/jdk/javax/swing/JButton/TestButtonFontMetrics.java line 97:

> 95:         JFrame frame = new JFrame("TestButtonFontMetrics");
> 96:         frame.setLayout(new GridBagLayout());
> 97:         frame.setDefaultCloseOperation(WindowConstants.DISPOSE_ON_CLOSE);

Suggestion:

Closing of the test UI is handled by `PassFailJFrame`: if it's closed, the test fails.

test/jdk/javax/swing/JButton/TestButtonFontMetrics.java line 99:

> 97:         frame.setDefaultCloseOperation(WindowConstants.DISPOSE_ON_CLOSE);
> 98:         TestButtonFontMetrics button = new TestButtonFontMetrics("Test");
> 99:         button.addActionListener(new DisableListener());

Suggestion:

        button.addActionListener(e -> ((JComponent) e.getSource()).setEnabled(false));

@DamonGuy [suggested using lambda expressions](https://github.com/openjdk/jdk/pull/22977#discussion_r1909319106) for `ActionListeners`.

test/jdk/javax/swing/JButton/TestButtonFontMetrics.java line 102:

> 100:         frame.add(button, new GridBagConstraints(0, 0, 1, 1, 0, 0,
> 101:              GridBagConstraints.CENTER, GridBagConstraints.NONE, new Insets(
> 102:                   10, 10, 0, 10), 0, 0));

Suggestion:

             GridBagConstraints.CENTER, GridBagConstraints.NONE,
             new Insets(10, 10, 0, 10), 0, 0));

Wrapping at `Insets` creation is easier to read. I'd also set the bottom inset to 10.

test/jdk/javax/swing/JButton/TestButtonFontMetrics.java line 103:

> 101:              GridBagConstraints.CENTER, GridBagConstraints.NONE, new Insets(
> 102:                   10, 10, 0, 10), 0, 0));
> 103:         frame.pack();

If you make the frame larger, it would display its title and would be easier to handle.

Since the test UI consists of a single button, I think it's a perfect case for using [`splitUIRight`](https://cr.openjdk.org/~aivanov/PassFailJFrame/api/PassFailJFrame.Builder.html#splitUIRight(PassFailJFrame.PanelCreator)) or simply [`splitUI`](https://cr.openjdk.org/~aivanov/PassFailJFrame/api/PassFailJFrame.Builder.html#splitUI(PassFailJFrame.PanelCreator)). Just return `JPanel` or `Box` with the button inside, see [`PrintLatinCJKTest.createTestUI`](https://github.com/openjdk/jdk/blob/0cb110ebb7f8d184dd855f64c5dd7924c8202b3d/test/jdk/java/awt/print/PrinterJob/PrintLatinCJKTest.java#L68-L92) for an example.

-------------

Changes requested by aivanov (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/24152#pullrequestreview-2706417765
PR Review Comment: https://git.openjdk.org/jdk/pull/24152#discussion_r2007915688
PR Review Comment: https://git.openjdk.org/jdk/pull/24152#discussion_r2007877018
PR Review Comment: https://git.openjdk.org/jdk/pull/24152#discussion_r2007852640
PR Review Comment: https://git.openjdk.org/jdk/pull/24152#discussion_r2007854081
PR Review Comment: https://git.openjdk.org/jdk/pull/24152#discussion_r2007855668
PR Review Comment: https://git.openjdk.org/jdk/pull/24152#discussion_r2007862675
PR Review Comment: https://git.openjdk.org/jdk/pull/24152#discussion_r2007876391