This week's book giveaways are in the Cloud and AI/ML forums.
We're giving away four copies each of Cloud Native Patterns and Natural Language Processing and have the authors on-line!
See this thread and this one for details.
Win a copy of Cloud Native PatternsE this week in the Cloud forum
or Natural Language Processing in the AI/ML forum!
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Devaka Cooray
  • Liutauras Vilda
  • Jeanne Boyarsky
  • Bear Bibeault
Sheriffs:
  • Paul Clapham
  • Knute Snortum
  • Rob Spoor
Saloon Keepers:
  • Tim Moores
  • Ron McLeod
  • Piet Souris
  • Stephan van Hulst
  • Carey Brown
Bartenders:
  • Tim Holloway
  • Frits Walraven
  • Ganesh Patekar

Animated Multi-Thread Graphical Object Instances Painting To The Same JPanel

 
Ranch Hand
Posts: 49
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Greetings all,

I'm new to Java Graphics, and am developing an app that displays ships on top of an aerial view of a harbour.  I followed some online tutorials and successfully used GeneralPath to draw ships within a class extended from JPanel.  As we live and program in an object oriented world, I then decided to create a ShipShape class and delegate drawing of each ship to that class by way of forwarding a Graphics2D reference from the JPanel class to the ShipShape class constructor.  This, too, was successful.

Finally I decided to take the next natural step: make each ShipShape instance run in its own thread, so each could exist independently of any another.  Each ship instance would also incorporate some simple animation, and this also influenced my decision to use threads, so any animation could occur within a loop inside a run() method.  Unfortunately, converting ShipShape to a Runnable class has not been successful - instances of ShipShape no longer draw to the JPanel screen.

I'm wondering if the Graphics2D reference passed to the ShipShape constructor gets broken now that ShipShape runs in a separate thread.  I've attached an abbreviated version of my three classes below.  Fully grasping multi-threading in Java graphics must be a vital skill.  I imagine that Java, animated, game characters would ideally exist within their own thread, right?

Possibly complicating matters is the intention that ship animations may differ slightly from instance to instance - some appear static, some contain flashing lights, some incorporate moving lights (like a battery charging indicator), etc.  Therefore, having the JPanel simply iterate through a ListArray<ShipShape>, sequentially repainting ShipShape instances one after the other, would be problematic - animations wouldn't occur simultaneously.

Your guidance would be greatly appreciated.

Thanks & regards,

Chris.





 
Rancher
Posts: 3409
33
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
The posted code won't compile with the ...
Could you post code that will compile so it can be executed for testing.

Is the GUI drawing being done on the EDT?  Use a SwingWorker to make sure GUI changes are on the EDT.
 
Marshal
Posts: 65021
246
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
As you doubtless know, Swing components are as a general rule not thread‑safe. So don't try animation with multiple threads. Use a Swing Timer instead. Any events from a Swing timer are injected into the EDT.
I do not know how long it would take to iterate a List and paint all the shapes therein, but try and see whether that can be done in 25ms or so. Remember we cannot see changes occurring faster than about 25−40ms.
 
Author
Posts: 974
1
  • Likes 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Christopher Dodunski wrote:
As we live and program in an object oriented world, I then decided to create a ShipShape class and delegate drawing of each ship to that class by way of forwarding a Graphics2D reference from the JPanel class to the ShipShape class constructor.  This, too, was successful.


I haven't had time to look closely at your code but, with that caveat, this approach is problematic, and I mean even before the multi-threading stuff you're asking about. When I first read this sentence my interpretation is that your implementation was definitely incorrect. On second reading I realize it's somewhat ambiguous and possibly what you were doing (before the multi-threading stuff) was ok. So it depends.

But in general, it's a mistake to forward a Graphics2D reference around. You should pretend that the Graphics2D object was created just before your JPanel's paint() method is called, exists for the duration of the call, and is destroyed afterward. That's not really what happens, but it would be a mistake to make any assumptions otherwise. So if your paint() method passes the Graphics2D object to a different method, or even a tree structure of methods, which is used briefly for painting, then that's absolutely fine. But if you're passing the Graphics2D to something that saves the reference in a field with plans to use it later, you really need to restructure your code not to do that. (This is what your ShipShape class is doing, which is bad, but I don't know if you were doing it that way in your pre-multi-threaded code.)

The correct way to do this, or at least a correct way, would be for your ShipShape objects to essentially have a paint() method of their own which takes a Graphics2D object as an argument and uses it only briefly. Then your HarbourPanel would keep a list of ShipShape objects (or whatever) and its paint() method would iterate through the ShipShape objects calling each paint method in a single thread. If you want to do multi-threaded stuff that's fine, but those other threads should manipulate the state of the ShipShape objects and/or the HarborPanel but not interfere with painting. The methods provided by the Graphics2D class should only be called by the single event dispatch thread. Does that make sense? (The other threads may, and probably should, call the HarbourPanel's repaint() method, which is documented to be thread-safe, when they realize they need to be redrawn. That wouldn't count as "interering with painting.")


While I'm here let me mention that you probably don't want to be instantiating new ImageIcons or new ShipShapes within paint() or paintComponent(). You want paint()/paintComponent() to be fast, and loading ImageIcons is slow. Instantiating ShipShapes is not especially slow so possibly that's ok, but typically you would set up your ImageIcons and ShipShapes in the initialization phase, so they're already there when paint() gets called.

BTW no big deal but HarbourPanel could probably extend JComponent instead of JPanel.
 
Christopher Dodunski
Ranch Hand
Posts: 49
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thank you all very much for your advice.

Summarising... Having each ShipShape instance run in its own thread is not in itself wrong.  However, having these instances retain a reference to a Graphics2D instance instantiated in HarbourPanel, and then proceeding to produce animation by repeatedly "pushing" graphical changes to this reference would be wrong.

You are suggesting two alternative approaches:

(1)  HarbourPanel stores instances of ShipShape instances in a ListArray<ShipShape>, and then iterates through it, one by one calling each ShipShape instance's paint(g2d) method to reflect on screen a graphical representation of each's current state.  I guess you could consider this a "pull" approach, as JPanel is systematically pulling state from each ShipShape instance.

(2)  Each ShipShape instance/thread calls JPanel whenever it changes state - for instance a new frame of an animation is generated - and JPanel (as above) invokes that ShipShape instance's paint(g2d) method to reflect that change.  I guess you could consider this a "push" approach, as unless a ShipShape instance pushes a change, JPanel sits idle.

My question is, given the variety of animations possible with each ShipShape instance - some flashing lights (2 frame animation), some chasing lights (up to an 11 frame animation), and some not animating at all (static) - is option (2) above the only viable solution?

Finally (re the first two replies), I assume SwingTimer could form part of option (1) for iterating through the ArrayList, whereas SwingWorker could be an alternative to multi-threading (animations would be generated in the "background" rather than within a loop of a thread's run() method).

Thanks & regards,

Chris Dodunski.
 
Brian Cole
Author
Posts: 974
1
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Christopher Dodunski wrote:You are suggesting two alternative approaches:


Not really. You need them both:
(1) HarbourPanel.paintComponent() iterates through the list of ShipShapes calling each ShipShape instance's paint(g2d) method.
(2) When a ShipShape object changes its state (or if the harbour itself does) then call HarbourPanel.repaint(). Not paint() or paintComponent(), but repaint(), which is different.

Note that calling repaint() doesn't do any immediate painting. But it does alert the system that it needs to call the paint() method before too long. To use your terminology (which I must admit I don't quite follow) it doesn't push but it requests a pull.
 
Saloon Keeper
Posts: 3407
149
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Another possibility that I sometimes use mimicks the way one programmed for the old Atari and Commodore 64 machines.

Say you have two BufferedImages, called 'display' and 'draw', each the size of your Harbour image.
Now, you could have a SwingWorker that lets each ShipShape update its state. It then draws the harbourimage into 'draw', and next let each ShipShape also draw itself to 'draw'. Once that is finished, it renames 'draw' to 'display' and vice versa. Then it repeats the cycle.

Meanwhile, whenever the SwingTimer fires, the Harbourpanel just draws 'display' into its panel. It couldn't care less what's in that image, or what made that image.

By the way: you have a very complicated way of executing your ShipShape runnables.
The normal way to start a Runnable rb is either via 'new Thread(rb).start' or letting an Executor execute r.
But why do you want your ShipShape to be a Runnable? If you have more than a handfull of ships, then you soon run out of available Threads (unless you have the very latest Intel or AMD processors). The idea that each ship can do its own animation if it is running in its own Thread is not plausible. You only see one moment of such an animation when the panel is repainting. Having each ship call redraw whenever it feels like it does not seem like a good idea to me either. Wouldn't you need some way of coordination then?
 
Christopher Dodunski
Ranch Hand
Posts: 49
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi all,

My reason for taking the (seemingly misguided) multi-thread route was largely to satisfy the requirement for multiple, simultaneous animation.  Complicating matters is the fact that animations may differ from ship to ship.  For instance, a flashing light animation would comprise 2 frames and repeat every 1 second.  Whereas a 'chasing light' animation may comprise up to 11 frames and repeat every 5 seconds.  With JPanel handling all such animation in a single thread, by iterating through a ListArray<ShipShape>, what would happen to remaining ships while JPanel was occupied generating a 5 second animation for just one?

I guess a solution is having JPanel paint just one frame of an animation before iterating to the next ship, but this didn't strike me as efficient coding.  Perhaps I am wrong and this is perfectly fine?

Thanks & regards,

Chris.
 
Brian Cole
Author
Posts: 974
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Maybe it's easiest to write some quick demo code.

The following code tries to take your approach but do it "right". If I were doing this on my own I don't think I'd give each drawn object its own thread. I'd probably use a timer (probably a java.util.Timer rather than a javax.swing.Timer in this particular case) or an event queue or something of that ilk.
 
Rancher
Posts: 828
19
Netbeans IDE Oracle MySQL Database Tomcat Server C++ Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
You have elements of animation from using the EDT and Technics commonly used in full screen manually controlled animations. The manual animations characteristically block the events scheduled on the EDT and everything just seemingly stops or become unpredictable at best.

Get rid of your infinite loop processing -- while(true) -- or get rid of your EDT reliance. Use manual or EDT, but rarely have I seen them merged successfully as an early learning exercise.
 
Brian Cole
Author
Posts: 974
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Les Morgan wrote:You have elements of animation from using the EDT and Technics commonly used in full screen manually controlled animations. The manual animations characteristically block the events scheduled on the EDT and everything just seemingly stops or become unpredictable at best.

Get rid of your infinite loop processing -- while(true) -- or get rid of your EDT reliance. Use manual or EDT, but rarely have I seen them merged successfully as an early learning exercise.



First of all, I'm not claiming to have any problem with my code. IMHO, it is correct. (Modulo each ball having its own thread, which I usually wouldn't do but was trying to help the original poster with his implementation using this approach.)

Second of all, I don't understand your complaints. I'm not dismissing them, but will you clarify why you object. I'll try to address what I can.

"The manual animations characteristically block the events scheduled on the EDT" • Nowhere in my code is the EDT blocked. If you think otherwise, please tell me what line number(s) you're worried about.

"Get rid of your infinite loop processing" • If you're talking about the sleep-within-loop idiom used here, then I agree. It would be better to take a different approach. But, given this approach, the infinite loops are appropriate.

"or get rid of your EDT reliance" • What are you talking about? The only thing that happens on the EDT in this is the painting. Everything else happens on other threads.
 
Piet Souris
Saloon Keeper
Posts: 3407
149
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Brian wrote: "The manual animations characteristically block the events scheduled on the EDT"
• Nowhere in my code is the EDT blocked. If you think otherwise, please tell me what line number(s) you're worried about.


I think Les could have a point here.
If you look at the paintComponent method in the Harbour class, it does

It could well be that one (or more) of the balls Threads is sleeping at that moment. What happens in that case? Does the EDT wait? I have no java at my disposal currently, so I can't check.
 
Bartender
Posts: 3323
86
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Piet Souris wrote:

Brian wrote: "The manual animations characteristically block the events scheduled on the EDT"
• Nowhere in my code is the EDT blocked. If you think otherwise, please tell me what line number(s) you're worried about.


I think Les could have a point here.
If you look at the paintComponent method in the Harbour class, it does

It could well be that one (or more) of the balls Threads is sleeping at that moment. What happens in that case? Does the EDT wait? I have no java at my disposal currently, so I can't check.


That shouldn't cause the EDT to block as the thread that may be sleeping is not the EDT.
What may be a problem though is there is no proper synchronization so the paint method may be painting the ball as the x and y values are being changed. Also shouldn't the 'visible' variable be declared as volatile as it is being used by two different threads.
 
Brian Cole
Author
Posts: 974
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Piet Souris wrote:It could well be that one (or more) of the balls Threads is sleeping at that moment. What happens in that case? Does the EDT wait? I have no java at my disposal currently, so I can't check.


That doesn't make sense. If the EDT wants to call a ball's paint method, it goes ahead and calls it. If that ball's private thread is sleeping (which it will be 99% of the time) that's completely irrelevant as far as the EDT is concerned. The threads don't share a lock or anything. (The balls' private threads don't do any locking at all.)

Tony Docherty wrote:
What may be a problem though is there is no proper synchronization so the paint method may be painting the ball as the x and y values are being changed. Also shouldn't the 'visible' variable be declared as volatile as it is being used by two different threads.


This is definitely as issue. If you want the balls' movement to happen atomically then you would have to write code to do that, taking care not to slow down the paint() method too much. In this program I don't care if I paint a ball in which, say, the x has changed and the y hasn't yet, because the animation will pretty much still be ok. It's easy to be cavalier in simple demos like this one. The original poster might not have that luxury.

visible is being used in two threads in the same way that x, y, and the others are. The private thread is read/writing it and the EDT is accessing it read-only. If the private thread is changing the visible value concurrently with an EDT paint, I don't care if paint() draws it visible or invisible. Either is fine to achieve the blinking animation effect. This is a luxury the original poster probably does have, unless he has stringent requirements about how the blinking needs to look.


I still maintain that my code is sound. If the original poster wants each ShipShape object to have its own thread to change its position and heading, then this is the kind of approach he should take. But of course I'm willing to consider further objections.
 
Brian Cole
Author
Posts: 974
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Tony Docherty wrote:Also shouldn't the 'visible' variable be declared as volatile as it is being used by two different threads.


Sorry, I talked about the two threads but I didn't talk about volatile. Yes, I think it might be smart to declare x, y, and visible to be volatile. It didn't occur to me but it should have. Without volatile I don't think there is any guarantee that the EDT thread will ever see changes to those variables, though of course in practice it does eventually.

 
Piet Souris
Saloon Keeper
Posts: 3407
149
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Brian Cole wrote:

Piet Souris wrote:It could well be that one (or more) of the balls Threads is sleeping at that moment. What happens in that case? Does the EDT wait? I have no java at my disposal currently, so I can't check.


That doesn't make sense. (...)


I was only asking, but you are right: there are no stupid answers, only stupid questions.

Just tested this at home. The code works without any problem, no matter how hard I tried by resizing the panel as fast as I could. The 'paint' method is not affected in any way by the Thread executing the 'run' method. This is new to me, and if I had a chance to test this earlier, I wouldn't have asked.
 
Brian Cole
Author
Posts: 974
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Piet Souris wrote:Just tested this at home. The code works without any problem, no matter how hard I tried by resizing the panel as fast as I could. The 'paint' method is not affected in any way by the Thread executing the 'run' method. This is new to me, and if I had a chance to test this earlier, I wouldn't have asked.


Thanks for the retraction.

It's funny that you mention resizing the panel, because I think the bounce() method does have a bug related to that. If you size the panel big and wait for a ball to get at the very bottom (or very right) then quickly resize to very small, the code in bounce() doesn't handle this correctly and the ball can get 'lost' with coordinates outside the visible area. I realized this when I wrote it, but I decided to keep the code simple and not address it. (Again, it's easy to be cavalier in simple demos.) I did try to "lose" a ball this way several times but failed each time, but I think it's possible if things go just right.
 
Tony Docherty
Bartender
Posts: 3323
86
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Brian Cole wrote:
I still maintain that my code is sound. If the original poster wants each ShipShape object to have its own thread to change its position and heading, then this is the kind of approach he should take. But of course I'm willing to consider further objections.


I agree and sorry if my post came across as criticism. I was originally posting to support your code with regard to the thread locking suggestion and then noticed a couple of issues which as you say probably aren't critical given the actual application. I tacked those comments onto the end of the post just before having to rush out and probably should have worded it better.

I did try to "lose" a ball this way several times but failed each time, but I think it's possible if things go just right.

Or just wrong  
 
Norm Radder
Rancher
Posts: 3409
33
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I'm taken Brian's nice code and messed it up a bit to try another solution:  Use one thread with a ScheduledThreadPoolExecutor object to schedule ball movements.  I've found that ScheduledThreadPoolExecutor loses tasks and leaves more and more balls unmoved over time.  I've run out of ideas on how to fix this.

Here is Brian's code with my mess:
 
Norm Radder
Rancher
Posts: 3409
33
 
Campbell Ritchie
Marshal
Posts: 65021
246
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Norm Radder wrote:Also posted at: . . .

People on both websites need to know about multiple postings, so they don't simply repeat what has already been said elsewhere.
 
Norm Radder
Rancher
Posts: 3409
33
 
Christopher Dodunski
Ranch Hand
Posts: 49
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thank you, Brian, for making the effort to produce some demo code.  I've studied and scrutinised it very closely, and am hugely grateful for what I've learned from that.

I've pushed on with my project, and it is now at the stage where I can present it to my employer as a working proof-of-concept.

In the next day or two I will place a link to an MP4 demo of the code in action - it works beautifully!

Again, thank you all for the sharing of ideas.  What a great environment for intermediate programmers like myself to grow their skills.
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!