From 316cdb4068f0ab0f24a28ca36183a8c141642dfb Mon Sep 17 00:00:00 2001 From: Denny Sheirer Date: Thu, 7 May 2020 09:51:21 +0000 Subject: [PATCH] #800 Resolves issue with rendering of icons in playlist alias editor and slow loading of icons in dropdown combobox. Adds error checking to the icon manager to notify user when there is an issue with loading a custom icon in the ADD method. (#803) Co-authored-by: Denny --- .../github/dsheirer/gui/icon/IconManager.java | 39 ++++++++-------- .../alias/AliasConfigurationEditor.java | 10 ++-- .../gui/playlist/alias/AliasItemEditor.java | 42 +++++++---------- .../java/io/github/dsheirer/icon/Icon.java | 46 ++++++++++++++++++- 4 files changed, 83 insertions(+), 54 deletions(-) diff --git a/src/main/java/io/github/dsheirer/gui/icon/IconManager.java b/src/main/java/io/github/dsheirer/gui/icon/IconManager.java index 660ec3fb..efceffbb 100644 --- a/src/main/java/io/github/dsheirer/gui/icon/IconManager.java +++ b/src/main/java/io/github/dsheirer/gui/icon/IconManager.java @@ -38,7 +38,6 @@ import javafx.scene.control.TableView; import javafx.scene.control.TextField; import javafx.scene.control.TitledPane; import javafx.scene.control.cell.PropertyValueFactory; -import javafx.scene.image.Image; import javafx.scene.image.ImageView; import javafx.scene.layout.GridPane; import javafx.scene.layout.HBox; @@ -358,7 +357,20 @@ public class IconManager extends Editor Icon icon = new Icon(); icon.setName(name); icon.setPath(path); - mIconModel.addIcon(icon); + + if(icon.getFxImage() != null && icon.getFxImage().getException() != null) + { + Alert alert = new Alert(Alert.AlertType.ERROR, "Unable to load icon image from selected " + + "file. Please select a valid image file for the icon", ButtonType.OK); + alert.setHeaderText("Invalid image file"); + alert.setTitle("Save Icon"); + alert.showAndWait(); + return; + } + else + { + mIconModel.addIcon(icon); + } } showEditor(false); @@ -538,30 +550,15 @@ public class IconManager extends Editor { Icon icon = getTableRow().getItem(); - if(icon != null) + if(icon != null && icon.getFxImage() != null) { - try - { - if(icon.getPath().startsWith("images")) - { - Image image = new Image(icon.getPath(), 0, 16, true, true); - setGraphic(new ImageView(image)); - } - else - { - Path path = Path.of(icon.getPath()); - Image image = new Image(path.toUri().toString(), 0, 16, true, true); - setGraphic(new ImageView(image)); - } - } - catch(Exception e) - { - mLog.error("Error loading image: " + (icon != null ? icon.getPath() : "null"), e); - } + setGraphic(new ImageView(icon.getFxImage())); + setText(null); } else { setGraphic(null); + setText("Can't load image"); } } } diff --git a/src/main/java/io/github/dsheirer/gui/playlist/alias/AliasConfigurationEditor.java b/src/main/java/io/github/dsheirer/gui/playlist/alias/AliasConfigurationEditor.java index b09e773d..3a706158 100644 --- a/src/main/java/io/github/dsheirer/gui/playlist/alias/AliasConfigurationEditor.java +++ b/src/main/java/io/github/dsheirer/gui/playlist/alias/AliasConfigurationEditor.java @@ -56,7 +56,6 @@ import javafx.scene.control.TextField; import javafx.scene.control.TextFormatter; import javafx.scene.control.TextInputDialog; import javafx.scene.control.cell.PropertyValueFactory; -import javafx.scene.image.Image; import javafx.scene.image.ImageView; import javafx.scene.layout.HBox; import javafx.scene.layout.Priority; @@ -779,14 +778,13 @@ public class AliasConfigurationEditor extends SplitPane { Icon icon = mPlaylistManager.getIconModel().getIcon(alias.getIconName()); - try + if(icon != null && icon.getFxImage() != null) { - Image image = new Image(icon.getPath(), 0, 16, true, true); - setGraphic(new ImageView(image)); + setGraphic(new ImageView(icon.getFxImage())); } - catch(Exception e) + else { - + setGraphic(null); } } } diff --git a/src/main/java/io/github/dsheirer/gui/playlist/alias/AliasItemEditor.java b/src/main/java/io/github/dsheirer/gui/playlist/alias/AliasItemEditor.java index 86bdd8a2..46a9b251 100644 --- a/src/main/java/io/github/dsheirer/gui/playlist/alias/AliasItemEditor.java +++ b/src/main/java/io/github/dsheirer/gui/playlist/alias/AliasItemEditor.java @@ -22,6 +22,7 @@ package io.github.dsheirer.gui.playlist.alias; +import com.google.common.collect.Ordering; import com.google.common.eventbus.Subscribe; import impl.org.controlsfx.autocompletion.AutoCompletionTextFieldBinding; import impl.org.controlsfx.autocompletion.SuggestionProvider; @@ -67,6 +68,7 @@ import javafx.beans.value.ChangeListener; import javafx.beans.value.ObservableValue; import javafx.collections.FXCollections; import javafx.collections.ListChangeListener; +import javafx.collections.transformation.SortedList; import javafx.event.ActionEvent; import javafx.event.EventHandler; import javafx.geometry.HPos; @@ -88,8 +90,8 @@ import javafx.scene.control.ScrollPane; import javafx.scene.control.SeparatorMenuItem; import javafx.scene.control.TextField; import javafx.scene.control.TitledPane; -import javafx.scene.image.Image; import javafx.scene.image.ImageView; +import javafx.scene.layout.ColumnConstraints; import javafx.scene.layout.GridPane; import javafx.scene.layout.HBox; import javafx.scene.layout.Priority; @@ -103,7 +105,6 @@ import org.controlsfx.control.ToggleSwitch; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.nio.file.Path; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -1041,7 +1042,7 @@ public class AliasItemEditor extends Editor mIconNodeComboBox = new ComboBox<>(); mIconNodeComboBox.setMaxWidth(Double.MAX_VALUE); mIconNodeComboBox.setDisable(true); - mIconNodeComboBox.setItems(mPlaylistManager.getIconModel().iconsProperty()); + mIconNodeComboBox.setItems(new SortedList(mPlaylistManager.getIconModel().iconsProperty(), Ordering.natural())); mIconNodeComboBox.setCellFactory(new IconCellFactory()); mIconNodeComboBox.getSelectionModel().selectedItemProperty() .addListener((observable, oldValue, newValue) -> modifiedProperty().set(true)); @@ -1556,6 +1557,15 @@ public class AliasItemEditor extends Editor @Override public ListCell call(ListView param) { + Label iconLabel = new Label(); + Label textLabel = new Label(); + GridPane gridPane = new GridPane(); + gridPane.setHgap(5); + GridPane.setHalignment(iconLabel, HPos.RIGHT); + gridPane.getColumnConstraints().add(new ColumnConstraints(50)); + gridPane.add(iconLabel, 0, 0); + gridPane.add(textLabel,1,0); + ListCell cell = new ListCell<>() { @Override @@ -1570,29 +1580,9 @@ public class AliasItemEditor extends Editor } else { - setText(item.getName()); - - String path = item.getPath(); - - try - { - if(path.startsWith("images")) - { - Image image = new Image(path, 0, 20, true, true); - setGraphic(new ImageView(image)); - } - else - { - Path filePath = Path.of(path); - Image image = new Image(filePath.toUri().toString(), 0, 16, true, true); - setGraphic(new ImageView(image)); - } - - } - catch(Exception e) - { - mLog.error("Error loading icon for [" + item.getName() + "] path [" + item.getPath() + "]"); - } + textLabel.setText(item.getName()); + iconLabel.setGraphic(new ImageView(item.getFxImage())); + setGraphic(gridPane); } } }; diff --git a/src/main/java/io/github/dsheirer/icon/Icon.java b/src/main/java/io/github/dsheirer/icon/Icon.java index 4482b9ea..5bbcd408 100644 --- a/src/main/java/io/github/dsheirer/icon/Icon.java +++ b/src/main/java/io/github/dsheirer/icon/Icon.java @@ -27,24 +27,28 @@ import javafx.beans.property.BooleanProperty; import javafx.beans.property.SimpleBooleanProperty; import javafx.beans.property.SimpleStringProperty; import javafx.beans.property.StringProperty; +import javafx.scene.image.Image; import javafx.util.Callback; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import javax.swing.ImageIcon; import java.net.URL; +import java.nio.file.Path; import java.util.Objects; @JacksonXmlRootElement(localName = "icon") public class Icon implements Comparable { private final static Logger mLog = LoggerFactory.getLogger(Icon.class); - + private static final int ICON_HEIGHT_JAVAFX = 16; private StringProperty mName = new SimpleStringProperty(); private StringProperty mPath = new SimpleStringProperty(); private BooleanProperty mDefaultIcon = new SimpleBooleanProperty(); private BooleanProperty mStandardIcon = new SimpleBooleanProperty(); private ImageIcon mImageIcon; + private Image mFxImage; + private boolean mFxImageLoaded = false; /** * JAXB constructor - do not use @@ -199,6 +203,46 @@ public class Icon implements Comparable return mImageIcon; } + /** + * Lazy loads an FX image for the icon and retains it in memory. + * @return loaded image or null if the image can't be loaded + */ + @JsonIgnore + public Image getFxImage() + { + if(!mFxImageLoaded && getPath() != null && !getPath().isEmpty()) + { + //Set the loaded flag to true regardless if the image loads so that if there is an error loading the + //image we log it once and don't attempt to reload it again + mFxImageLoaded = true; + + if(getPath() == null || getPath().isEmpty()) + { + mLog.error("Error loading icon [" + getName() + "] - null or empty file path to image"); + } + else + { + if(getPath().startsWith("images")) + { + mFxImage = new Image(getPath(), 0, ICON_HEIGHT_JAVAFX, true, true); + } + else + { + Path filePath = Path.of(getPath()); + mFxImage = new Image(filePath.toUri().toString(), 0, ICON_HEIGHT_JAVAFX, true, true); + } + + if(mFxImage.getException() != null) + { + mLog.error("Error loading icon [" + getName() + " " + getPath() + "] - " + + mFxImage.getException().getLocalizedMessage()); + } + } + } + + return mFxImage; + } + @Override public int compareTo(Icon other) {