a baffling problem

[Randall Twede]

i have a Paint program that i am improving. it used a custom class called StandardColorSelector which used a JComboBox to select a color. i changed it to use a JColorSelector instead. the problem is that now when i change either the background color or the foreground color, it fails to store(or redraw) the previously drawn shapes like it used to . it is pretty long so i will try to select relevant parts that might be the problem. please excuse the indentation.
old

    private StandardColorSelector backcolor = new StandardColorSelector();
    private StandardColorSelector forecolor = new StandardColorSelector();

        public Color getBack()
    {
        return backcolor.getSelectedColor();
    }
        
    public Color getFore()
    {
        return forecolor.getSelectedColor();
    }

new

	private JButton backButton;
	private JButton foreButton;
	private Color backColor;
	private Color foreColor;

    public void actionPerformed(ActionEvent e)
    {
		if(e.getSource() == backButton)
		{
			backColor = JColorChooser.showDialog(this, "Color Chooser", Color.WHITE);
		}
		if(e.getSource() == foreButton)
		{
			foreColor = JColorChooser.showDialog(this, "Color Chooser", Color.BLACK);
		}

    public Color getBack()
    {
        return backColor;
    }
        
    public Color getFore()
    {
        return foreColor;
    }

this is pretty much all i changed

[Steve Luke]

Sorry, that isn't enough information. There is nothing there about 'drawn shapes' or 'saving' so we would have to speculate on everything relevant to the problem. Make a SSCCE (http://sscce.org/) that shows the problem. Make it as small as possible, but make it show the problem...

[Randall Twede]

i was afraid of that. best i can do is post all the code, even though i only changed Paint.java

/*
 * Paint.java
 * Program for drawing shapes
 * Created on February 3, 2004, 11:01 AM
 * this version fixes the "undo  bug" and provides more color choices
 */

/**
 *
 * Randall Twede
 */
import java.awt.*;
import java.awt.event.*;
import javax.swing.*;
import java.io.File;
import javax.imageio.ImageIO;
import java.awt.image.BufferedImage;
import java.io.IOException;
import java.util.List;
import java.util.Arrays;

public class Paint extends JFrame implements ItemListener, ActionListener
{    
    BufferedImage image;	// give DrawingArea access
    private File file;
    private JMenuBar menuBar = new JMenuBar();
    private JMenu fileMenu = new JMenu( "File" );
    private JMenuItem openMenuItem = new JMenuItem( "Open" );
    private JMenuItem saveAsMenuItem = new JMenuItem( "Save As" );
    private JMenu toolsMenu = new JMenu( "Tools" );
    private JMenuItem strokeMenuItem = new JMenuItem( "Stroke" );
    private final JFileChooser fc = new JFileChooser();
    private JPanel topControls = new JPanel();
    private JPanel bottomControls = new JPanel();
    private JRadioButton selected = new JRadioButton();
    private JRadioButton optionsArray[] = new JRadioButton[5];
    private DrawingArea canvas = new DrawingArea(this);
	private JButton backButton;
	private JButton foreButton;
	private Color backColor;
	private Color foreColor;
    private JButton undo;
    private JButton clear;
    private Container c;
    
    /** Creates a new instance of Paint */
    public Paint()
    {
        setTitle("Paint");
        c = getContentPane();
        c.setLayout(new BorderLayout());
        initComponents();
        setJMenuBar(menuBar);
        c.add(topControls, "North");
        c.add(canvas, "Center");
        c.add(bottomControls, "South");
    }
    
    // Improves readability of the constructor for this Class
    private void initComponents()
    {
        openMenuItem.addActionListener(this);
        saveAsMenuItem.addActionListener(this);
        fileMenu.add( openMenuItem );
        fileMenu.add( saveAsMenuItem );
		strokeMenuItem.addActionListener(this);
		toolsMenu.add( strokeMenuItem );
        menuBar.add( fileMenu );
		menuBar.add( toolsMenu );
		backButton = new JButton("Background");
		foreButton = new JButton("Foreground");
		undo = new JButton("Undo");
        clear = new JButton("Clear");
        ButtonGroup optionsGroup = new ButtonGroup();
		//no need to declare length of the array
        String labels[] = {"Draw Rect","Fill Rect","Draw Oval","Fill Oval","Draw Line"};        
        for(int i = 0; i < optionsArray.length; i++)
        {
            optionsArray[i] = new JRadioButton(labels[i]);
            topControls.add(optionsArray[i]);
            optionsArray[i].addItemListener(this);
            optionsGroup.add(optionsArray[i]);
        }
        optionsArray[0].setSelected(true);
        selected = optionsArray[0]; // sets the default selection to Draw Rect
        bottomControls.setLayout(new GridLayout(1, 4));
		bottomControls.add(foreButton);
		foreButton.addActionListener(this);
		bottomControls.add(backButton);
		backButton.addActionListener(this);
        bottomControls.add(undo);
		undo.addActionListener(this);
		bottomControls.add(clear);
        clear.addActionListener(this);
    }
    
    // Determines which JRadioButton was selected
    public void itemStateChanged(ItemEvent e)
    {
        for(int i = 0; i < optionsArray.length; i++)
        {
            if(optionsArray[i].isSelected())
            {
                selected = optionsArray[i];
            }
        }
    }
            
    public void actionPerformed(ActionEvent e)
    {
		if(e.getSource() == backButton)
		{
			backColor = JColorChooser.showDialog(this, "Color Chooser", Color.WHITE);
		}
		if(e.getSource() == foreButton)
		{
			foreColor = JColorChooser.showDialog(this, "Color Chooser", Color.BLACK);
		}
       	if(e.getSource() == undo)
		{
			canvas.undo();
		}
		else if(e.getSource() == openMenuItem)
		{
			open();
		}
		else if(e.getSource() == saveAsMenuItem)
		{
			save();          
		}
		else if(e.getSource() == strokeMenuItem)
		{
			JOptionPane choice = new JOptionPane();
			int stroke = Integer.parseInt(choice.showInputDialog("Choose a width. The defaut is 1", "1"));
			canvas.setStroke(stroke);	    
		}
        else
        {
            canvas.clear();
        }
    }
                            
    // Returns a reference to the selected JRadioButton
    public JRadioButton getSelected()
    {
        return selected;
    }
    
    public Color getBack()
    {
        return backColor;
    }
        
    public Color getFore()
    {
        return foreColor;
    }

    private void open()
    {
        int returnVal = fc.showOpenDialog(this);
        if (returnVal == JFileChooser.APPROVE_OPTION)
        {
            try
            {
                file = fc.getSelectedFile();  
                image = ImageIO.read(file);
                if(image != null)
				{
					canvas.clear();
					image = ImageIO.read(file);	//work around
                    // Call paintComponent()
                    canvas.repaint();
				}
				else
				{
                    String message = "This program can only open image files, like gif or jpg";
                    JOptionPane.showMessageDialog( this, message, "Error", JOptionPane.ERROR_MESSAGE );
				}
            }
            catch (IOException ex)
            {
				String message = "could not open file";
				JOptionPane.showMessageDialog( this, message, "IOException", JOptionPane.ERROR_MESSAGE );      
			}           
		}
    }
    private void save()
    {
	    List<String> types = Arrays.asList( ImageIO.getWriterFileSuffixes() );
		int returnVal = fc.showSaveDialog(this);
        if (returnVal == JFileChooser.APPROVE_OPTION)
        {
			try
			{
                File file = fc.getSelectedFile();
				String fileName = file.getPath();
				if (fileName == null) return;
				int offset = fileName.lastIndexOf( "." );
				if (offset == -1)
				{
					String message = "file suffix was not specified";
					JOptionPane.showMessageDialog( this, message, "Error", JOptionPane.ERROR_MESSAGE );
				}
				else
				{
					String type = fileName.substring(offset + 1);
					if (types.contains(type))
					{
						Dimension d = canvas.getSize();
						Rectangle region = new Rectangle(0, 0, d.width, d.height);
						image = new BufferedImage(region.width, region.height, BufferedImage.TYPE_INT_RGB);
						Graphics2D g2d = image.createGraphics();
						canvas.paint( g2d );
						g2d.dispose();
						ImageIO.write(image, type, new File( fileName ));
					}
					else
					{
						String message = "unknown writer file suffix (" + type + ")";
						JOptionPane.showMessageDialog( this, message, "Error", JOptionPane.ERROR_MESSAGE );
					}
				}
			}
			catch(IOException ex)
			{
				String message = "could not save file";
				JOptionPane.showMessageDialog( this, message, "IOException", JOptionPane.ERROR_MESSAGE );
			}           
	    }
    }
        
    // Creates an instance of Paint
    public static void main(String args[])
    {
        Paint test = new Paint();
        test.setSize(500, 500);
        test.setLocation(200, 100);
        test.setVisible(true);
        test.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    }
    

    
}

/*
 * DrawingArea.java
 * Class for drawing shapes
 * Created on February 4, 2004, 12:03 PM
 */

/**
 *
 * Randall Twede
 */
import java.awt.*;
import java.awt.event.*;
import javax.swing.*;
import java.util.Vector;

public class DrawingArea extends JPanel
{
   private MyMouseListener theListener = new MyMouseListener();
   private Vector shapes = new Vector();
   private Point start = new Point();
   private Point end = new Point();
   private Paint parent;    // Reference to the JFrame containing this Panel
   private Shape current;
   private int x1 = 0;
   private int x2 = 0;
   private int y1 = 0;
   private int y2 = 0;
   private int w = 0;
   private int h = 0;
   private int stroke = 1;
    
    /** Creates a new instance of DrawingArea */
    public DrawingArea(Paint p)
    {
        parent = p;
        setBorder( BorderFactory.createTitledBorder( "Drawing Area" ) );
        addMouseListener(theListener);
		addMouseMotionListener(theListener);
    }
    
    public void paintComponent(Graphics g)
    {
        Shape shape;
        
        super.paintComponent(g);
        setBackground(parent.getBack());    // Set the background color
    	// Draws the image if there is one
        if(parent.image != null)
		{
            g.drawImage(parent.image, 0, 0, null);
		}
    	// Draws all the shapes stored in the vector
        for(int i = 0; i < shapes.size(); i++)
        {
            shape = (Shape)shapes.elementAt(i);
            shape.draw(g);
        }
    	// Draws the current Shape
		if(current != null)
		{
			current.draw(g);
		}
    }

    // Sets the width of the lines
    public void setStroke(int stroke)
    {
		this.stroke = stroke;
    }

    //Erases the last Shape drawn 
    public void undo()
    {
		if(!shapes.isEmpty())	//fixes undo bug
		{
			shapes.removeElementAt(shapes.size() - 1);
			current = null;
			repaint();
		}
    }
   
    // Clears the drawing area
    public void clear()
    {
        parent.image = null;
        shapes.clear();
		current = null;
        repaint();
    }

    // Determines top left coordinates, width, and height
    private void setParameters()
    {
        x1 = start.x;
        y1 = start.y;
        x2 = end.x;
        y2 = end.y;
        int temp = 0;
        // Swap if necessary
        if (x1 > x2)
        {
            temp = x1;
            x1 = x2;
            x2 = temp;
        }
        // Swap if necessary
        if (y1 > y2)
        {
            temp = y1;
            y1 = y2;
            y2 = temp;
        }
        w = x2 - x1;
        h = y2 - y1;
    }

    class MyMouseListener extends MouseAdapter
    {
        // Gets the starting point
        public void mousePressed(java.awt.event.MouseEvent mouseEvent)
        {
            start = mouseEvent.getPoint();
        }

        //Stores the Shape in a Vector
        public void mouseReleased(java.awt.event.MouseEvent mouseEvent)
        {
            shapes.add(current);
        }

	// Creates a Shape object    
        public void mouseDragged(java.awt.event.MouseEvent mouseEvent)
        {
            JRadioButton shape = parent.getSelected();	// Retrieves a referenced to the selected JRadioButton
            String type = shape.getText();        	// Sets the type of shape to draw
            Color color = parent.getFore();		// Sets the drawing color
            end = mouseEvent.getPoint();		// Gets the ending point
            setParameters();				// Calls a private method
			if (type.equals("Draw Line"))
			{
				current = new Shape(start.x, start.y, end.x, end.y, type, color, stroke);
			}
			else
			{
				current = new Shape(x1, y1, w, h, type, color, stroke);
			}
            repaint();
        }
    }
}

/*
 * Shape.java
 * Class to represent a shape
 * Created on February 4, 2004, 12:02 PM
 * last modified 12/2011
 */

/**
 *
 * Randall Twede
 */
import java.awt.*;

public class Shape
{
    private int x;
    private int y;
    private int w;
    private int h;
    private String shape;
    private Color color; 
    private int stroke;
    
    /** Creates a new instance of Shape */
    public Shape(int horiz, int vert, int width, int height, String shape, Color color, int stroke)
    {
        x = horiz;
        y = vert;
        w = width;
        h = height;
        this.shape = shape;
        this.color = color;
	this.stroke = stroke;
    }

    // Draws this shape object
    public void draw(Graphics g)
    {
	Graphics2D g2d = (Graphics2D)g;
        g2d.setRenderingHint(RenderingHints.KEY_ANTIALIASING,  
        RenderingHints.VALUE_ANTIALIAS_ON);
        g2d.setColor(color);    
	g2d.setStroke(new BasicStroke(stroke));
        // Determine which shape to draw
        if(shape.equals("Draw Rect"))
        {
            g2d.drawRect(x, y, w, h);
        }
        else if(shape.equals("Fill Rect"))
        {
            g2d.fillRect(x, y, w, h);
        }
        else if(shape.equals("Draw Oval"))
        {
            g2d.drawOval(x, y, w, h);
        }
        else if(shape.equals("Fill Oval"))
        {
            g2d.fillOval(x, y, w, h);
        }
        else if(shape.equals("Draw Line"))
        {
            g2d.drawLine(x, y, w, h);
        }
    }
}

/*
 * StandardColorSelector.java
 * Class to present a drop down list of Java's standard colors
 * Created on February 13, 2004, 10:00 AM
 */

/**
 *
 * Randall Twede
 */
import java.awt.*;
import javax.swing.*;

public class StandardColorSelector extends JPanel
{
    private String names[] = {"Black", "Blue", "Cyan",
             "Dark Gray", "Gray", "Green", "Light Gray",
             "Magenta", "Orange", "Pink", "Red",
             "White", "Yellow"};
    private Color colors[] = {Color.black, Color.blue, Color.cyan, Color.darkGray,
             Color.gray, Color.green, Color.lightGray, Color.magenta,
             Color.orange, Color.pink, Color.red, Color.white, Color.yellow};
    private JComboBox selector = new JComboBox(names);
    
    /** Creates a new instance of StandardColorSelector */
    public StandardColorSelector()
    {
        selector.setEditable(false);
        selector.setSelectedIndex(0);	// Default selection is black
        add(selector);
    }
    
    public Color getSelectedColor()
    {
        return colors[selector.getSelectedIndex()];
    }

    // Requires an integer parameter between 0 and 12 inclusive
    public void setDefaultSelection(int selection)
    {
        selector.setSelectedIndex(selection);
    }
    
}

[Randall Twede]

a little further information. say i set the background to a pink color and the foreground to say a green color and draw something. if i then try to change the background color, but change my mind and choose cancel, it eliminates what i drew and changes the background to the default of white. as i mentioned, the program works fine using the StandardColorSelector(JComboBox).

[Steve Luke]

Randall Twede wrote:a little further information. say i set the background to a pink color and the foreground to say a green color and draw something. if i then try to change the background color, but change my mind and choose cancel, it eliminates what i drew and changes the background to the default of white.

That is as it should be. When you cancel, the default value returned is Color.WHITE because that is what:

backColor = JColorChooser.showDialog(this, "Color Chooser", Color.WHITE);

Says to do. If you want it to revert to last-selected-background you would substitute Color.WHITE to the last backColor:

backColor = JColorChooser.showDialog(this, "Color Chooser", backColor);

as i mentioned, the program works fine using the StandardColorSelector(JComboBox).

Your first two conditions in the actionPerformed are if{}. Then you do a long line of if {} else if {} else if {} ... else {}. When you do the backButton, for example, it will execute the first if, find out it is true and so execute that code. Then it will do the second if, find out it is false and skip it. Then it will execute the third if, find it is false and fall to the else if, find it is false and fall to the next else if, etc... until it gets to else {}. Since there is no condition on the else, and since the previous if was false, the code inside the else{} gets executed. What does that code do?

This is why you have different actionListeners for each button. Makes inappropriate button interaction less likely.

[Randall Twede]

thank you Steve!
that is obvious now.
when i added the two new buttons to actionPerformed, i started them both with if!!!
so i have if, if ,if, else if
when i should have if, else if, else if, else if
as for seperate listeners, i have been chided for that before

[Randall Twede]

that is much better. i am surprised i did'nt see it myself
i just have one problem now.
i choose background and foreground and draw. i change foreground and draw again. everything is fine.
but if i change background, the background color doesn't change until i draw something else. i am thinking i need to call repaint() when i change background. with the old program, the background color changed as soon as i chose a new one, now it waits untill i draw something new. then it does change

[Bill Clar]

Did the old program run on an older JVM? I'm guessing but, maybe the need to call repaint() directly was an added feature in newer JVMs?

[Steve Luke]

Or possibly, a repaint() was called automatically as a side effect of the combobox selection or something.

[Randall Twede]

i think Steve is correct here. i think the JVM used to call it for me(because of the JComboBox), but now i have to call it myself.
one or two final points for anyone interested. the API can be your best friend. it told me that if the user chooses cancel or closes the dialog from the title bar it will return null. apparently the color you pass to the constructor is only for if the user makes no choice and clicks OK.

it works fine now but i am still a bit baffled. i have no idea how white and black are the default background and foreground colors. to be safe i am going to assign those values when i declare the variables. also for some reason the foreground works ok the way it was but i needed a few lines of extra code for the background. again i am going to err on the side of safety by making them the same.

    public void actionPerformed(ActionEvent e)
    {
		if(e.getSource() == backButton)
		{
			Color temp = JColorChooser.showDialog(this, "Color Chooser", backColor);
			if(temp != null)
			{
				backColor = temp;
			}
			repaint();
		}
		else if(e.getSource() == foreButton)
		{
			foreColor = JColorChooser.showDialog(this, "Color Chooser", foreColor);
		}

the only thing left to do is add a tab to the JColorChooser so the user can use the old "JComboBox" to choose. the tutorial is pretty clear on how to do that.