0

I have a problem. I have to make an app, which would contain n number of Threads which would move the balls around the circle (with different velocities). Now I've got my Thread extending JComponent and implementing Runnable. Everything works just fine, except the run() method doesn't seem to call paintComponent. How can I fix that? The code:

Thread (Kulka.java):

package internet;

import internet.RotateWheel.TestPane;
import java.awt.Color;
import java.awt.Graphics;
import java.awt.Graphics2D;
import java.awt.Image;
import java.awt.Point;
import static java.lang.Thread.sleep;
import javax.swing.JComponent;
import javax.swing.JPanel;

public class Kulka extends JComponent implements Runnable{
    int x, y;
    float predkosc;
    float obecna_predkosc = 0f;
    TestPane parent;

    public Kulka(TestPane parent, int x, int y, float v){
        this.predkosc = v;
        this.x = x;
        this.y = y;
        this.parent = parent;
    }

    public Point getPointOnCircle(float degress, float radius) {

        int x = Math.round(getWidth() / 2);
        int y = Math.round(getHeight() / 2);

        double rads = Math.toRadians(degress - 90);

        int xPosy = Math.round((float) (x + Math.cos(rads) * radius));
        int yPosy = Math.round((float) (y + Math.sin(rads) * radius));

        return new Point(xPosy, yPosy);

    }

    @Override
    public void paintComponent(Graphics g){
        super.paintComponent(g);
        Graphics2D g2 = (Graphics2D) g;
        g2.setColor(Color.RED);
        int diameter = Math.min(getWidth(), getHeight());
        int x = (getWidth() - diameter) / 2;
        int y = (getHeight() - diameter) / 2;

        g2.setColor(Color.RED);
        float innerDiameter = 20;
        System.out.println("Diameter: " + diameter);
        Point p = getPointOnCircle(obecna_predkosc, (diameter / 2f) - (innerDiameter / 2));
        g2.fillOval(x + p.x - (int) (innerDiameter / 2), y + p.y - (int) (innerDiameter / 2), (int) innerDiameter, (int) innerDiameter);        
    }

    public synchronized void run(){
        while (true){
            //System.out.println(obecna_predkosc);
            try {
                System.out.println(predkosc);
                this.repaint();
                sleep(1000);
            } catch (InterruptedException e){
                e.printStackTrace();
            }
        }
    }

}

This is what happens:

enter image description here

Class with Frame and Panel:

package internet;
import java.awt.Color;
import java.awt.Dimension;
import java.awt.EventQueue;
import java.awt.Graphics;
import java.awt.Graphics2D;
import java.awt.Point;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.awt.event.MouseAdapter;
import java.awt.event.MouseEvent;
import java.util.LinkedList;
import java.util.Random;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.Timer;
import javax.swing.UIManager;
import javax.swing.UnsupportedLookAndFeelException;

public class RotateWheel {

    public static void main(String[] args) {
        new RotateWheel();
    }

    public RotateWheel() {
        EventQueue.invokeLater(new Runnable() {
            @Override
            public void run() {
                try {
                    UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName());
                } catch (ClassNotFoundException | InstantiationException | IllegalAccessException | UnsupportedLookAndFeelException ex) {
                    ex.printStackTrace();
                }

                JFrame frame = new JFrame("Testing");
                frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
                frame.add(new TestPane());
                frame.pack();
                frame.setLocationRelativeTo(null);
                frame.setVisible(true);
            }
        });
    }



    public class TestPane extends JPanel {

        LinkedList<Thread> kulki =  new LinkedList<Thread>();        
        int n = 5;
        private float degrees = 0;
        Random random = new Random();

        public TestPane() {            
            /*Timer timer = new Timer(60, new ActionListener() {

                @Override
                public void actionPerformed(ActionEvent e) {
                    degrees += 1f;

                    repaint();
                }
            });
            timer.start();*/
        }


        public Point getPointOnCircle(float degress, float radius) {

            int x = Math.round(getWidth() / 2);
            int y = Math.round(getHeight() / 2);

            double rads = Math.toRadians(degress - 90);

            int xPosy = Math.round((float) (x + Math.cos(rads) * radius));
            int yPosy = Math.round((float) (y + Math.sin(rads) * radius));

            return new Point(xPosy, yPosy);

        }

        @Override
        public Dimension getPreferredSize() {
            return new Dimension(200, 200);
        }

        @Override
        protected void paintComponent(Graphics g) {
            super.paintComponent(g);
            Graphics2D g2d = (Graphics2D) g.create();

            int diameter = Math.min(getWidth(), getHeight());
            int x = (getWidth() - diameter) / 2;
            int y = (getHeight() - diameter) / 2;

            g2d.setColor(Color.GREEN);
            g2d.drawOval(x, y, diameter, diameter);

            g2d.setColor(Color.RED);
            float innerDiameter = 20;

            if (kulki.isEmpty()){
                for (int i=0; i<n; i++){
                    Point p = getPointOnCircle(degrees, (diameter / 2f) - (innerDiameter / 2));
                    Kulka kulka = new Kulka(this, p.x, p.y, random.nextFloat() * (5f - 2f) + 1f);
                    Thread watek = new Thread(kulka);
                    kulki.add(watek);                                       
                }
                for (Thread t: kulki){
                    t.start();
                }
            }

            System.out.println("No elo XD");


            g2d.dispose();
        }


    }

}
  • 2
    Based on my testing, it is (`System.out.println("Diameter: " + diameter);` is been called regularly). Consider providing a runnable which demonstrates your issues – MadProgrammer Jan 16 '17 at 21:28
  • 2
    Why are you creating a thread within the `paintComponent` method? When do you update the state of the `kulki` ... in fact, when do you paint them? – MadProgrammer Jan 16 '17 at 21:37
  • Dunno, where should I create it then? I paint them after running `run()` method in `Kulka` –  Jan 16 '17 at 21:39
  • `Kulka` is a `JPanel`, but it's never added to anything which could possibly paint it. I think that's the first key issue. `paintComponent` can be called many times over the life span of your program, in fact, you're expecting the `Kulka` component's `paintComponent` method to be called whenever you call `repaint`, so you should never do anything which could change the state of any of the components, painting should paint the current state only. – MadProgrammer Jan 16 '17 at 21:44
  • Kulka is a JComponent. So should I empty the whole `paintComponent`? –  Jan 16 '17 at 21:45
  • 1
    More threads != more efficient. I'd change your design to have a single thread (although I'd personally use a Swing `Timer`) that manages all the elements which need to be updated on each iteration of the loop – MadProgrammer Jan 16 '17 at 21:46
  • *"Kulka is a JComponent. So should I empty the whole paintComponent"* - No, my gut feeling is, `Kulka` shouldn't be a component at all, but an entity which can be updated and painted, probably through the `TestPane` – MadProgrammer Jan 16 '17 at 21:46
  • Something more [like this](http://stackoverflow.com/questions/13022754/java-bouncing-ball/13022788#13022788) for example – MadProgrammer Jan 16 '17 at 21:47
  • Okay, its not my idea. Its an excercise for programming class, when I'm obligated to use `n` number of threads. –  Jan 16 '17 at 21:52
  • Okay, but's a bad idea ;). I'd still not be making `Kulka` component – MadProgrammer Jan 16 '17 at 21:54
  • Not enough rep to chat :( I can make Kulka JPanel, if that makes a difference. Or I can even don't make it anything of JAnything. But then, how would my Threads paint anything? –  Jan 16 '17 at 22:00
  • The threads would ask the parent panel (`TestPane` in this case) to `repaint`, which would cause it to iterate over the list of entities and call their "paint" methods. Another way would be to have each entity ask the "parent" to paint them, allowing it to generate a list of entities that need to be repainted. In the end, it will all be the same, as Swing's painting process consolidates multiple requests into as few physical paint updates – MadProgrammer Jan 16 '17 at 22:02
  • But how to ask TestPane to repaint with different parameters (like smaller circles position)? –  Jan 16 '17 at 22:11
  • The entity should be self contained, it should update it's internal properties and then request an update, this can be done by passing a reference of the "container" class to the entity, which allows the entity to inform the "container" that it's been updated and the entity can then decide how it should react to it. This is a little backwards to how it's normally done, but that's the constraints of your requirements – MadProgrammer Jan 16 '17 at 22:20
  • So Panel+Frame should be one class, and then I should pass the object of it to the Thread class, or the other way around? –  Jan 16 '17 at 22:24
  • The "container" class (who ever is actually going to paint the entities) should be passed to the entities. Each entity should then be self contained – MadProgrammer Jan 16 '17 at 22:29
  • But I only got a JFrame, JPanel, and a Thread extending JComponent. So which one is my container then –  Jan 16 '17 at 22:33
  • Personally, I'd say the `JPanel` – MadProgrammer Jan 16 '17 at 22:34
  • But I actually pass the `container` as a parameter in `Kulka` constructor –  Jan 16 '17 at 22:38
  • But you do nothing with it – MadProgrammer Jan 16 '17 at 22:38
  • Right! So how can I do something with it? –  Jan 16 '17 at 22:47

1 Answers1

0

Let's look at the obvious issues

  • In TestPane, you create a series of Kulka components, but never add them to anything
  • You can the state of the UI from within the paintComponent method
  • There's no (obvious) reason for Kulka to be any type of component

What are the possible solutions?

  • Create your Kulka elements within the constructor of the class or some other method unrelated to the UI
  • Painting should paint the current state, nothing else
  • Kulka could be just a POJO, which has some means to paint to a Graphics context and update itself as required. Technically, this could be self contained, but there's nothing wrong with having it rely on an external update source

Kulka...

This is a pretty basic class, it's reliant on an external updater and painter, which makes it flexible. This means you could use multiple threads, a single thread or a Timer to act as the updater and it won't care. It also doesn't care about how it's painted, only that it requires a Graphics context to do it.

public class Kulka {

    private PaintContainer container;
    private float angle;
    private float delta;

    public Kulka(PaintContainer container, float angle, float delta) {
        this.container = container;
        this.angle = angle;
        this.delta = delta;
    }

    public void update() {
        angle += delta;
        container.wasUpdated(this);
    }

    public void paint(Graphics2D g2d) {
        int diameter = Math.min(container.getWidth(), container.getHeight());
        float innerDiameter = 20;

        Point point = getPointOnCircle(angle, (diameter / 2f) - (innerDiameter / 2));
        g2d.setColor(Color.RED);
        g2d.fill(new Ellipse2D.Double(point.x, point.y, innerDiameter, innerDiameter));
    }

    public Point getPointOnCircle(float degress, float radius) {

        int x = Math.round(container.getWidth() / 2);
        int y = Math.round(container.getHeight() / 2);

        double rads = Math.toRadians(degress - 90);

        int xPosy = Math.round((float) (x + Math.cos(rads) * radius));
        int yPosy = Math.round((float) (y + Math.sin(rads) * radius));

        return new Point(xPosy, yPosy);

    }

}

PaintContainer...

Because I'm a crusty old developer, I prefer to use interfaces where I can, but strictly speaking, it's not required. I've used it here to force the separation and prevent the Kulka class from interacting with the painting container in ways I don't want it to.

public interface PaintContainer {
    public int getWidth();
    public int getHeight();

    public void wasUpdated(Kulka kulka);
}

Updater...

Okay, so based on your requirements, you need a single thread for each Kulka, as I've noted, this is a bad idea and won't scale well, but this is what you require.

This simply acts as a wrapper around the Kulka and the Thread. Now, it would be possible for Kulka to be self contained and update itself, but I thought it'd be better if Kulka had no concept of how it was updated, only that it can.

public class Updater implements Runnable {
    private Kulka kulka;
    private Thread t;

    public Updater(Kulka kulka) {
        this.kulka = kulka;
    }

    public void start() {
        if (t == null) {
            t = new Thread(this);
            t.start();
        }
    }

    @Override
    public void run() {
        while (true) {
            kulka.update();
            try {
                Thread.sleep(16);
            } catch (InterruptedException ex) {
            }
        }
    }
}

The painter...

I did think about trying to use a "smart" painting process, only updating those elements which have changed, but the fact is, on each paint cycle, we need to repaint everything any way :P

public class TestPane extends JPanel implements PaintContainer {

    private Set<Kulka> entities;

    public TestPane() {
        entities = new HashSet<>(10);
        for (int index = 0; index < 10; index++) {
            float degrees = (float)(Math.random() * 359d);
            float delta = (float)((Math.random() * 10d) - 5d);
            Kulka kulka = new Kulka(this, degrees, delta);
            entities.add(kulka);
            Updater updater = new Updater(kulka);
            updater.start();
        }
    }

    @Override
    public Dimension getPreferredSize() {
        return new Dimension(200, 200);
    }

    @Override
    synchronized protected void paintComponent(Graphics g) {
        super.paintComponent(g);
        Graphics2D g2d = (Graphics2D) g.create();

        int diameter = Math.min(getWidth(), getHeight());
        int x = (getWidth() - diameter) / 2;
        int y = (getHeight() - diameter) / 2;

        g2d.setColor(Color.GREEN);
        g2d.drawOval(x, y, diameter, diameter);

        g2d.setColor(Color.RED);
        g2d.dispose();

        for (Kulka kulka : entities) {
            g2d = (Graphics2D) g.create();
            kulka.paint(g2d);
            g2d.dispose();
        }
    }

    @Override
    synchronized public void wasUpdated(Kulka kulka) {
        repaint();
    }

}
MadProgrammer
  • 343,457
  • 22
  • 230
  • 366
  • Jesus, bro, thanks so much. Then I just create main class, create frame, add TestPane and thats it? –  Jan 16 '17 at 23:37
  • 1
    Pretty much. There is a misculation in the `getPointOnCircle` method which is causing the `Kulka` to be offset, but I'll leave that for you to figure out ;) – MadProgrammer Jan 16 '17 at 23:41