New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked #5082
Changes from 4 commits
d782b7c
d4b21ed
bda9178
fefcd37
df34ab0
87ab3e3
3b81c97
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
@@ -1,5 +1,5 @@ | ||
/* | ||
* Copyright (c) 2011, 2020, Oracle and/or its affiliates. All rights reserved. | ||
* Copyright (c) 2011, 2021, Oracle and/or its affiliates. All rights reserved. | ||
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. | ||
* | ||
* This code is free software; you can redistribute it and/or modify it | ||
@@ -326,6 +326,20 @@ public void paint(final Graphics g, final JComponent c) { | ||
} | ||
} | ||
|
||
protected void paintFocus(Graphics g, AbstractButton b, | ||
Rectangle viewRect, Rectangle textRect, Rectangle iconRect) { | ||
if (g instanceof Graphics2D) { | ||
((Graphics2D)g).setStroke(new BasicStroke(3)); | ||
((Graphics2D)g).setRenderingHint( | ||
RenderingHints.KEY_ANTIALIASING, | ||
RenderingHints.VALUE_ANTIALIAS_ON); | ||
|
||
} | ||
Color ringColor = UIManager.getLookAndFeelDefaults().getColor("Focus.color"); | ||
g.setColor(ringColor); | ||
g.drawRoundRect(5, 3, b.getWidth() - 10, b.getHeight() - 7, 15, 15); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't you need to restore the properties of Graphics2D? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I might as well. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about the color property? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That would be unnecessary, whoever does painting does not just assumes that the color is set to his preferred choice, everywhere in the code color is set up before doing any painting and i do not see anywhere that old color being preserved. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can trace the usage of graphics.getColor() for example in AquaMenuItemUI/WindowsMenuUI/BevelBorder/etc to check that the old color property is usually saved and then restored. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
A few lines above paintFocus() is called there is a section that sets color to something different in case of opaque button and that does not affect the next step - text painting - since text painter will grab text color and set the graphics draw color to it. However in the interest of moving forward i will ad saving the color - after al it is not a performance critical task. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have you checked if button does not have any text? It might affect the drawRoundRect values which are hardcoded now...In MetalButtonUI#paintFocus, it seems they cater to Button focus ring with and without text by taking care of setBounds(). Do we need to do something similar here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes, i did. Neither my code nor native focus painter is affected by it. |
||
} | ||
|
||
protected String layoutAndGetText(final Graphics g, final AbstractButton b, final AquaButtonBorder aquaBorder, final Insets i, Rectangle viewRect, Rectangle iconRect, Rectangle textRect) { | ||
// re-initialize the view rect to the selected insets | ||
viewRect.x = i.left; | ||
@@ -0,0 +1,91 @@ | ||
/* | ||
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved. | ||
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. | ||
* | ||
* This code is free software; you can redistribute it and/or modify it | ||
* under the terms of the GNU General Public License version 2 only, as | ||
* published by the Free Software Foundation. | ||
* | ||
* This code is distributed in the hope that it will be useful, but WITHOUT | ||
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or | ||
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License | ||
* version 2 for more details (a copy is included in the LICENSE file that | ||
* accompanied this code). | ||
* | ||
* You should have received a copy of the GNU General Public License version | ||
* 2 along with this work; if not, write to the Free Software Foundation, | ||
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. | ||
* | ||
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA | ||
* or visit www.oracle.com if you need additional information or have any | ||
* questions. | ||
*/ | ||
|
||
/* | ||
* @test | ||
* @requires (os.family == "mac") | ||
* @bug 8269951 | ||
* @summary Test checks that focus is painted on JButton even | ||
* when borders turned off | ||
* @library ../../regtesthelpers | ||
* @build Util | ||
* @run main AquaButtonFocusTest | ||
*/ | ||
|
||
|
||
import javax.swing.JButton; | ||
import javax.swing.UIManager; | ||
import java.awt.Graphics; | ||
import java.awt.image.BufferedImage; | ||
|
||
public class AquaButtonFocusTest { | ||
public static void main(String[] args) { | ||
new AquaButtonFocusTest().performTest(); | ||
} | ||
|
||
public void performTest() { | ||
try { | ||
UIManager.setLookAndFeel("com.apple.laf.AquaLookAndFeel"); | ||
} catch (Exception e) { | ||
throw new RuntimeException("Can not initialize Aqua L&F"); | ||
} | ||
|
||
FocusableButton one = new FocusableButton("One"); | ||
one.setSize(100, 100); | ||
one.setBorderPainted(false); | ||
BufferedImage noFocus = new BufferedImage(200, 100, BufferedImage.TYPE_INT_ARGB); | ||
Graphics g = noFocus.createGraphics(); | ||
one.paint(g); | ||
g.dispose(); | ||
BufferedImage focus = new BufferedImage(200, 100, BufferedImage.TYPE_INT_ARGB); | ||
one.setFocusOwner(true); | ||
g = focus.createGraphics(); | ||
one.paint(g); | ||
g.dispose(); | ||
if (Util.compareBufferedImages(noFocus, focus)) { | ||
throw new RuntimeException("Focus is not painted on JButton"); | ||
} | ||
} | ||
|
||
class FocusableButton extends JButton { | ||
private boolean focusOwner = false; | ||
|
||
public FocusableButton(String label) { | ||
super(label); | ||
} | ||
|
||
public void setFocusOwner(boolean focused) { | ||
this.focusOwner = focused; | ||
} | ||
|
||
@Override | ||
public boolean isFocusOwner() { | ||
return focusOwner; | ||
} | ||
|
||
@Override | ||
public boolean hasFocus() { | ||
return this.focusOwner; | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we normally call like this in Basic L&F which is extended by different L&Fs so that it will pick up the defaults from the particular L&F in question, otherwise UIManager.getColor(). should suffice as Focus.color is defined in AquaLookAndFeel.
But I am not sure with this hardcoded values..Can't we leverage viewRect or textRect to get the required coordinates?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
viewRect is set to be exactly (0, 0, b.width, b.height). Insets are buried deep inside JRS classes and used as hardcoded valued, i can not get them from there and both textRect and iconRect are not representing what needs to be drawn to be consistent with the way OS draws focus. Plus they are being initialized only later down the code and at the time of this call they are empty. Not that it would matter.