Skip to content
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

Closed
wants to merge 7 commits into from
@@ -328,10 +328,16 @@ public void paint(final Graphics g, final JComponent c) {

protected void paintFocus(Graphics g, AbstractButton b,
Rectangle viewRect, Rectangle textRect, Rectangle iconRect) {
final Border border = b.getBorder();
if (border instanceof AquaButtonBorder) {
((AquaButtonBorder)border).paintButton(b, g, 0, 0, b.getWidth(), b.getHeight());
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");
Copy link
Contributor

@prsadhuk prsadhuk Aug 13, 2021

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?

Copy link
Member Author

@azuev-java azuev-java Aug 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UIManager.getColor(). should suffice

Fixed.

But I am not sure with this hardcoded values..Can't we leverage viewRect or textRect to get the required coordinates?

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.

g.setColor(ringColor);
g.drawRoundRect(5, 3, b.getWidth() - 10, b.getHeight() - 7, 15, 15);
Copy link
Member

@mrserb mrserb Aug 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't you need to restore the properties of Graphics2D?

Copy link
Member Author

@azuev-java azuev-java Aug 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't you need to restore the properties of Graphics2D?

I might as well.

Copy link
Member

@mrserb mrserb Aug 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the color property?

Copy link
Member Author

@azuev-java azuev-java Aug 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the color property?

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.

Copy link
Member

@mrserb mrserb Aug 16, 2021

Choose a reason for hiding this comment

The 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.

Copy link
Member Author

@azuev-java azuev-java Aug 17, 2021

Choose a reason for hiding this comment

The 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.

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.

Copy link
Contributor

@prsadhuk prsadhuk Aug 18, 2021

Choose a reason for hiding this comment

The 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?

Copy link
Member Author

@azuev-java azuev-java Aug 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you checked if button does not have any text?

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) {