Search code examples
javamultithreadingarraylistconcurrencypaintevent

Java paint 'stuttering' -list concurrency


One of my applications paints objects to a screen by reading an array List:

simple code summary:

@Override
public synchronized void paintComponent(Graphics g) {
  for(Object gO:paintList) {
    g.drawImage( gO.getObjImage(), gO.getXLoc(), gO.getYLoc(), outer.getTransparent(), null);
  }
}

The problem is I add more objects every time the user clicks the mouse, so if if the user clicks fast enough, I can cause the program painting to stutter since it cannot read while it is writing (the arrayList is synchronized). What is the common practice developers use to deal with this concurrency issue?

edit: here is the code that calls the repaint:

byte ticks = 0;
    while(true) {
        currentTime = System.nanoTime();
        if(ticks == 25) {
            drawPanel.repaint();
            ticks = 0;
        } else if (ticks%5 == 0) {//if ticks is a multiple of 5 (5,10,15,...)
            drawPanel.operations();
            ticks++;
        } else if(ticks < 25) {
            ticks++;
        }
        try {
            /*
            Refer to: 'http://stackoverflow.com/questions/1036754/difference-between-wait-and-sleep'
            on differences between Thread.sleep() and wait()
            */
            wait(1);//old timings: (long)(refreshRate*1000)
        } catch (InterruptedException ex) {
            Logger.getLogger(DeathWish.class.getName()).log(Level.SEVERE, null, ex);
        }
        //Debugging
        //System.out.println(ticks);
        currentTime = System.nanoTime();

*where operations() calculates changes in 'paintable' object's properties, removes objects that meet certain conditions and adds new objects to the paint list. Logically to me, the adding and writing should be separated? I can post the operations() method, if there isn't enough information, but I'm trying not to post huge sections of code so it is easier to interpret.


Solution

  • I guess you're getting things a bit wrong with synchronization :

    • ArrayList is a list implemented without synchronization
    • A synchronized method means that only 1 Thread at a time can access the method, but the variables inside your function are not synchronized at all

    What you want, is to have your list temporary synchronized.

    You could do something like that :

    @Override
    public void paintComponent(Graphics g) {
      synchronized(paintList) {
        for(Object gO:paintList) {
          g.drawImage( gO.getObjImage(), gO.getXLoc(), gO.getYLoc(), outer.getTransparent(), null);
        }
      }
    }
    

    and in the code adding objects yo your list do somewhat the same.

    EDIT from here :

    If you want to remove all the concurrency problems between the add thread and the paint Thread, here's how you could do :

    in the method to add images :

    public synchronized void addImage(...) {
      Something newImage = .....
      List<Something> newPaintList = new ArrayList<>(paintList.size() + 1);
      newPaintList.addAll(paintList);
      newPaintList.add(newImage);
      paintList = newPaintList;
    }
    

    And in the paint method, remove the synchronization part.

    @Override
    public void paintComponent(Graphics g) {
      for(Object gO:paintList) {
        g.drawImage( gO.getObjImage(), gO.getXLoc(), gO.getYLoc(), outer.getTransparent(), null);
      }
    }
    

    With this, you won't have any concurrency between the reads and the writes, since the only operation done on paintList is reads.

    The addImage should be synchronized to avoid two different Threads adding images at the same time, which could make one addImage ignored.