Hello All,

I am fairly new to Java programming (only a couple of months) and completely new to graphics in Java (couple days).

Below is the relevent paintComonent function I wrote. Basically it draws a circuit based on a few member variables. This part works fine.

The problem I am seeing is that the background starts to collect various parts of the Applet, instead of staying white. For example I have in the same applet a label with an image, part of this is displayed under my graphics. I could mask the problem by always clearing the whole panel before drawing, but it looks like memory is either getting corrupted or at least not cleared before being used by my paintComponet function. So I am not sure if I should be looking for a bigger issue somewhere else.

Any thoughts? has anyone seen something like this before?

public class SwitchGraphicsPanel extends javax.swing.JPanel
{
...
    @Override
    public void paintComponent(Graphics g)
    {
        super.paintComponents(g);
        int nOffset = 80;
        int nOffsetTxt = 80;
        int newCirState = 0;
        
        Graphics2D g2d = (Graphics2D)g;
        g2d.setFont(new Font("Arial", Font.BOLD, 18));
        g2d.setStroke(TraceLine);

        // <editor-fold defaultstate="collapsed" desc="Erase for New State">
        if(m_bS1Bypass || m_bS2Bypass)
        {
            if(m_bBypassIso)
            {
                nOffsetTxt = 160;
                nOffset = 2;
                newCirState =1+ (m_bS1Bypass ? 2:0) + (m_bS2Bypass ?4:0);
//                newCirState = 2;
            }
            else
            {
                nOffsetTxt = 160;
                nOffset = 35;
                newCirState = (m_bS1Bypass ? 2:0) + (m_bS2Bypass ?4:0);
            }
        }

        if(m_nCirState != newCirState)
        {   //erase full circuit
            g2d.clearRect(0, 0, 250, 200);
            m_nCirState = newCirState;
        }
        else
        {//erase switches only
            g2d.clearRect(4 + nOffset, 37, 20, 30);
            g2d.clearRect(4 + nOffset, 130, 20, 30);
            if(m_bS1Bypass || m_bS2Bypass )
            {
                g2d.clearRect(127, 83, 26, 16);
                if(m_bS1Bypass)
                {
                    g2d.clearRect(127, 44, 26, 16);
                }
                if(m_bS2Bypass )
                {
                    g2d.clearRect(127, 142, 26, 16);
                }
            }
        }
        // </editor-fold>
        // <editor-fold defaultstate="collapsed" desc="S1 Power">
        if(m_bS1Power)
            g2d.setColor(Color.red);
        else
            g2d.setColor(Color.black);
        // draw circle (color already set to foreground)
        g2d.drawString("S1", 65 + nOffsetTxt, 18);

        if(m_bS1Bypass )
        {
            //  x
            //  |
            //  -o
            g2d.fillOval(95, 4, 12, 12);
            g2d.drawLine(101, 10, 101, 60); //   |
            g2d.drawLine(101, 60, 115, 60); //   -
            g2d.drawOval(115, 54, 12, 12);  //  o

            //s1 Bypass Switch
            if(m_bS1BypassPos )
            {
                g2d.drawLine(127, 60, 148 , 52);
            }
            else
            {
                g2d.drawLine(127,60, 142 , 44);
            }

        }

        if(m_bS1Bypass || m_bS2Bypass )
        {
            //Source 1
            g2d.drawLine(220, 0, 210, 10);  //   /
            g2d.drawLine(210, 10, 220, 20); //   \
            g2d.drawLine(210, 10, 95, 10); //   -
            if(m_bBypassIso)
            {
                g2d.drawLine(75, 10, 105, 10); //   -
                g2d.drawLine(85, 0, 75, 10);  //   /
                g2d.drawLine(75, 10, 85, 20); //   \
                g2d.setColor(Color.black);
            }

        }

        //Source 1
        g2d.drawLine(60 + nOffset, 0, 50 + nOffset, 10);  //   /
        g2d.drawLine(50 + nOffset, 10, 60 + nOffset, 20); //   \
        g2d.drawLine(50 + nOffset, 10, 20 + nOffset, 10); //   -
        g2d.drawLine(20 + nOffset, 10, 20 + nOffset, 35); //  |
        g2d.drawOval(14+ nOffset, 35, 12, 12);  //  o
// </editor-fold>
        // <editor-fold defaultstate="collapsed" desc="S2 Power">

        if(m_bS2Power)
            g2d.setColor(Color.red);
        else
            g2d.setColor(Color.black);
        g2d.drawString("S2", 65 + nOffsetTxt, 195);
        
        if(m_bS2Bypass )
        {
            g2d.fillOval(95, 182, 12, 12);
            g2d.drawLine(101, 188, 101, 158); //   |
            g2d.drawLine(101, 158, 115, 158); //   -
            g2d.drawOval(115, 152, 12, 12);  //  o
            //s2 Bypass Switch
            if(m_bS2BypassPos )
            {
                g2d.drawLine(127,158 , 148 , 150);
            }
            else
            {
                g2d.drawLine(127, 158, 142, 142);
            }

        }

        if(m_bS1Bypass || m_bS2Bypass )
        {
            //Source 2
            g2d.drawLine(220, 178, 210, 188);  //   /
            g2d.drawLine(210, 188, 220, 198); //   \
            g2d.drawLine(210, 188, 95, 188); //   -
            if(m_bBypassIso)
            {
                g2d.drawLine(75, 188, 105, 188); //   -
                g2d.drawLine(85, 178, 75, 188);  //   /
                g2d.drawLine(75, 188, 85, 198); //   \
                g2d.setColor(Color.black);
            }
        }

        //Source 2
        g2d.drawLine(60 + nOffset, 178, 50 + nOffset, 188); //   /
        g2d.drawLine(50 + nOffset, 188, 60 + nOffset, 198); //   \
        g2d.drawLine(50 + nOffset, 188, 20 + nOffset, 188);  //   -
        g2d.drawLine(20 + nOffset, 188, 20 + nOffset, 163);  //  |
        g2d.drawOval(14+ nOffset, 151, 12, 12);  //  o
        // </editor-fold>
        // <editor-fold defaultstate="collapsed" desc="Load">

        if(!m_bBypassIso 
                && ((m_bS1Power && m_bS1Switch && !(m_bS1BypassPos||m_bS2BypassPos))
                        || (m_bS2Power && m_bS2Switch && !(m_bS1BypassPos||m_bS2BypassPos)))
                    || ((m_bS1Power && m_bS1BypassPos && m_bS1Bypass)
                        || (m_bS2Power && m_bS2BypassPos && m_bS2Bypass))    )
            g2d.setColor(Color.red);
        else
            g2d.setColor(Color.black);
        g2d.drawString("L", 65 + nOffsetTxt, 106);

        if(m_bS1Bypass || m_bS2Bypass )
        {
            //Load
            g2d.drawLine(220, 89, 210, 99);  //   /
            g2d.drawLine(210, 99, 220, 109); //   \
            g2d.drawLine(210, 99, 153, 99); //   -
            g2d.fillOval(167, 93, 12, 12);
            g2d.drawOval(141, 93, 12, 12);  //  o

            
            if(m_bS1Bypass)
            {
                g2d.drawOval(141, 54, 12, 12);  //  o
                g2d.drawLine(153,60,173,60);   //   -
                g2d.drawLine(173,99,173,60);   //   |
            }
            if(m_bS2Bypass)
            {
                g2d.drawLine(173,99,173,158);   //   |
                g2d.drawLine(153,158,173,158);   //   -
                g2d.drawOval(141, 152, 12, 12);  //  o

            }
            if(m_bBypassIso)
            {
                g2d.setColor(Color.black);
                g2d.drawLine(75, 99, 105, 99); //   -
                g2d.drawLine(85, 89, 75, 99);  //   /
                g2d.drawLine(75, 99, 85, 109); //   \
            }
            else
            {
                if((m_bS1Switch && m_bS1Power)
                    || (m_bS2Switch && m_bS2Power))
                    g2d.setColor(Color.red);
            }
            //Load Bypass Switch
            if((!m_bS1Switch && !m_bS2Switch)
                    || (m_bS1Switch && !m_bS1Power)
                    || (m_bS2Switch && !m_bS2Power))
                g2d.setColor(Color.black);

            if(m_bS1BypassPos || m_bS2BypassPos)
            {
                g2d.drawLine(127,99, 142 , 83);
            }
            else
            {
                g2d.drawLine(127,99, 148 , 91);
            }
            g2d.drawOval(115, 93, 12, 12);  //  o
            g2d.drawLine(115, 99, 95, 99); //   -

      }


        //Load
        g2d.drawOval(14+ nOffset, 65, 12, 12);  //  o
        g2d.drawOval(14+ nOffset, 120, 12, 12);  //  o
        g2d.drawLine(20 + nOffset, 78, 20 + nOffset, 120);  //  |
        g2d.fillOval(14+ nOffset, 93, 12, 12);
        g2d.drawLine(60 + nOffset, 89, 50 + nOffset, 99); //   /
        g2d.drawLine(50 + nOffset, 99, 60 + nOffset, 109); //   \
        g2d.drawLine(50 + nOffset, 99, 20 + nOffset, 99);  //   -


        //S1 ATS Switch
        if(m_bS1Switch)
            g2d.drawLine(20 + nOffset, 65, 12 + nOffset, 39);
        else
            g2d.drawLine(20 + nOffset, 65, 4 + nOffset, 45);

        //S2 ATS Switch
        if(m_bS2Switch)
            g2d.drawLine(20 + nOffset, 132, 12 + nOffset, 157);
        else
            g2d.drawLine(20 + nOffset, 132, 4 + nOffset, 150);
// </editor-fold>


    }
}

Note that you are calling super.paintComponents(g); , not super.paintComponent(g); . That "s" makes a difference.

@PBoucher Also consider moving stuff in some reasonable methods that puts close related stuff together.

  • Try to use meaningful names, stuff like "m_bS2Bypass" that is presumably IDE generated variable name is just rubbish.
  • Simplify if/else statements as this is disaster and can easily lead logical flows
    if(!m_bBypassIso 
                    && ((m_bS1Power && m_bS1Switch && !(m_bS1BypassPos||m_bS2BypassPos))
                            || (m_bS2Power && m_bS2Switch && !(m_bS1BypassPos||m_bS2BypassPos)))
                        || ((m_bS1Power && m_bS1BypassPos && m_bS1Bypass)
                            || (m_bS2Power && m_bS2BypassPos && m_bS2Bypass))    )

    Wouldn't be easier to establish when to use black colour instead of this complex statement for red?

  • Check for repeating patterns
    //Source 1
                g2d.drawLine(220, 0, 210, 10);  //   /
                g2d.drawLine(210, 10, 220, 20); //   \
                g2d.drawLine(210, 10, 95, 10); //   -
                if(m_bBypassIso)
                {
                    g2d.drawLine(75, 10, 105, 10); //   -
                    g2d.drawLine(85, 0, 75, 10);  //   /
                    g2d.drawLine(75, 10, 85, 20); //   \
                    g2d.setColor(Color.black);
                }

    and replace them with simple method calls where you just pass some data for computation

    drawLines(g2d, Arrays.asList(new DrawPoints(220, 0, 210, 10), new DrawPoints(210, 10, 220, 20), new DrawPoints(210, 10, 95, 10));
    
    
    public void drawLines(Graphics2D g2d, List<DrawPoints> list){
        for(DrawPoint each : list){
            g2d.drawLine(each.getX1(), each.getY1(), each.getX2(), each.getY2());
        } 
    }

It looks like you are updating the previous image by writing blank rectangles in various places, so you really don't want to call super.paintComponent (and absolutely not super.paintComponents) if you expect images to persist from one call to the next.

Note that you are calling super.paintComponents(g); , not super.paintComponent(g); . That "s" makes a difference.

Thank you Ezzaral. That typo seemed to be the problem

@PBoucher Also consider moving stuff in some reasonable methods that puts close related stuff together.

  • Try to use meaningful names, stuff like "m_bS2Bypass" that is presumably IDE generated variable name is just rubbish.
  • Simplify if/else statements as this is disaster and can easily lead logical flows
    if(!m_bBypassIso 
                    && ((m_bS1Power && m_bS1Switch && !(m_bS1BypassPos||m_bS2BypassPos))
                            || (m_bS2Power && m_bS2Switch && !(m_bS1BypassPos||m_bS2BypassPos)))
                        || ((m_bS1Power && m_bS1BypassPos && m_bS1Bypass)
                            || (m_bS2Power && m_bS2BypassPos && m_bS2Bypass))    )

    Wouldn't be easier to establish when to use black colour instead of this complex statement for red?

  • Check for repeating patterns
    //Source 1
                g2d.drawLine(220, 0, 210, 10);  //   /
                g2d.drawLine(210, 10, 220, 20); //   \
                g2d.drawLine(210, 10, 95, 10); //   -
                if(m_bBypassIso)
                {
                    g2d.drawLine(75, 10, 105, 10); //   -
                    g2d.drawLine(85, 0, 75, 10);  //   /
                    g2d.drawLine(75, 10, 85, 20); //   \
                    g2d.setColor(Color.black);
                }

    and replace them with simple method calls where you just pass some data for computation

    drawLines(g2d, Arrays.asList(new DrawPoints(220, 0, 210, 10), new DrawPoints(210, 10, 220, 20), new DrawPoints(210, 10, 95, 10));
    
    
    public void drawLines(Graphics2D g2d, List<DrawPoints> list){
        for(DrawPoint each : list){
            g2d.drawLine(each.getX1(), each.getY1(), each.getX2(), each.getY2());
        } 
    }

-Unfortunately names like "m_bS2Bypass" ARE meaningful m_ = member var b = bool S2Bypass is End users term for this part of the circuit (source 2's Bypass relay)

- the hideous logic is just the first pass at getting it to work correctly. it works and if I have time I will optimize it.

- Same for the repeating patterns, this is the first pass it works and if I have time I will consolidate it. but as it stands this part is low priority.

It looks like you are updating the previous image by writing blank rectangles in various places, so you really don't want to call super.paintComponent (and absolutely not super.paintComponents) if you expect images to persist from one call to the next.

Thank you James,

Fixing the typo with paintCompnent fixed my problem and let me remove the clearRect calls

This article has been dead for over six months. Start a new discussion instead.