1

I have an ImageViewComponent that extends JComponent. This component is added to a JPanel that is added to a JFrame. From another class I regularly update the int[] image field of ImageViewComponent from another class. The problem is that this process eats up a lot of memory. It even consumes so much memory (+/- 130 MB after a few seconds according to JProfiler, and eventually it surpasses 1GB) that the entire program undergoes a 'lag spike' during garbage collection (the lag in the program happens at the same time that the memory is cleared).

This is the code of ImageViewComponent:

public class ImageViewComponent extends JComponent {

    private int image_width, image_height, updateInterval, updateCounter;
    private int[] imageArray;
    private BufferedImage currentDisplayedImage;
    private Image scaledDisplayedImage;

    /**
     * @param width          The width of this component
     * @param height         The height of this component
     * @param ui             The higher, the less frequent the image will be updated
     */
    public ImageViewComponent(int width, int height, int ui) {
        setPreferredSize(new Dimension(width, height));
        this.updateInterval = ui;
        this.updateCounter = 0;
        this.currentDisplayedImage = null;
        this.scaledDisplayedImage = null;
    }

    public void setImage(int[] image, int width, int height) {
        this.imageArray = image;
        this.image_width = width;
        this.image_height = height;
    }

    @Override
    public void paint(Graphics g) {
        super.paint(g);

        if (image_width == 0 || image_height == 0)
            return;
        else if (updateCounter != updateInterval && currentDisplayedImage != null) {
            g.drawImage(scaledDisplayedImage, 0, 0, this);
            updateCounter++;
            return;
        }

        this.currentDisplayedImage = new BufferedImage(image_width, image_height, BufferedImage.TYPE_INT_RGB);
        this.currentDisplayedImage.setRGB(0, 0, image_width, image_height, this.imageArray, 0, image_width);

        this.scaledDisplayedImage = this.currentDisplayedImage.getScaledInstance(this.getPreferredSize().width,
                this.getPreferredSize().height, BufferedImage.SCALE_DEFAULT);

        g.drawImage(scaledDisplayedImage, 0, 0, this);

        // reset update counter
        updateCounter = 0;
    }

}

JProfiler states that 70% of the program its active memory is allocated in this class, 50% is in Graphics.drawImage while 20% is in BufferedImage initialization.

I have tried fixing it by putting the line this.currentDisplayedImage = new BufferedImage(image_width, image_height, BufferedImage.TYPE_INT_RGB) in `setImage' and have it only set it once with a boolean flag but this makes the drawn image turn completely black for short amounts of time once in a while, nor does it fix the memory problem. I also tried this suggestion, which didn't work either.

How can I fix this memory issue?

Community
  • 1
  • 1
mathivh
  • 13
  • 7
  • Avoid doing all the work in the `paintComponent` method. Use a `ComponentListener` to detect changes to the component size and update the scaled instance then. I'd also create the new `BufferedImage` only in the `setImage` method, calling `repaint` to trigger a paint pass – MadProgrammer May 21 '17 at 21:12
  • @MadProgrammer The component itself doesn't change in size, nor does the image size. I rescale it because the `int[] image` has a bigger size than the component. The `setImage` method is called very often with each time a completely different image (it gets its images from a simulator that runs at approx. 60fps). I will create the `BufferedImage` in the `setImage` with a boolean flag to run in only once as I tried earlier. Anything else I should consider? – mathivh May 21 '17 at 21:24
  • Provide a runnable example – MadProgrammer May 21 '17 at 22:08
  • I'd also verify that it's the component which is causing the memory leak and not what ever generates the `int[]` – MadProgrammer May 21 '17 at 22:21

1 Answers1

3

There are several issues with the code. Some refer to performance, others to style or best practices, and others (at least potentially) refer to memory consumption.

  • Performance: The getScaledInstance method is distressingly slow. See https://stackoverflow.com/a/32278737/3182664 and others for better alternatives
  • Style: It's imageWidth, not image_width
  • Best practices: For a JComponent, you usually, you override paintComponent and not paint
  • Memory consumption: That's the main point...:

As MadProgrammer already pointed out: Do things as rarely as possible. The role and purpose of this updateCounter is not entirely clear. I think that the responsibility for updating the image less frequently should be in the class that uses your component - particularly, in the class that calls updateImage (which should simply be done less often). Maintaining this in the paint method is not very reliable.

In your current code, it seems like the currentDisplayedImage is (despite its name) neither displayed nor used in any other way. It may, however, be a good idea to keep it: It will be needed to be filled with the int[] data, and as a source for the scaled image that might have to be created.

One possible implementation of your class might look as follows:

import java.awt.Dimension;
import java.awt.Graphics;
import java.awt.geom.AffineTransform;
import java.awt.image.AffineTransformOp;
import java.awt.image.BufferedImage;

import javax.swing.JComponent;

public class ImageViewComponent extends JComponent {

    private int updateInterval, updateCounter;
    private BufferedImage fullImage;
    private BufferedImage displayedImage;

    /**
     * @param width          The width of this component
     * @param height         The height of this component
     * @param ui             The higher, the less frequent the image will be updated
     */
    public ImageViewComponent(int width, int height, int ui) {
        setPreferredSize(new Dimension(width, height));
        this.updateInterval = ui;
        this.updateCounter = 0;
        this.fullImage = null;
        this.displayedImage = 
            new BufferedImage(width, height, BufferedImage.TYPE_INT_ARGB);
    }

    public void setImage(int[] image, int width, int height) {

        // Note: The updateInvervall/updateCounter stuff COULD
        // probably also be done here...
        if (fullImage == null ||
            fullImage.getWidth() != width ||
            fullImage.getHeight() != height)
        {
            fullImage = new BufferedImage(
                width, height, BufferedImage.TYPE_INT_RGB);
        }
        fullImage.setRGB(0, 0, width, height, image, 0, width);
        scaleImage(fullImage, displayedImage);
        repaint();
    }

    @Override
    public void paintComponent(Graphics g) {
        super.paintComponent(g);
        g.drawImage(displayedImage, 0, 0, this);
    }


    private static BufferedImage scaleImage(
        BufferedImage input, BufferedImage output)
    {
        double scaleX = (double) output.getWidth() / input.getWidth();
        double scaleY = (double) output.getHeight() / input.getHeight();
        AffineTransform affineTransform = 
            AffineTransform.getScaleInstance(scaleX, scaleY);
        AffineTransformOp affineTransformOp = 
            new AffineTransformOp(affineTransform, null);
        return affineTransformOp.filter(input, output);
    }    

}

but note that this does not do this "updateInterval" handling, for the reason mentioned above.

And a side note: Maybe you don't even have to scale the image. If your intention is to have the image always being displayed at the size of the component, then you can simply do

@Override
public void paintComponent(Graphics g) {
    super.paintComponent(g);

    // Draw the FULL image, which, regardless of its size (!) 
    // is here painted to just fill this component:
    g.drawImage(fullImage, 0, 0, getWidth(), getHeight(), null);
}

Usually, drawing a scaled image like this is pretty fast. But depending on many factors, separating the step of scaling and painting the image, like you did, may also be a reasonable option.

Community
  • 1
  • 1
Marco13
  • 53,703
  • 9
  • 80
  • 159
  • Using your suggestion in combination with calling `setImage` in an `invokeLater` block in the calling method fixed the memory issue. FYI the last part of your post (separating scaling and painting) didn't seem to make much of a difference, if any. Thanks! – mathivh May 22 '17 at 17:34