2

I'm making a picture viewer application with JavaFX 17. To summarize, the application is like Windows Photo / Windows Picture Viewer. The user could open a picture or a folder. The application will display the given picture or the first picture from the given folder. My application will display one picture at a time, User could navigate the picture using available control (next, prev, last, & beginning).

I've checked the below threads to make sure it's optimised enough:

But, I found that my code has an issue with processing 200 pictures with each size around 1~2 MB.

Without background loading, the application does not display anything. Even though the navigation control state is changed because of knowing that there are available pictures. So, click next & prev only shows a blank screen. When using the background loading, only a few of the very first image is loaded. After several next control, suddenly it's become blank again.

Here is my minimal, reproducible example:

package com.swardana.mcve.image;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.Executors;
import javafx.application.Application;
import javafx.concurrent.Task;
import javafx.scene.Scene;
import javafx.scene.image.Image;
import javafx.scene.image.ImageView;
import javafx.scene.input.KeyEvent;
import javafx.scene.layout.VBox;
import javafx.stage.Stage;

/**
 * JavaFX App
 */
public class App extends Application {

    @Override
    public void start(Stage stage) {
        var view = new View();
        var path = Paths.get("Path/to/many/images");
        var storage = new Storage(new PictureSource(path));
        storage.setOnSucceeded(eh -> view.exhibit(storage.getValue()));
        Executors.newSingleThreadExecutor().submit(storage);
        var scene = new Scene(view, 640, 480);
        scene.addEventFilter(KeyEvent.KEY_PRESSED, eh -> {
            switch (eh.getCode()) {
                case RIGHT:
                    view.next();
                    break;
                case DOWN:
                    view.last();
                    break;
                case LEFT:
                    view.prev();
                    break;
                case UP:
                    view.beginning();
                    break;    
                default:
                    throw new AssertionError();
            }
        });
        stage.setScene(scene);
        stage.show();
    }

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

    public class Picture {

        private final String name;
        private final Image image;

        public Picture(final String name, final Path src) throws IOException {
            this(name, new Image(src.toUri().toURL().toExternalForm(), true));
        }

        public Picture(final String name, final Image img) {
            this.name = name;
            this.image = img;
        }

        public final String name() {
            return this.name;
        }

        public final Image image() {
            return this.image;
        }

    }

    public class PictureSource {

        private final Path source;

        public PictureSource(final Path src) {
            this.source = src;
        }

        public final List<Picture> pictures() {
            var dir = this.source.toString();
            final List<Picture> pictures = new ArrayList<>();
            try (var stream = Files.newDirectoryStream(this.source, "*.{png,PNG,JPG,jpg,JPEG,jpeg,GIF,gif,BMP,bmp}")) {
                for (final var path : stream) {
                    var picName = path.getFileName().toString();
                    pictures.add(
                        new Picture(picName, path)
                    );
                }
                return pictures;
            } catch (final IOException ex) {
                throw new RuntimeException(ex);
            }
        }
    }
    
    public class Storage extends Task<List<Picture>> {
        private final PictureSource source;

        public Storage(final PictureSource src) {
            this.source = src;
        }

        @Override
        protected final List<Picture> call() throws Exception {
            return this.source.pictures();
        }
    }
    
    public class View extends VBox {
        private final ImageView image;
        private List<Picture> pictures;
        private int lastIdx;
        private int index;
        
        public View() {
            this.image = new ImageView();
            this.initGraphics();
        }
        
        // This method to accept value from the `Storage`.
        public void exhibit(final List<Picture> pics) {
           this.pictures = pics;
           this.index = 0;
           this.lastIdx = pics.size();
           this.onChange();
        }
        
        public void next() {
            if (this.index != this.lastIdx - 1) {
                this.index++;
                this.onChange();
            }
        }
        
        public void prev() {
            if (this.index != 0) {
                this.index--;
                this.onChange();
            }
        }
        
        public void last() {
            this.index = this.lastIdx - 1;
            this.onChange();
        }
        
        public void beginning() {
            this.index = 0;
            this.onChange();
        }

        // Whenever the action change, update the image from pictures.
        public void onChange() {
            this.image.setImage(this.pictures.get(this.index).image());
        }
        
        private void initGraphics() {
            this.getChildren().add(this.image);
        }
        
    }

}

Really appreciate any help and advise.

Sukma Wardana
  • 540
  • 8
  • 21
  • 2
    [mcve] please.. mind the __M__ (leave out all the fluff - just a simple class to load the images and how you use it on a background thread) – kleopatra Apr 12 '22 at 08:46
  • @kleopatra I've updated my answer to remove all the fluff and provide minimal reproducible example. – Sukma Wardana Apr 13 '22 at 03:31
  • hmm .. cannot reproduce the problem (could be the images, though). A note: I think, your storage is not needed, it will return immediately anyway - the loading is done by the image itself, that is you have many background threads that you don't (want to) control. So it might be that you are trying to navigate to an image that's not yet fully loaded - not sure what the system is supposed to do in such a case (doc states "showing a placeholder" while I'm seeing a blank view until the image is fully loaded - checked its progress property) – kleopatra Apr 13 '22 at 10:25
  • Yes, my code doesn't have any issue processing images under 200 items. But, when reaching 200 images it shows this behaviour. Might need to see another options on how to solve this issue – Sukma Wardana Apr 14 '22 at 02:37
  • Try to use [`GridView`](https://controlsfx.github.io/javadoc/11.0.3/org.controlsfx.controls/org/controlsfx/control/GridView.html). – SedJ601 Apr 15 '22 at 05:10

1 Answers1

2

The problem is that you load all images in full size (it requires a lot of memory) at one time and they saved in List<Pictures> so they stay in memory. I tried to load 100 big pictures with your code and to 10th picture i got OutOfMemoryError: Java heap space (used heap was about 2.5GB in size).

I found two possible solutions:

  1. Resize images.
  2. Load image on demand (lazy).

Resizing images to 800px width reduce used heap to 600MB. For that i changed the Picture's class constructor.

public Picture(final String name, final Path src) throws IOException {
    this(name, new Image(src.toUri().toURL().toExternalForm(), 800, 0, true, true, true));
}

In case where an image is loaded only when it is necessary, the most time used heap size was about 250MB, with several jumps to 500MB. Again, I changed constructors of the class Picture and introduced a new field imageUrl, so a Path just converted to a URL string and an Image object is not created.

private final String imageUrl;

public Picture(final String name, final Path src) throws IOException {
    this(name, src.toUri().toURL().toExternalForm());
}

public Picture(final String name, final String imageUrl) {
    this.name = name;
    this.imageUrl = imageUrl;
}

The image() method now doesn't return a preload image but load an image on demand and does it synchronously.

public final Image image() {
    return new Image(imageUrl);
}

For a folder with 850 images I got this:

Loading image while Picture is created and save them all in a List results to consuming a lot of memory and GC activity (that can't do anything to free memory).

graphs for cpu and heap usage without lazy loading

And with lazy loading I got these graphs.

graphs for cpu and heap usage with lazy loading

siarhei987
  • 231
  • 1
  • 6
  • Resizing the image size it's not possible, because need to display the image quality as is. Actually, I've considered lazy loading. But, since my user could go between pictures, creating a new Image every time won't make it more memory consumption rather than load everything at the beginning as Image? – Sukma Wardana Apr 14 '22 at 02:40
  • Yes. I edit the answer with test for a folder that contains 850 images and now you can see resource consumption on graphs. – siarhei987 Apr 14 '22 at 06:55
  • Finally follow your lazy loading solutions. The lazy loading performance is still acceptable than create an Image in front. – Sukma Wardana Jun 14 '22 at 07:52