4

I am currently writing a Fractal Explorer program, and I am encountering a really weird issue with it: I am drawing the fractal on a BufferedImage, and I get random black areas in that image. Screenshots: https://i.stack.imgur.com/VOvz3.jpg

The image is calculated multi-threaded: The big image is split into four (because I have a four-core processor) sub-images that are calculated individually. The black areas appear at the beginning of each of the sub-images. They are always rectangular, not necessarily following the order in which the pixels are calculated (left-to-right, but the area does not always stretch to the far side of the sub-image).

I have verified that immediately after the pixel is drawn (with Graphics.drawLine), BufferedImage.getRGB returns the right color for the pixel, but after the calculation is finished, it can return black instead, as the pixel is drawn on the screen.

The problem seems to vanish if I disable multi-threaded calculating (by assigning only one core to javaw.exe via the task manager) but I really don't want to have to abandon multi-core calculation. Has anyone else encountered this problem (I have not found anything via Google and stackoverflow), and do you know how to fix it?

The Graphics.drawLine call is synchronized on the Graphics object; if I additionally synchronized it on the BufferedImage, nothing changes.

If you want to see the bug for yourself, you can download the program at http://code.lucaswerkmeister.de/jfractalizer/. It is also available on GitHub (https://github.com/lucaswerkmeister/JFractalizer), but I only switched to GitHub recently, and in the first GitHub commit the problem is already apparent.

rolve
  • 10,083
  • 4
  • 55
  • 75
Lucas Werkmeister
  • 2,584
  • 1
  • 17
  • 31

2 Answers2

3

I think the problem is that neither BufferedImage nor Graphics is thread safe and that you see stale values in the thread that reads the BufferedImage after the computation.

Synchronizing on the BufferedImage like you said should actually help. But note that you must synchronize all accesses from all threads, including the read-only accesses. So my guess is that the thread that draws the BufferedImage on some component (which should be the AWT thread) does so without synchronization and therefore sees stale values.

However, I would suggest that instead of sharing a BufferedImage among multiple threads, you give each thread a separate image on which it can draw. Then, after all threads are finished, combine their work on a new image in the AWT thread.

Also, I suggest you use an ExecutorService for that if you don't do so already. It has the advantage that the visibility issues of the return values of the Callable tasks (in your case the image parts of the worker threads) are handled by the library classes.

If you combine these two approaches, you will not need to do any manual synchronization, which is always a good thing (as it's easy to get wrong).

rolve
  • 10,083
  • 4
  • 55
  • 75
  • Synchronizing the access to the BufferedImage in the paint method did not help. Also, the black areas aren't temporary - they are there even after all potentially concurrent accesses to the BufferedImage have long ceased (for example, when I save the image with ImageIO). I suppose I could give each thread its own image, but I want to see the progress of the calculation while it's not completed yet. I will check in a minute whether that works. – Lucas Werkmeister Oct 03 '12 at 14:08
  • (Continuing the previous comment) And no, I'm not using an ExecutorService, I didn't even know about it. (I suspected that there are things to ease multi-threaded programming, but I've never used Java in larger programs - heck, I'm still a student - and I wanted to go with simple threading as long as that worked. I suppose I will check these things out then.) – Lucas Werkmeister Oct 03 '12 at 14:12
  • First report: I changed it so that every thread now gets a direct reference to the general image (not yet its own image) and draws on its own createGraphics. It's so much faster, but the black areas increased in the same amount. I know it's faster because now the synchronization is useless, but I think that the missing synchronization should not be a problem once I get rid of the black areas: The black areas appear only at the beginning, and later in the image the concurrent paint calls seem to not disturb the BufferedImage at all. (I uploaded a third screenshot.) – Lucas Werkmeister Oct 03 '12 at 14:33
  • Okay, I changed it so that now every thread actually draws on its own image, and performance improved drastically again - thank you for that alone already! It also got rid of the black areas. There is some flickering while the image is still calculated and drawn at the same time, but that's temporary. I will next look into this ExecutorService, let's see what that does. Anyway, thank you! – Lucas Werkmeister Oct 04 '12 at 08:16
0

Buffered images may not be thread safe because their data may live on the graphics card. However, this can be overridden. By using the ((DataBufferInt) image.getRaster().getDataBuffer()).getData() secret technique for high speed full image drawing (data buffer type depends on the image type you chose), you will get an unaccelerated image. As long as you never write to the same pixel twice, this should be theoretically completely safe. But remember to join() your threads somehow before reading pixels out of the image. join() method from thread not actually recommended as this requires thread death.

Related note: The flicker you are witnessing is probably an artifact of the way awt renders to the screen. It runs in immediate mode, meaning every draw action you take immediately updates the screen. This slows down rendering of multiple objects directly to the window. You can get around the flicker by implementing a double buffering strategy. I like to draw to an intermediate image and then draw only that image to the screen.

warren
  • 563
  • 2
  • 13