corrupted background in a jPanel

Question

PBoucher 0 Newbie Poster

13 Years Ago

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>


    }
}

java

4 Contributors
6 Replies
327 Views
2 Days Discussion Span
Latest Post 13 Years Ago Latest Post by PBoucher

Ezzaral 2,714 Posting Sage

13 Years Ago

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

peter_budo 2,532 Code tags enforcer

13 Years Ago

@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());
    } 
}

Reply to this topic

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.

JamesCherrill 4,733 Most Valuable Poster Team Colleague Featured Poster · Answer 1 · 2011-06-11T16:05:31+00:00

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.

PBoucher 0 Newbie Poster · Answer 2 · 2011-06-13T19:14:14+00:00

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 0 Newbie Poster · Answer 3 · 2011-06-13T19:23:56+00:00

@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.

PBoucher 0 Newbie Poster · Answer 4 · 2011-06-13T19:38:08+00:00

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