On Fri, 21 Mar 2025 11:26:34 GMT, Prasanta Sadhukhan <psadhuk...@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:  Then, after clicking the button, the button text renders with the default font:  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
