Why does paintComponent get called only on some JPanels?

Question

I'm trying to add a background to different JPanels (henceforth they will be called Window). These Windows are classes I created and make them inherit JPanel. Then depending on the state of the program one Window is set as the content panel of the program's JFrame. The problem comes when in some Windows the background gets set and in others not. The background setting is performed ussing the paintComponent(Graphics g) method, but despite I've tried to fix the bug, I didnt' success.

Here is the code I think might be useful for those who wanna help:

Main Loop:

public class Game{

//here comes other stuff (constructor, main, other methods...)

    private void run(){
        while(true){
            if(GameState.changed){
                Screen.getInstance().seeWindow(state);
                GameState.changed = false;
            }else {
                Screen.getInstance().requestFocus(state);
            }
        }
    }
}

Screen class:

package view;

import game.GameState;
import view.wins.*;

import javax.swing.*;
import java.awt.*;

public class Screen extends JFrame {
    private final int WIDTH;
    private final int HEIGHT;

    private static Screen instance = null;
    private JComponent titleWindow, menuWindow, settingsWindow;

    private Screen(){
        WIDTH = 1152;
        HEIGHT = 768;

        setTitle("Game");
        setDefaultCloseOperation(EXIT_ON_CLOSE);
        Dimension dim = Toolkit.getDefaultToolkit().getScreenSize();
        this.setLocation(dim.width/2-WIDTH/2, (dim.height - 50)/2-HEIGHT/2);
        Dimension size = new Dimension(WIDTH, HEIGHT);

        setPreferredSize(size);
        setMinimumSize(size);
        setMaximumSize(size);
        setSize(size);

        setResizable(false);
        setVisible(true);
    }

    public static Screen getInstance() {
        if(instance == null){
            instance = new Screen();
        }
        return instance;
    }

    public void seeWindow(GameState state){
        switch(state){
            case TITLE -> setContentPane(getTitleWindow());
            case MENU -> setContentPane(getMenuWindow());
            case SETTINGS -> setContentPane(getSettingsWindow());
        }
        pack();
    }

    public void requestFocus(GameState state){
        switch (state){
            case TITLE -> getTitleWindow().requestFocus();
            case MENU -> getMenuWindow().requestFocus();
            case SETTINGS -> getSettingsWindow().requestFocus();
        }
    }

    private JComponent getTitleWindow(){
        if(titleWindow == null){
            titleWindow = new TitleWindow();
        }
        return titleWindow;
    }

    private JComponent getMenuWindow(){
        if(menuWindow == null){
            menuWindow = new MenuWindow();
        }
        return menuWindow;
    }

    private JComponent getSettingsWindow(){
        if(settingsWindow == null){
            settingsWindow = new SettingsWindow();
        }
        return settingsWindow;
    }
}

Window abstract class:

package view.wins;

import utilz.GFXManager;
import view.Screen;

import javax.swing.*;
import java.awt.*;
import java.util.Observer;

public abstract class Window extends JComponent implements Observer {
    private Image background;

    public Window(String background){
        setLayout(new BorderLayout());
        setPreferredSize(Screen.getInstance().getPreferredSize());

        setBackground(background);
        setFocusable(true);
    }

    protected void setBackground(String backgroundName){
        this.background = GFXManager.getInstance().getImage("backgrounds/" + backgroundName + ".png");
    }

    @Override
    protected void paintComponent(Graphics g) {
        super.paintComponent(g);
        g.drawImage(background, 0, 0, null);
    }

}

A Window where the background is correctly set:

package view.wins;

import game.GameState;
import game.Game;
import jdk.swing.interop.SwingInterOpUtils;
import logic.TitleLogic;
import utilz.GFXManager;

import javax.swing.*;
import java.awt.*;
import java.awt.event.*;
import java.util.Observable;
import java.util.Observer;

public class TitleWindow extends Window implements Observer {
    private final String BELOW_TITLE_TEXT;
    private final ImageIcon TITLE_ICON;

    private JLabel lblTitleIcon, lblBelowTitle;
    private KeyController keyController;

    public TitleWindow(){
        super("title_background");
        BELOW_TITLE_TEXT = "Press enter to start";
        TITLE_ICON = new ImageIcon(GFXManager.getInstance().getImage("texts/title.png"));

        TitleLogic.getInstance().addObserver(this);

        setLayout(new BorderLayout());
        addKeyListener(new KeyController());

        add(getLblTitleIcon(), BorderLayout.CENTER);
        add(getLblBelowTitle(), BorderLayout.SOUTH);
    }

    private JLabel getLblTitleIcon(){
        if(lblTitleIcon == null){
            lblTitleIcon = new JLabel(TITLE_ICON);
        }
        return lblTitleIcon;
    }

    private JLabel getLblBelowTitle(){
        if(lblBelowTitle == null){
            lblBelowTitle = new JLabel(BELOW_TITLE_TEXT, SwingConstants.CENTER);
            lblBelowTitle.setFont(new Font("MS Gothic", Font.PLAIN, 24));
            lblBelowTitle.setForeground(new Color(30,230,120));
        }
        return lblBelowTitle;
    }

    private KeyController getKeyController(){
        if(keyController == null){
            keyController = new KeyController();
        }
        return keyController;
    }

    @Override
    public void update(Observable o, Object arg) {
        if(TitleLogic.getInstance().isTickColorChange()){
            getLblBelowTitle().setForeground(new Color(120, 30, 230));
        }else{
            getLblBelowTitle().setForeground(new Color(30,230,120));
        }
    }

    private class KeyController extends KeyAdapter {
        @Override
        public void keyTyped(KeyEvent e) {
            if(e.getKeyChar() == '\n'){
                Game.getInstance().setState(GameState.MENU);
            }else if(e.getKeyChar() == 'c'){
                TitleLogic.getInstance().tickColorChange();
            }
        }
    }
}

A Window where the background isn't set:

package view.wins;

import game.GameState;
import game.Game;
import logic.MenuLogic;
import view.objs.Button;

import javax.swing.*;
import java.awt.*;
import java.awt.event.MouseAdapter;
import java.awt.event.MouseEvent;
import java.util.Observable;
import java.util.Observer;

public class MenuWindow extends Window implements Observer {
    private JPanel btnPanel;
    private Button btnStartNewGame, btnLoadGame, btnSettings, btnExit;
    private Controller controller;

    public MenuWindow(){
        super("title_background");
        MenuLogic.getInstance().addObserver(this);

        setLayout(new BorderLayout());

        add(getBtnPanel(), BorderLayout.CENTER);
    }

    private JPanel getBtnPanel(){
        if(btnPanel == null){
            btnPanel = new JPanel(new GridLayout(4,1));

            btnPanel.add(getBtnStartNewGame());
            btnPanel.add(getBtnLoadGame());
            btnPanel.add(getBtnSettings());
            btnPanel.add(getBtnExit());
        }
        return btnPanel;
    }

    private Button getBtnStartNewGame(){
        if(btnStartNewGame == null){
            btnStartNewGame = new Button("mediumLong", "Start new game", getController());
        }
        return btnStartNewGame;
    }

    private Button getBtnLoadGame(){
        if(btnLoadGame == null){
            btnLoadGame = new Button("mediumLong", "Load game", getController());
        }
        return btnLoadGame;
    }

    private Button getBtnSettings(){
        if(btnSettings == null){
            btnSettings = new Button("mediumLong", "Settings", getController());
        }
        return btnSettings;
    }

    private Button getBtnExit(){
        if(btnExit == null){
            btnExit = new Button("mediumLong", "Exit", getController());
        }
        return btnExit;
    }

    private Controller getController(){
        if(controller == null){
            controller = new Controller();
        }
        return controller;
    }

    @Override
    public void update(Observable o, Object arg) {

    }

    private class Controller extends MouseAdapter {
        @Override
        public void mouseClicked(MouseEvent e) {
            if(e.getSource().equals(getBtnStartNewGame())){

            }else if(e.getSource().equals(getBtnLoadGame())){

            }else if(e.getSource().equals(getBtnSettings())){
                Game.getInstance().setState(GameState.SETTINGS);
            }else if(e.getSource().equals(getBtnExit())){
                System.exit(0);
            }
        }

        @Override
        public void mouseEntered(MouseEvent e) {
            ((Button)e.getSource()).changeHighlight();
        }

        @Override
        public void mouseExited(MouseEvent e) {
            ((Button)e.getSource()).changeHighlight();
        }
    }
}

Button is a class of my own.

If anyone wants to test it for his own or see more code here is the github repository.

I tried everything and more. I've noticed that the Graphics of MenuWindow aren't initialized, so the problem could come because that window isn't rendered. I don't know.

Answer 1

This...

private void run(){
    while(true){
        if(GameState.changed){
            Screen.getInstance().seeWindow(state);
            GameState.changed = false;
        }else {
            Screen.getInstance().requestFocus(state);
        }
    }
}

is a bad idea. Apart from the fact that a "wild loop" is just generally a bad idea on its own, Swing is also not thread safe and is single threaded.

This means that if this is running within the context of the Event Dispatching Thread, it will block it and prevent from ever been able to process any new events. If it's not running in the EDT, you're risking causing any number of graphic glitches or other "dirty thread" issues.

Add into the fact that a "wild loop" like this will also consume the CPU cycles, you're adding a tremendous performance overhead for little to no gain.

Start by taking a look at Concurrency in Swing

Screen.getInstance().requestFocus(state); is also a hack around the, known, limitations of KeyListener which are better solved through the use the key bindings API

Taking all that into account, Kavaliro should look something more like...

package game;

import view.Screen;

public class Kavaliro {
    private static Kavaliro instance = null;

    private GameState state;

    private Kavaliro() {
        state = GameState.TITLE;
        Screen.getInstance().seeWindow(state);
    }

    public static Kavaliro getInstance() {
        if (instance == null) {
            instance = new Kavaliro();
        }
        return instance;
    }

    public static void main(String[] args) {
        Kavaliro game = Kavaliro.getInstance();
    }

    public void setState(GameState state) {
        Screen.getInstance().seeWindow(state);
    }
}

This is also not a good way to make a singleton in Java.

public static Kavaliro getInstance() {
    if (instance == null) {
        instance = new Kavaliro();
    }
    return instance;
}

You could take a look at Java Singleton Design Pattern Best Practices with Examples and What is an efficient way to implement a singleton pattern in Java? OR you could just make use of Dependency Injection (and probably some research into dependency injection vs singleton)

---

Now, as I said, KeyListener is a generally a poor choice of monitoring user keyboard input, this means that TitleWindow should change, to something a little more like...

package view.wins;

import game.GameState;
import game.Kavaliro;
import game.Utilities;
import logic.TitleLogic;
import utilz.GFXManager;

import javax.swing.*;
import java.awt.*;
import java.awt.event.*;
import java.util.Observable;
import java.util.Observer;

public class TitleWindow extends Window implements Observer {
    private final String BELOW_TITLE_TEXT;
    private final ImageIcon TITLE_ICON;

    private JLabel lblTitleIcon, lblBelowTitle;

    public TitleWindow() {
        super("title_background");
        BELOW_TITLE_TEXT = "Press enter to start";
        TITLE_ICON = new ImageIcon(GFXManager.getInstance().getImage("texts/title.png"));

        TitleLogic.getInstance().addObserver(this);

        setLayout(new BorderLayout());

        Utilities.addKeyBinding(this, Utilities.keyStrokeFor(Utilities.Input.ENTER), new AbstractAction() {
            @Override
            public void actionPerformed(ActionEvent e) {
                Kavaliro.getInstance().setState(GameState.MENU);
            }
        });
        Utilities.addKeyBinding(this, Utilities.keyStrokeFor(Utilities.Input.TITLE_TOGGLE), new AbstractAction() {
            @Override
            public void actionPerformed(ActionEvent e) {
                TitleLogic.getInstance().tickColorChange();
            }
        });

        add(getLblTitleIcon(), BorderLayout.CENTER);
        add(getLblBelowTitle(), BorderLayout.SOUTH);
    }

    private JLabel getLblTitleIcon() {
        if (lblTitleIcon == null) {
            lblTitleIcon = new JLabel(TITLE_ICON);
        }
        return lblTitleIcon;
    }

    private JLabel getLblBelowTitle() {
        if (lblBelowTitle == null) {
            lblBelowTitle = new JLabel(BELOW_TITLE_TEXT, SwingConstants.CENTER);
            lblBelowTitle.setFont(new Font("MS Gothic", Font.PLAIN, 24));
            lblBelowTitle.setForeground(new Color(30, 230, 120));
        }
        return lblBelowTitle;
    }

    @Override
    public void update(Observable o, Object arg) {
        if (TitleLogic.getInstance().isTickColorChange()) {
            getLblBelowTitle().setForeground(new Color(120, 30, 230));
        } else {
            getLblBelowTitle().setForeground(new Color(30, 230, 120));
        }
    }
}

Now, to make life easier, I wrote a quick "utility" class...

package game;

import java.awt.event.KeyEvent;
import javax.swing.Action;
import javax.swing.ActionMap;
import javax.swing.InputMap;
import javax.swing.JComponent;
import static javax.swing.JComponent.WHEN_IN_FOCUSED_WINDOW;
import javax.swing.KeyStroke;

public class Utilities {

    public static enum KeyState {
        PRESSED, RELEASED
    }

    public static enum Input {
        ENTER(KeyEvent.VK_ENTER), TITLE_TOGGLE(KeyEvent.VK_C);

        private int keyEvent;

        private Input(int keyEvent) {
            this.keyEvent = keyEvent;
        }

        protected KeyStroke getKeyStroke(int modifiers, KeyState keyState) {
            return KeyStroke.getKeyStroke(keyEvent, 0, keyState == KeyState.RELEASED ? true : false);
        }
    }

    public static void addKeyBinding(JComponent parent, KeyStroke keyStroke, Action action) {
        addKeyBinding(parent, keyStroke.toString(), keyStroke, action);
    }

    public static void addKeyBinding(JComponent parent, String name, KeyStroke keyStroke, Action action) {
        InputMap inputMap = parent.getInputMap(WHEN_IN_FOCUSED_WINDOW);
        ActionMap actionMap = parent.getActionMap();

        inputMap.put(keyStroke, name);
        actionMap.put(name, action);
    }

    public static KeyStroke keyStrokeFor(Input key) {
        return keyStrokeFor(key, 0, KeyState.PRESSED);
    }

    public static KeyStroke keyStrokeFor(Input key, int modifiers) {
        return keyStrokeFor(key, modifiers, KeyState.PRESSED);
    }

    public static KeyStroke keyStrokeFor(Input key, int modifiers, KeyState keyState) {
        return key.getKeyStroke(modifiers, keyState);
    }

    public static KeyStroke keyStrokeFor(int key) {
        return keyStrokeFor(key, 0, KeyState.PRESSED);
    }

    public static KeyStroke keyStrokeFor(int key, int modifiers) {
        return keyStrokeFor(key, modifiers, KeyState.PRESSED);
    }

    public static KeyStroke keyStrokeFor(int key, int modifiers, KeyState keyState) {
        return KeyStroke.getKeyStroke(key, 0, keyState == KeyState.RELEASED ? true : false);
    }

}

This takes care of a lot of the boiler plate/repeating code, but also provides a means to "limit" the possible inputs to the API. Note the Input enum allows for ENTER and TITLE_TOGGLE as viable options for the keyStrokeFor methods.

Note that TITLE_TOGGLE "hides" the key stroke which is used, but it is pretty self documenting at the same time. There are lots of other ways you could build out these concepts, this is just an example.

And...

public class Button extends JLabel {

would get a very large, no, from me. There is already a button component within the API and you should use it. It even supports things like roll over, for example and example, as well as a verity of other functionalities, which you're going to be spending a lot of time re-building.

See How to Use Buttons, Check Boxes, and Radio Buttons. And, yes, you can remove the Look And Feel fill background and borders if you really want to.

---

I don't know about you, and I don't use Intellij, but return ImageIO.read(new File(GFX_PATH + name)); broke for me.

Resources like these should really be embedded within the application's runtime context (tech babble for "included in the Jar"). This will prevent issues with trying to locate the resources at runtime when the "working directory" context is not the same as the location of the res folder.

You should be using...

return ImageIO.read(getClass().getResource(GFX_PATH + name));

but how you configure Intellji to include the context of the res folder into your application context is beyond me.

---

public void seeWindow(GameState state){
    // Use a CardLayout
    switch(state){
        case TITLE -> setContentPane(getTitleWindow());
        case MENU -> setContentPane(getMenuWindow());
        case SETTINGS -> setContentPane(getSettingsWindow());
    }
    pack();
}

is, well, short sighted. Instead you should be using a CardLayout which will do this for you, and in away which generally will work reliably.

See How to Use CardLayout for more details