Skip to content

Commit

Permalink
Don't sort Alias TableView while bulk changes are applied (#1039)
Browse files Browse the repository at this point in the history 
* Don't sort backing SortedList while AliasBulkEditor is applying changes.

SortedList takes too long to sort and can cause large slowdowns and UI
freezes while many changes to Aliases are applied at once via the
AliasBulkEditor.
  • Loading branch information
Noah Lynch committed Nov 14, 2021
1 parent 770f58c commit 87e9bf8
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 13 deletions.
62 changes: 57 additions & 5 deletions src/main/java/io/github/dsheirer/gui/playlist/alias/AliasBulkEditor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,19 @@

package io.github.dsheirer.gui.playlist.alias;

import java.util.List;

import org.controlsfx.control.ToggleSwitch;

import com.google.common.collect.Ordering;

import io.github.dsheirer.alias.Alias;
import io.github.dsheirer.gui.playlist.Editor;
import io.github.dsheirer.icon.Icon;
import io.github.dsheirer.playlist.PlaylistManager;
import javafx.beans.property.BooleanProperty;
import javafx.beans.property.ReadOnlyBooleanProperty;
import javafx.beans.property.SimpleBooleanProperty;
import javafx.collections.transformation.SortedList;
import javafx.event.ActionEvent;
import javafx.event.EventHandler;
Expand All @@ -41,9 +49,6 @@
import javafx.scene.layout.GridPane;
import javafx.scene.paint.Color;
import javafx.util.Callback;
import org.controlsfx.control.ToggleSwitch;

import java.util.List;

/**
* Editor for multiple selected aliases providing limited options for changing attributes of multiple aliases
Expand All @@ -60,6 +65,9 @@ public class AliasBulkEditor extends Editor<List<Alias>>
private ToggleSwitch mMonitorAudioToggleSwitch;
private ComboBox<Integer> mMonitorPriorityComboBox;
private Button mApplyMonitorButton;

private BooleanProperty mChangeInProgressProperty;
private ReadOnlyBooleanProperty mChangeInProgressROProperty;

/**
* Constructs an instance
Expand All @@ -68,6 +76,8 @@ public class AliasBulkEditor extends Editor<List<Alias>>
public AliasBulkEditor(PlaylistManager playlistManager)
{
mPlaylistManager = playlistManager;

mChangeInProgressProperty = new SimpleBooleanProperty();

GridPane gridPane = new GridPane();
gridPane.setPadding(new Insets(10,10,10,10));
Expand Down Expand Up @@ -160,6 +170,29 @@ public void dispose()
{
//no-op
}

/**
* Property indicating whether a bulk change is in progress. Bulk changes may cause drastic
* slow downs in some UI components.
* @return changeInProgressProperty
*/
public ReadOnlyBooleanProperty changeInProgressProperty()
{
if(mChangeInProgressROProperty == null)
mChangeInProgressROProperty = BooleanProperty.readOnlyBooleanProperty(mChangeInProgressProperty);

return mChangeInProgressROProperty;
}

private void startChange()
{
mChangeInProgressProperty.set(true);
}

private void endChange()
{
mChangeInProgressProperty.set(false);
}

private Label getEditingLabel()
{
Expand Down Expand Up @@ -189,12 +222,16 @@ private Button getApplyColorButton()
{
mApplyColorButton = new Button("Apply");
mApplyColorButton.setOnAction(event -> {
startChange();

int colorValue = ColorUtil.toInteger(getColorPicker().getValue());

for(Alias alias: getItem())
{
alias.setColor(colorValue);
}

endChange();
});
}

Expand All @@ -207,10 +244,14 @@ private Button getResetColorButton()
{
mResetColorButton = new Button("Reset Color");
mResetColorButton.setOnAction(event -> {
startChange();

for(Alias alias: getItem())
{
alias.setColor(0);
}

endChange();
});
}

Expand Down Expand Up @@ -246,6 +287,8 @@ private Button getApplyIconButton()
@Override
public void handle(ActionEvent event)
{
startChange();

Icon icon = getIconNodeComboBox().getSelectionModel().getSelectedItem();

if(icon != null)
Expand All @@ -255,6 +298,8 @@ public void handle(ActionEvent event)
alias.setIconName(icon.getName());
}
}

endChange();
}
});
}
Expand Down Expand Up @@ -295,6 +340,7 @@ private ComboBox<Integer> getMonitorPriorityComboBox()
return mMonitorPriorityComboBox;
}

private static int setCount = 0;
private Button getApplyMonitorButton()
{
if(mApplyMonitorButton == null)
Expand All @@ -305,6 +351,8 @@ private Button getApplyMonitorButton()
@Override
public void handle(ActionEvent event)
{
startChange();

boolean canMonitor = getMonitorAudioToggleSwitch().isSelected();
Integer priority = getMonitorPriorityComboBox().getSelectionModel().getSelectedItem();
if(canMonitor)
Expand All @@ -318,11 +366,15 @@ public void handle(ActionEvent event)
{
priority = io.github.dsheirer.alias.id.priority.Priority.DO_NOT_MONITOR;
}


final Integer pri = priority;
for(Alias alias: getItem())
{
alias.setCallPriority(priority);
alias.setCallPriority(pri);
//System.out.println(++setCount);
}

endChange();
}
});
}
Expand Down
30 changes: 22 additions & 8 deletions src/main/java/io/github/dsheirer/gui/playlist/alias/AliasConfigurationEditor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,15 @@

package io.github.dsheirer.gui.playlist.alias;

import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import java.util.function.Predicate;

import org.controlsfx.control.textfield.TextFields;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import io.github.dsheirer.alias.Alias;
import io.github.dsheirer.alias.AliasFactory;
import io.github.dsheirer.alias.AliasList;
Expand Down Expand Up @@ -68,14 +77,6 @@
import jiconfont.IconCode;
import jiconfont.icons.font_awesome.FontAwesome;
import jiconfont.javafx.IconNode;
import org.controlsfx.control.textfield.TextFields;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import java.util.function.Predicate;

/**
* Editor for aliases
Expand Down Expand Up @@ -445,6 +446,19 @@ private SortedList<Alias> getAliasSortedList()
{
mAliasSortedList = new SortedList<>(getAliasFilteredList());
mAliasSortedList.comparatorProperty().bind(getAliasTableView().comparatorProperty());

//Don't re-sort while the bulk editor is still applying changes to aliases
getAliasBulkEditor().changeInProgressProperty().addListener((observable, oldValue, newValue) -> {
if(newValue)
{
mAliasSortedList.comparatorProperty().unbind();
mAliasSortedList.setComparator(null);
}
else
{
mAliasSortedList.comparatorProperty().bind(getAliasTableView().comparatorProperty());
}
});
}

return mAliasSortedList;
Expand Down

0 comments on commit 87e9bf8

Please sign in to comment.