A Java Swing puzzle: What doesn't the first iteration display on the JPanel?

Question

Here's one I've come close to figuring out, but never really did.

The code listed below is supposed to draw a green circle as soon as it sees its first click. It doesn't. Subsequent clicks draw lines that connect the current clicked point with the previous one, in red. The code fails for the first click and works on all subsequent ones. Why doesn't the first click display? It runs!

What am I doing wrong?

The code should compile on any current JDE.

TIA

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

class Demo extends JFrame
            implements  ActionListener, ListSelectionListener, MouseListener {

  int clkCt = 0, // Count of the number of clicks we've done.
      oldX,      // Penultimate X value
      oldY,      // Penultimate X value
      scrH,      // Height of the drawing canvas.
      scrW;      // Width of the drawing canvas.

  JFrame f;      // Holder for the drawing canvas.

  JLabel ctL;    // Displays the number of clicks we've done.

  JPanel canvas; // The drawing canvas.

  public void demoLines() {

    Dimension d = Toolkit.getDefaultToolkit().getScreenSize();
    scrH = (int) ((double) d.height * 0.75);
    scrW = (int) ((double) d.width * 0.75);

    oldX = scrH / 2;
    oldY = oldX;

    // Create and set up the window.
    f = new JFrame("Multi Click Demo");
    f.getContentPane().setLayout(null);
    int h = scrH / 5;
    f.setBounds(h, h, scrW, scrH);

    // Create a panel
    canvas = new JPanel();
    canvas.setBackground(Color.black);
    canvas.setForeground(Color.red);
    canvas.setLayout(null);
    canvas.setBounds(0, 0, scrW, scrH);
    canvas.setPreferredSize(new Dimension(scrW, scrH));
    canvas.addMouseListener(this);
    f.getContentPane().add(canvas);

    // Create the exit button.
    JButton exit = new JButton("Exit");
    exit.addActionListener(new ActionListener() {
      public void actionPerformed(ActionEvent e) {
        goAway();
      }
    });
    exit.setBackground(Color.black);
    exit.setForeground(Color.red);
    exit.setBounds(0, 0, (scrW / 15), (scrH / 15));
    canvas.add(exit); //*/

    // Create the label for the click count.
    ctL = new JLabel("None Yet");
    ctL.setBackground(Color.black);
    ctL.setForeground(Color.red);
    ctL.setBounds((scrH / 15), (scrH * 13 / 15), (scrW / 15), (scrH / 15));
    canvas.add(ctL);

    f.getContentPane().add(canvas);
    f.setVisible(true);

    Graphics g = canvas.getGraphics();
    if (g == null) {
      System.out.println("No graphics for canvas!");
    } else {
      canvas.revalidate(); // This didn't help.
      paintComponent(g, (oldX + oldX / 2), (oldY + oldY / 2));
    }
  }

  void goAway() {
    f.setVisible(false);
    f.dispose();
  }

  public void mouseClicked(MouseEvent m) {
    // Where was the mouse clicked?
    int clkdBtn = m.getButton(),
        x = m.getX(),
        y = m.getY();
    Graphics g = canvas.getGraphics();
    paintComponent(g, x, y);
  }

  public void paintComponent(Graphics g,
                             int x,
                             int y) {

    // This always runs.
    ctL.setText(clkCt + "");

    if (clkCt == 0) {
      // This never displays!
      g.setColor(Color.green);
      int r = scrH * 4 / 5;
      g.drawOval((scrH / 10), (scrH / 10), r, r);
    }

   g.setColor(Color.red);
   g.drawLine(oldX, oldY, x, y);
   oldX = x;
    oldY = y;
    clkCt++;
  }


  public void actionPerformed(ActionEvent event) { }
  public void valueChanged(ListSelectionEvent event) { }

  public void mouseEntered(MouseEvent e) { }
  public void mouseExited(MouseEvent e) { }
  public void mousePressed(MouseEvent e) { }
  public void mouseReleased(MouseEvent e) { }

  public static void main(String[] s) {

    Demo m = new Demo();
    m.demoLines();
  }
}

Reimeus · Answer 1 · 2013-06-16T20:50:26.923

What am I doing wrong?

You're using getGraphics for custom painting. Any previous painting will be lost when a repaint is called. Instead move all the custom painting functionality to a new class based on JComponent or JPanel and override paintComponent there. Remember to invoke super.paintComponent(g).

See Custom Painting Approaches for solutions for drawing multiple components from within paintComponent. You can build a List<Shape> of custom drawable components, iterate through the list from within the method, and use drawLine or drawOval as appropriate.

score 2 · Answer 2 · answered Jun 16 '13 at 20:19

2

remove the following lines from demolines()

Graphics g = canvas.getGraphics();
if (g == null) {
  System.out.println("No graphics for canvas!");
} else {
  canvas.revalidate(); // This didn't help.
  paintComponent(g, (oldX + oldX / 2), (oldY + oldY / 2));
}

and add canvas.addMouseListener(this); in the end of the function

answered Jun 16 '13 at 20:19

DarthCoder

354
1
11

That Worked! Nice and easy. Thanks! – Boffin Jun 18 '13 at 20:49

score 2 · Answer 3 · answered Jun 17 '13 at 06:48

There were a multitude of problems with that code. I fixed them but neglected to document every change. Look closely over this code and ask questions if you do not understand (from reading the relevant documentation/tutorial) why I made the change.

enter image description here

import java.awt.*;
import java.awt.event.*;
import javax.swing.*;
import javax.swing.event.*;
import javax.swing.border.EmptyBorder;

public class Demo extends JPanel
        implements ListSelectionListener, MouseListener {

    int clkCt = 0, // Count of the number of clicks we've done.
            oldX, // Penultimate X value
            oldY, // Penultimate X value
            scrH = 100, // Height of the drawing canvas.
            scrW = 400;      // Width of the drawing canvas.
    JLabel ctL;    // Displays the number of clicks we've done.
    int x, y;

    public void demoLines() {
        oldX = scrH / 2;
        oldY = oldX;

        // Create a panel
        setBackground(Color.black);
        setForeground(Color.red);
        setPreferredSize(new Dimension(scrW, scrH));

        // Create the label for the click count.
        ctL = new JLabel("None Yet");
        ctL.setBackground(Color.black);
        ctL.setForeground(Color.red);
        ctL.setBounds((scrH / 15), (scrH * 13 / 15), (scrW / 15), (scrH / 15));
        add(ctL);
        addMouseListener(this);
    }

    public void mouseClicked(MouseEvent m) {
        // Where was the mouse clicked?
        x = m.getX();
        y = m.getY();
        repaint();
    }

    @Override
    public void mousePressed(MouseEvent e) {}

    @Override
    public void mouseReleased(MouseEvent e) {}

    @Override
    public void mouseEntered(MouseEvent e) {}

    @Override
    public void mouseExited(MouseEvent e) {}

    public void paintComponent(Graphics g) {

        // This always runs.
        ctL.setText(clkCt + "");

        if (clkCt == 0) {
            // This never displays!
            g.setColor(Color.green);
            int r = scrH * 4 / 5;
            g.drawOval((scrH / 10), (scrH / 10), r, r);
        }

        g.setColor(Color.red);
        g.drawLine(oldX, oldY, x, y);
        oldX = x;
        oldY = y;
        clkCt++;
    }

    public void valueChanged(ListSelectionEvent event) {
    }

    public static void main(String[] args) {
        Runnable r = new Runnable() {

            @Override
            public void run() {
                Demo m = new Demo();
                m.demoLines();
                JFrame f = new JFrame("Demo");
                f.add(m);
                // Ensures JVM closes after frame(s) closed and
                // all non-daemon threads are finished
                f.setDefaultCloseOperation(JFrame.DISPOSE_ON_CLOSE);
                // See http://stackoverflow.com/a/7143398/418556 for demo.
                f.setLocationByPlatform(true);

                // ensures the frame is the minimum size it needs to be
                // in order display the components within it
                f.pack();
                // should be done last, to avoid flickering, moving,
                // resizing artifacts.
                f.setVisible(true);
            }
        };
        // Swing GUIs should be created and updated on the EDT
        // http://docs.oracle.com/javase/tutorial/uiswing/concurrency/initial.html
        SwingUtilities.invokeLater(r);
    }
}

Notes

Don't extend frame or other top level containers. Instead create & use an instance of one.
Java GUIs might have to work on a number of platforms, on different screen resolutions & using different PLAFs. As such they are not conducive to exact placement of components. To organize the components for a robust GUI, instead use layout managers, or combinations of them, along with layout padding & borders for white space.
Instead of painting in a top level container such as JFrame, add a JPanel & do custom painting in the paintComponent(Graphics) method. Also return a sensible preferred size for the custom component, to assist the layout manager.
For custom painting in a JComponent, override paintComponent(Graphics) instead of paint(Graphics).

Thanks for the code. It works, but doesn't change the background color. This is important for the larger project I'm working on. When I put the JPanel in a frame, the color changed. — Boffin, Jun 18 '13 at 20:49

score 1 · Answer 4 · answered Jun 16 '13 at 20:28

1

You should always call super.paintComponent() as the first line when you override paintComponent().

answered Jun 16 '13 at 20:28

Code-Apprentice

81,660
23
145
268

*"..when you override `paintComponent()`."* Yes, you should, but if you look at the code carefully you might notice it does ***not*** override the method, because 1) it includes extra arguments over the usual method, and 2) `JFrame` has no `paintComponent(Graphics)` method! – Andrew Thompson Jun 17 '13 at 06:29

A Java Swing puzzle: What doesn't the first iteration display on the JPanel?

4 Answers4

Notes