mirror of
https://github.com/bancika/diy-layout-creator
synced 2026-11-07 12:24:58 +10:00
explorer refresh improvement
This commit is contained in:
Bane Stojkovic
@@ -605,6 +605,10 @@
|
||||
<changeType>IMPROVEMENT</changeType>
|
||||
<description>Include 'Potentiometer (Miniature)' and 'Trimmer Potentiometer' in Guitar Wiring Analyzer</description>
|
||||
</org.diylc.appframework.update.Change>
|
||||
<org.diylc.appframework.update.Change>
|
||||
<changeType>IMPROVEMENT</changeType>
|
||||
<description>Improve performance of 'Project Explorer' pane</description>
|
||||
</org.diylc.appframework.update.Change>
|
||||
<org.diylc.appframework.update.Change>
|
||||
<changeType>BUG_FIX</changeType>
|
||||
<description>Fixed the issue of disappearing components in some edge cases</description>
|
||||
|
||||
+88
-51
@@ -5,6 +5,7 @@ import java.awt.Dimension;
|
||||
import java.awt.Font;
|
||||
import java.awt.GridBagConstraints;
|
||||
import java.awt.GridBagLayout;
|
||||
import java.awt.event.HierarchyEvent;
|
||||
import java.awt.event.ItemEvent;
|
||||
import java.awt.event.ItemListener;
|
||||
import java.awt.event.MouseAdapter;
|
||||
@@ -14,6 +15,7 @@ import java.util.Arrays;
|
||||
import java.util.Collection;
|
||||
import java.util.Collections;
|
||||
import java.util.Comparator;
|
||||
import java.util.HashMap;
|
||||
import java.util.HashSet;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
@@ -67,6 +69,9 @@ public class ExplorerPane extends JPanel {
|
||||
private JComboBox<Sort> sortBox;
|
||||
private ListSelectionListener selectionListener;
|
||||
|
||||
private boolean stale;
|
||||
private ExplorerSnapshot snapshot;
|
||||
|
||||
public ExplorerPane(ISwingUI swingUI, IPlugInPort plugInPort) {
|
||||
super();
|
||||
this.swingUI = swingUI;
|
||||
@@ -105,6 +110,12 @@ public class ExplorerPane extends JPanel {
|
||||
add(getSortBox(), gbc);
|
||||
|
||||
setPreferredSize(new Dimension(240, 200));
|
||||
|
||||
addHierarchyListener(e -> {
|
||||
if ((e.getChangeFlags() & HierarchyEvent.SHOWING_CHANGED) != 0 && isShowing()) {
|
||||
refreshIfStale();
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
public JLabel getTitleLabel() {
|
||||
@@ -238,6 +249,13 @@ public class ExplorerPane extends JPanel {
|
||||
});
|
||||
componentList.setModel(getComponentListModel());
|
||||
componentList.setFont(DEFAULT_FONT);
|
||||
// without a fixed height every content change re-measures each row through the HTML
|
||||
// renderer, which dominates the cost of a refresh on large projects
|
||||
componentList.setFixedCellHeight(new DefaultListCellRenderer()
|
||||
.getListCellRendererComponent(componentList,
|
||||
"<html><font color='#c0c0c0'>[L0]</font> Xg (<font color='#666666'>Xg</font>)</html>",
|
||||
0, false, false)
|
||||
.getPreferredSize().height);
|
||||
componentList.setSelectionMode(ListSelectionModel.MULTIPLE_INTERVAL_SELECTION);
|
||||
componentList.setCellRenderer(new ComponentListCellRenderer(plugInPort.getComponentTypes()));
|
||||
selectionListener = new ListSelectionListener() {
|
||||
@@ -324,10 +342,36 @@ public class ExplorerPane extends JPanel {
|
||||
|
||||
public void setComponents(List<IDIYComponent<?>> components,
|
||||
Collection<IDIYComponent<?>> selectedComponents) {
|
||||
this.getComponentListModel().setComponents(components);
|
||||
ExplorerSnapshot newSnapshot =
|
||||
ExplorerSnapshot.of(components, plugInPort.getComponentGroups());
|
||||
// moving, rotating or nudging leaves everything the list shows untouched
|
||||
if (!newSnapshot.equals(snapshot)) {
|
||||
snapshot = newSnapshot;
|
||||
this.getComponentListModel().setComponents(components);
|
||||
}
|
||||
setSelection(new HashSet<IDIYComponent<?>>(selectedComponents));
|
||||
}
|
||||
|
||||
/**
|
||||
* Called instead of {@link #setComponents} while the pane is not showing. When the project
|
||||
* itself was replaced the list is emptied so it does not hold on to the old one.
|
||||
*/
|
||||
public void markStale(boolean projectReplaced) {
|
||||
stale = true;
|
||||
if (projectReplaced && snapshot != null) {
|
||||
snapshot = null;
|
||||
this.getComponentListModel().setComponents(new ArrayList<IDIYComponent<?>>());
|
||||
}
|
||||
}
|
||||
|
||||
private void refreshIfStale() {
|
||||
if (stale) {
|
||||
stale = false;
|
||||
setComponents(plugInPort.getCurrentProject().getComponents(),
|
||||
plugInPort.getSelectedComponents());
|
||||
}
|
||||
}
|
||||
|
||||
public void setSelection(Set<IDIYComponent<?>> selectedComponents) {
|
||||
List<ComponentGroupItem> selectedValuesList = componentList.getSelectedValuesList();
|
||||
|
||||
@@ -348,61 +392,16 @@ public class ExplorerPane extends JPanel {
|
||||
if (selectedComponents.size() == 0) {
|
||||
list.clearSelection();
|
||||
} else {
|
||||
// Build a map from component to its group (if any)
|
||||
Set<ComponentGroup> groups = plugInPort.getComponentGroups();
|
||||
Map<IDIYComponent<?>, Set<IDIYComponent<?>>> componentToGroup = new java.util.HashMap<>();
|
||||
if (groups != null && !groups.isEmpty()) {
|
||||
// Build a map from UUID to component for quick lookup
|
||||
List<IDIYComponent<?>> allComponents = plugInPort.getCurrentProject().getComponents();
|
||||
Map<UUID, IDIYComponent<?>> idToComponent = allComponents.stream()
|
||||
.collect(Collectors.toMap(IDIYComponent::getId, comp -> comp));
|
||||
|
||||
for (ComponentGroup group : groups) {
|
||||
// Convert ComponentGroup (with UUIDs) to Set<IDIYComponent<?>>
|
||||
Set<IDIYComponent<?>> componentSet = group.getComponentIds().stream()
|
||||
.map(idToComponent::get)
|
||||
.filter(java.util.Objects::nonNull)
|
||||
.collect(Collectors.toSet());
|
||||
|
||||
for (IDIYComponent<?> component : componentSet) {
|
||||
componentToGroup.put(component, componentSet);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Find indices of items to select
|
||||
List<ComponentGroupItem> components = this.getComponentListModel().getComponents();
|
||||
Map<IDIYComponent<?>, Integer> rowIndex = this.getComponentListModel().getRowIndex();
|
||||
Set<Integer> selectedIndices = new HashSet<>();
|
||||
Set<Set<IDIYComponent<?>>> selectedGroups = new HashSet<>();
|
||||
|
||||
for (IDIYComponent<?> selected : selectedComponents) {
|
||||
Set<IDIYComponent<?>> group = componentToGroup.get(selected);
|
||||
if (group != null) {
|
||||
// Component is in a group
|
||||
if (!selectedGroups.contains(group)) {
|
||||
// Find the ComponentGroupItem for this group
|
||||
for (int i = 0; i < components.size(); i++) {
|
||||
ComponentGroupItem groupItem = components.get(i);
|
||||
if (groupItem.getGroup().equals(group)) {
|
||||
selectedIndices.add(i);
|
||||
selectedGroups.add(group);
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// Component is not in any group, find the ComponentGroupItem that contains only this component
|
||||
for (int i = 0; i < components.size(); i++) {
|
||||
ComponentGroupItem groupItem = components.get(i);
|
||||
if (groupItem.getGroup().size() == 1 && groupItem.getGroup().contains(selected)) {
|
||||
selectedIndices.add(i);
|
||||
break;
|
||||
}
|
||||
}
|
||||
Integer index = rowIndex.get(selected);
|
||||
if (index != null) {
|
||||
selectedIndices.add(index);
|
||||
}
|
||||
}
|
||||
|
||||
int[] indices = selectedIndices.stream().mapToInt(i -> i).toArray();
|
||||
int[] indices = selectedIndices.stream().mapToInt(i -> i).sorted().toArray();
|
||||
|
||||
list.getSelectionModel().setValueIsAdjusting(true);
|
||||
list.setSelectedIndices(indices);
|
||||
@@ -428,6 +427,7 @@ public class ExplorerPane extends JPanel {
|
||||
|
||||
private List<IDIYComponent<?>> componentsRaw;
|
||||
private List<ComponentGroupItem> components;
|
||||
private Map<IDIYComponent<?>, Integer> rowIndex = new HashMap<>();
|
||||
|
||||
private String searchText;
|
||||
private Sort sort = Sort.Z_INDEX;
|
||||
@@ -443,6 +443,10 @@ public class ExplorerPane extends JPanel {
|
||||
return components;
|
||||
}
|
||||
|
||||
public Map<IDIYComponent<?>, Integer> getRowIndex() {
|
||||
return rowIndex;
|
||||
}
|
||||
|
||||
public Sort getSort() {
|
||||
return sort;
|
||||
}
|
||||
@@ -485,6 +489,13 @@ public class ExplorerPane extends JPanel {
|
||||
|
||||
// Build list with groups and ungrouped components
|
||||
this.components = buildComponentListWithGroups(sortedComponents);
|
||||
Map<IDIYComponent<?>, Integer> newRowIndex = new HashMap<>();
|
||||
for (int i = 0; i < this.components.size(); i++) {
|
||||
for (IDIYComponent<?> component : this.components.get(i).getGroup()) {
|
||||
newRowIndex.put(component, i);
|
||||
}
|
||||
}
|
||||
this.rowIndex = newRowIndex;
|
||||
|
||||
int newSize = this.components.size();
|
||||
|
||||
@@ -716,4 +727,30 @@ public class ExplorerPane extends JPanel {
|
||||
return group.hashCode();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Everything the list reads from the project, copied so that in-place edits of components
|
||||
* and groups are detected. Components compare by identity, so an undo that swaps in clones
|
||||
* still forces a rebuild.
|
||||
*/
|
||||
private record ExplorerSnapshot(List<IDIYComponent<?>> components, List<String> labels,
|
||||
Map<Set<UUID>, String> groups) {
|
||||
|
||||
static ExplorerSnapshot of(List<IDIYComponent<?>> components, Set<ComponentGroup> groups) {
|
||||
List<String> labels = new ArrayList<>(components.size() * 3);
|
||||
for (IDIYComponent<?> component : components) {
|
||||
labels.add(component.getName());
|
||||
// toString() drives the name and type sorts
|
||||
labels.add(component.toString());
|
||||
labels.add(component.getValueForDisplay());
|
||||
}
|
||||
Map<Set<UUID>, String> groupNames = new HashMap<>();
|
||||
if (groups != null) {
|
||||
for (ComponentGroup group : groups) {
|
||||
groupNames.put(new HashSet<>(group.getComponentIds()), group.getName());
|
||||
}
|
||||
}
|
||||
return new ExplorerSnapshot(new ArrayList<>(components), labels, groupNames);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+9
-1
@@ -57,12 +57,19 @@ public class ExplorerPlugin implements IPlugIn {
|
||||
|
||||
@Override
|
||||
public EnumSet<EventType> getSubscribedEventTypes() {
|
||||
return EnumSet.of(EventType.FILE_STATUS_CHANGED, EventType.PROJECT_LOADED, EventType.PROJECT_MODIFIED, EventType.SELECTION_CHANGED);
|
||||
return EnumSet.of(EventType.PROJECT_LOADED, EventType.PROJECT_MODIFIED,
|
||||
EventType.SELECTION_CHANGED);
|
||||
}
|
||||
|
||||
@SuppressWarnings("unchecked")
|
||||
@Override
|
||||
public void processMessage(EventType eventType, Object... params) {
|
||||
// the dispatcher offers no way to unsubscribe, so a collapsed pane only records that it
|
||||
// has to catch up once it is shown again
|
||||
if (!getExplorerPane().isShowing()) {
|
||||
getExplorerPane().markStale(eventType == EventType.PROJECT_LOADED);
|
||||
return;
|
||||
}
|
||||
switch (eventType) {
|
||||
case PROJECT_MODIFIED:
|
||||
getExplorerPane().setComponents(((Project)params[1]).getComponents(), plugInPort.getSelectedComponents());
|
||||
@@ -72,6 +79,7 @@ public class ExplorerPlugin implements IPlugIn {
|
||||
break;
|
||||
case SELECTION_CHANGED:
|
||||
getExplorerPane().setSelection((Set<IDIYComponent<?>>)params[0]);
|
||||
break;
|
||||
default:
|
||||
break;
|
||||
}
|
||||
|
||||
@@ -0,0 +1,134 @@
|
||||
# Project Explorer Refresh Cost
|
||||
|
||||
`ExplorerPane` (`diylc-swing/.../plugins/explorer`) rebuilds its whole list on every
|
||||
`PROJECT_MODIFIED`, `PROJECT_LOADED` and `SELECTION_CHANGED`, even when the panel is collapsed
|
||||
and even when the change cannot affect what it shows. This plan removes both costs without
|
||||
touching `diylc-core` or the event contract.
|
||||
|
||||
## 1. How the explorer is driven today
|
||||
|
||||
- `Presenter` uses `MessageDispatcher<EventType>(true)` — **synchronous**. Every
|
||||
`processMessage` runs on the EDT inside the call that changed the project, so explorer work
|
||||
adds directly to the latency of the edit that triggered it.
|
||||
- The dispatcher reads `getSubscribedEventTypes()` **once, at registration**, and there is no
|
||||
unregister path on `IPlugInPort`. "Disconnecting" therefore means returning early from
|
||||
`processMessage`, which costs one method call per event and is equivalent in practice.
|
||||
- `ExplorerPlugin.processMessage`:
|
||||
- `PROJECT_MODIFIED` / `PROJECT_LOADED` → `ExplorerPane.setComponents(all, selection)`.
|
||||
- `SELECTION_CHANGED` → `setSelection(...)`. (It also falls through to `default`; harmless.)
|
||||
- `FILE_STATUS_CHANGED` is subscribed but unused.
|
||||
- What `setComponents` costs, for *n* components:
|
||||
1. sort/filter/reverse copy of the component list;
|
||||
2. `buildComponentListWithGroups` — a UUID→component map over the whole project plus a
|
||||
set per group;
|
||||
3. `fireContentsChanged(0, n)` — with no fixed cell height, `BasicListUI` re-measures
|
||||
**every row** through the renderer, and most rows are `<html>` labels, so each row is
|
||||
an HTML parse. This is the dominant cost;
|
||||
4. `setSelection` rebuilds the group map again and does a linear scan per selected item.
|
||||
|
||||
### Which interactions reach the explorer
|
||||
|
||||
Read from `Presenter`; to be confirmed by measurement (step 0).
|
||||
|
||||
| Interaction | Events reaching the explorer | Changes the list? |
|
||||
| --- | --- | --- |
|
||||
| Mouse drag of components / control points, per mouse move | none — `dragOver` fires only `REPAINT` | — |
|
||||
| Drag start (`dragStarted`) | `SELECTION_CHANGED` | selection only |
|
||||
| Drag end (`dragEnded`) | `SELECTION_CHANGED` + `PROJECT_MODIFIED("Drag")` | no |
|
||||
| Arrow-key nudge (`nudgeSelection`), per key repeat | `PROJECT_MODIFIED("Move Selection")` | no |
|
||||
| Rotate / mirror / align / distribute | `PROJECT_MODIFIED` | no |
|
||||
| Click on canvas (even on the already-selected part) | `SELECTION_CHANGED` | selection only |
|
||||
| Edit properties (name, value) | `PROJECT_MODIFIED("Edit Selection")` | yes |
|
||||
| Add / delete / duplicate / paste, z-order, group / ungroup, undo / redo | `PROJECT_MODIFIED` or `PROJECT_LOADED` | yes |
|
||||
|
||||
So the explorer does not rebuild on every mouse move; it rebuilds once per *gesture* and once
|
||||
per *key repeat*, and each rebuild is O(n) HTML layouts. On a large project that is the stall
|
||||
felt at the start and end of each drag and the lag when holding an arrow key. If profiling in
|
||||
step 0 shows explorer work *during* a drag, something else is dispatching and needs finding
|
||||
before this plan is enough.
|
||||
|
||||
## 2. Decisions
|
||||
|
||||
**Visibility is checked with `isShowing()` and a `HierarchyListener`, not the config key.**
|
||||
`CollapsiblePanel` hides the pane with `setVisible(false)`; `isShowing()` also covers an
|
||||
ancestor being hidden and the frame not yet being displayed. `HierarchyEvent.SHOWING_CHANGED`
|
||||
fires for all of those, whereas listening to `ConfigPlugin.PROJECT_EXPLORER` would miss
|
||||
everything except the menu toggle.
|
||||
|
||||
**A hidden pane keeps no state in sync; it just remembers that it is stale.** On becoming
|
||||
visible it pulls the current project and selection from `IPlugInPort` rather than replaying
|
||||
events. On `PROJECT_LOADED` while hidden it also drops its model so it does not pin the
|
||||
previous project's object graph.
|
||||
|
||||
**"Does this change affect the list" is decided by comparing a snapshot of what the list
|
||||
shows, not by the action string.** `PROJECT_MODIFIED` carries a description (`"Drag"`,
|
||||
`"Move Selection"`, …) that could be whitelisted, but those strings are scattered through
|
||||
`Presenter`, are not a contract, and a missed one shows stale data. A snapshot is robust:
|
||||
the pane records exactly the inputs its model and renderer read, and skips the rebuild when
|
||||
they are unchanged. Capturing it is O(n) plain field reads, far cheaper than O(n) HTML
|
||||
layouts.
|
||||
|
||||
The snapshot is a small private record (per `typed-records-over-string-keyed-maps`), holding
|
||||
copies, not live references, because `Presenter` mutates components and groups in place:
|
||||
|
||||
- the component list **by identity, in z-order**;
|
||||
- per component: `getName()`, `toString()` (used by the NAME/TYPE sorts) and
|
||||
`getValueForDisplay()`;
|
||||
- per group: name and set of member UUIDs (`ComponentGroup` has no `equals`, and a rename
|
||||
mutates the existing instance).
|
||||
|
||||
The layer badge comes from `ComponentType.getZOrder()`, which is static per class, so it is
|
||||
covered by identity.
|
||||
|
||||
**Selection sync is O(selected), not O(n).** `refresh()` builds a
|
||||
`Map<IDIYComponent<?>, Integer>` from component to row index alongside the row list.
|
||||
`setSelection` uses it instead of rebuilding the group map and scanning rows. The existing
|
||||
"selection already matches" early-out stays.
|
||||
|
||||
**Rows get a fixed height.** `setFixedCellHeight` from the font metrics stops `BasicListUI`
|
||||
from measuring every row through the HTML renderer whenever contents change. Rows are a
|
||||
single line, so nothing is lost. This also speeds up the rebuilds that remain necessary.
|
||||
|
||||
**No debounce timer for now.** With the snapshot check, a nudge burst costs one O(n)
|
||||
comparison per repeat and no layout. A `javax.swing.Timer` coalescer would add latency to
|
||||
genuine edits for little gain; revisit only if step 0 shows the comparison itself matters.
|
||||
|
||||
## 3. Changes
|
||||
|
||||
All in `diylc-swing/src/main/java/org/diylc/swing/plugins/explorer/`. No core, library or
|
||||
file-format changes; no regression-suite impact (the explorer is not rendered in exports).
|
||||
|
||||
1. **`ExplorerPane`**
|
||||
- Field `boolean stale`; method `refreshIfStale()` that, when showing, calls
|
||||
`setComponents(plugInPort.getCurrentProject().getComponents(),
|
||||
plugInPort.getSelectedComponents())`.
|
||||
- `HierarchyListener` in the constructor: on `SHOWING_CHANGED` with `isShowing()`, call
|
||||
`refreshIfStale()`.
|
||||
- `setComponents`: build the snapshot; if equal to the previous one, skip
|
||||
`ComponentListModel.setComponents` and go straight to `setSelection`.
|
||||
- `ComponentListModel.refresh()`: also build the component→row map.
|
||||
- `setSelection`: use the row map.
|
||||
- `getComponentList()`: `setFixedCellHeight(...)`.
|
||||
- Snapshot record declared at the bottom of the file with the other nested types.
|
||||
2. **`ExplorerPlugin.processMessage`**
|
||||
- First line: if `!explorerPane.isShowing()`, mark the pane stale (and on `PROJECT_LOADED`
|
||||
clear its model), then return.
|
||||
- Add the missing `break` after `SELECTION_CHANGED`.
|
||||
- Drop `FILE_STATUS_CHANGED` from `getSubscribedEventTypes()`.
|
||||
|
||||
Search and sort changes call `refresh()` directly and bypass the snapshot
|
||||
check; the snapshot only gates the event-driven path.
|
||||
|
||||
## 4. Status and next steps
|
||||
|
||||
Everything in §3 is implemented in the working tree and `diylc-swing` compiles. Not yet done:
|
||||
|
||||
1. **Measure.** On a large project (several hundred components), time `setComponents` and
|
||||
`setSelection`, or run a profiler, while dragging, holding an arrow key and editing a
|
||||
value. Compare against `master` to confirm the table in §1 and the gain.
|
||||
2. **Manual check in the running app.** Collapse the explorer, edit, then expand: the list and
|
||||
selection should be correct. Repeat with undo/redo and file open while it is collapsed.
|
||||
Also check: rename and value edits in the property editor; group, rename the group, then
|
||||
ungroup; z-order drag inside the explorer; search and sort while visible; row height
|
||||
looks right under each look-and-feel/theme.
|
||||
3. Add a debounce only if step 1 shows the snapshot comparison itself is significant.
|
||||
Reference in New Issue
Block a user