kirill-grouchnikov / substance

A modern and high-performant Swing look-and-feel library
163 stars 110 forks source link

NullPointerException in CheckBoxMenuItemIcon.getIconWidth() #86

Closed codemasterover9000 closed 6 years ago

codemasterover9000 commented 6 years ago

Version of Substance

8.0.02

Version of Java

java version "1.8.0_162" Java(TM) SE Runtime Environment (build 1.8.0_162-b12) Java HotSpot(TM) 64-Bit Server VM (build 25.162-b12, mixed mode)

Version of OS

Microsoft Windows [Version 10.0.16299.371]

The issue you're experiencing (expected vs actual, screenshot, stack trace etc)

When a JMenu contains both regular JItems, and a JCheckBoxMenuItem, all with a custom icon, and the user opens the menu an NPE occurs:

D:\download\substance>java -cp .;substance-8.0.02.jar;trident-1.5.00.jar Walkthrough
Exception in thread "AWT-EventQueue-0" java.lang.NullPointerException
        at org.pushingpixels.substance.internal.utils.icon.CheckBoxMenuItemIcon.getIconWidth(CheckBoxMenuItemIcon.java:203)
        at org.pushingpixels.substance.internal.utils.menu.MenuUtilities.layoutMenuItem(MenuUtilities.java:342)
        at org.pushingpixels.substance.internal.utils.menu.MenuUtilities.getMenuLayoutInfo(MenuUtilities.java:289)
        at org.pushingpixels.substance.internal.utils.menu.MenuUtilities.getMetrics(MenuUtilities.java:793)
        at org.pushingpixels.substance.internal.utils.menu.MenuUtilities.getPopupLayoutMetrics(MenuUtilities.java:856)
        at org.pushingpixels.substance.internal.utils.menu.MenuUtilities.getPopupLayoutMetrics(MenuUtilities.java:822)
        at org.pushingpixels.substance.internal.utils.menu.MenuUtilities.getPreferredWidth(MenuUtilities.java:897)
        at org.pushingpixels.substance.internal.ui.SubstanceMenuItemUI.getPreferredMenuItemSize(SubstanceMenuItemUI.java:229)
        at javax.swing.plaf.basic.BasicMenuItemUI.getPreferredSize(BasicMenuItemUI.java:376)
        at javax.swing.JComponent.getPreferredSize(JComponent.java:1662)
        at javax.swing.BoxLayout.checkRequests(BoxLayout.java:484)
        at javax.swing.BoxLayout.preferredLayoutSize(BoxLayout.java:301)
        at javax.swing.plaf.basic.DefaultMenuLayout.preferredLayoutSize(DefaultMenuLayout.java:60)
        at java.awt.Container.preferredSize(Container.java:1797)
        at java.awt.Container.getPreferredSize(Container.java:1781)
        at javax.swing.JComponent.getPreferredSize(JComponent.java:1664)
        at javax.swing.JMenu.getPopupMenuOrigin(JMenu.java:377)
        at javax.swing.JMenu.setPopupMenuVisible(JMenu.java:343)
        at javax.swing.JPopupMenu.menuSelectionChanged(JPopupMenu.java:1478)
        at javax.swing.MenuSelectionManager.setSelectedPath(MenuSelectionManager.java:125)
        at javax.swing.plaf.basic.BasicMenuUI.appendPath(BasicMenuUI.java:222)
        at javax.swing.plaf.basic.BasicMenuUI.access$200(BasicMenuUI.java:49)
        at javax.swing.plaf.basic.BasicMenuUI$Handler.mousePressed(BasicMenuUI.java:461)
        at java.awt.AWTEventMulticaster.mousePressed(AWTEventMulticaster.java:279)
        at java.awt.Component.processMouseEvent(Component.java:6530)
        at javax.swing.JComponent.processMouseEvent(JComponent.java:3324)
        at java.awt.Component.processEvent(Component.java:6298)
        at java.awt.Container.processEvent(Container.java:2237)
        at java.awt.Component.dispatchEventImpl(Component.java:4889)
        at java.awt.Container.dispatchEventImpl(Container.java:2295)
        at java.awt.Component.dispatchEvent(Component.java:4711)
        at java.awt.LightweightDispatcher.retargetMouseEvent(Container.java:4889)
        at java.awt.LightweightDispatcher.processMouseEvent(Container.java:4523)
        at java.awt.LightweightDispatcher.dispatchEvent(Container.java:4467)
        at java.awt.Container.dispatchEventImpl(Container.java:2281)
        at java.awt.Window.dispatchEventImpl(Window.java:2746)
        at java.awt.Component.dispatchEvent(Component.java:4711)
        at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:760)
        at java.awt.EventQueue.access$500(EventQueue.java:97)
        at java.awt.EventQueue$3.run(EventQueue.java:709)
        at java.awt.EventQueue$3.run(EventQueue.java:703)
        at java.security.AccessController.doPrivileged(Native Method)
        at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:80)
        at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:90)
        at java.awt.EventQueue$4.run(EventQueue.java:733)
        at java.awt.EventQueue$4.run(EventQueue.java:731)
        at java.security.AccessController.doPrivileged(Native Method)
        at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:80)
        at java.awt.EventQueue.dispatchEvent(EventQueue.java:730)
        at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:205)
        at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:116)
        at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:105)
        at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
        at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:93)
        at java.awt.EventDispatchThread.run(EventDispatchThread.java:82)

Code triggering the problem (created by expanding the Walkthrough example):

import java.awt.*;
import java.net.URL;
import javax.swing.*;
import java.awt.image.*;
import org.pushingpixels.substance.api.skin.SubstanceBusinessBlueSteelLookAndFeel;

public class Walkthrough extends JFrame {
    public Walkthrough() {
        super("Sample app");
        this.setLayout(new FlowLayout());
        this.add(new JButton("button"));
        this.add(new JCheckBox("check"));
        this.add(new JLabel("label"));

        JMenuBar bar = new JMenuBar();
        JMenu menu = new JMenu("menu");
        JMenuItem item = new JMenuItem("item", getIcon("icon.png"));
        item.setHorizontalTextPosition(JButton.TRAILING);
        item.setVerticalTextPosition(JButton.CENTER);
        item.setDisabledIcon(createDisabledIcon(getIcon("icon.png")));
        menu.add(item);

        JCheckBoxMenuItem checkitem = new JCheckBoxMenuItem("checkitem", getIcon("icon.png"));
        checkitem.setHorizontalTextPosition(JButton.RIGHT);
//      checkitem.updateUI();  // workaround
        checkitem.setVerticalTextPosition(JButton.CENTER);
        checkitem.setEnabled(true);
        checkitem.setSelected(true);
        checkitem.setDisabledIcon(createDisabledIcon(getIcon("icon.png")));
        menu.add(checkitem);

        bar.add(menu);
        this.setJMenuBar(bar);

        this.setIconImage(new BufferedImage(1, 1, BufferedImage.TYPE_4BYTE_ABGR));
        this.setSize(new Dimension(250, 100));
        this.setLocationRelativeTo(null);
        this.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    }

    private ImageIcon getIcon(String name)
    {
        URL location = getClass().getResource(name);
        if (location != null)
            return new ImageIcon(location);
        else 
            return null;
    }

    public static void main(String[] args) {
        JFrame.setDefaultLookAndFeelDecorated(true);
        SwingUtilities.invokeLater(new Runnable() {
            public void run() {
                try {
                    UIManager.setLookAndFeel(new SubstanceBusinessBlueSteelLookAndFeel());
                } catch (Exception e) {
                    System.out.println("l&f failed to initialize");
                }
                Walkthrough w = new Walkthrough();
                w.setVisible(true);
            }
        });
    }

    public static ImageIcon createDisabledIcon(ImageIcon i) {
        return new ImageIcon(createDisabledImage(i.getImage()));
    }

    public static Image createDisabledImage(Image i) {
        GrayFilter filter = new GrayFilter(true, 50);
        ImageProducer prod = new FilteredImageSource(i.getSource(), filter);
        Image grayImage = Toolkit.getDefaultToolkit().createImage(prod);
        return grayImage;
    }
}

I've found a workaround that I commented in the code. However, that should not be necessary.

Icon resource used by the sample: icon

kirill-grouchnikov commented 6 years ago

I'm not seeing the crash with the latest 8.1.00dev build on my Mac. It might take me some time to test on a Windows box since that's not my dev environment.

kirill-grouchnikov commented 6 years ago

No crash running 8.0.02 as well on Mac with Java 1.8.0_112.

codemasterover9000 commented 6 years ago

On Windows using Java 1.8.0_131 also doesn't crash. I found this commit in the OpenJDK repo which is referenced in the bugfixes of 1.8.0_152 which might be related: http://hg.openjdk.java.net/jdk8u/jdk8u/jdk/rev/908d5fa49906

It looks like a change listener in BasicMenuItemUI is now updating the checkIcon when setHorizontalTextPosition is called.

kirill-grouchnikov commented 6 years ago

Confirming crash in Java 1.8.0_161 on my Mac. Will be addressed shortly in 8.1.00dev branch.

kirill-grouchnikov commented 6 years ago

That is actually a pretty bad change of behavior.

The UI delegate gets the chance to install its own visual decorations in the regular installDefaults() method. But then, the basic UI delegate incorrectly replaces the checkIcon set earlier with the version taken from the UIManager global table - a version that is not connected to the specific menuItem (which, in Substance's case, is needed for proper animations and size-aware painting).

There is the option to "install" a check icon factory on the UIManager, but the matching MenuItemCheckIconFactory class is in sun.swing package, only exposed to the Vista look and feel. What a mess this is.

I'll need to find the way to essentially "roll back" the check icon override.