Skip to content

Commit 23599d9

Browse files
committed
Significant editor + control-flow-line performance improvements
1 parent 1a3f436 commit 23599d9

File tree

2 files changed

+54
-133
lines changed

2 files changed

+54
-133
lines changed

recaf-ui/src/main/java/software/coley/recaf/ui/control/richtext/Editor.java

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
import java.util.Collections;
5555
import java.util.List;
5656
import java.util.Objects;
57+
import java.util.Optional;
5758
import java.util.concurrent.CompletableFuture;
5859
import java.util.concurrent.ExecutorService;
5960
import java.util.function.Consumer;
@@ -241,8 +242,8 @@ public StackPane getPrimaryStack() {
241242
* <b>Must be called on FX thread.</b>
242243
*/
243244
public void redrawParagraphGraphics() {
244-
int startParagraphIndex = Math.max(0, codeArea.firstVisibleParToAllParIndex() - 1);
245-
int endParagraphIndex = Math.min(codeArea.getParagraphs().size() - 1, codeArea.lastVisibleParToAllParIndex());
245+
int startParagraphIndex = Math.max(0, virtualFlow.getFirstVisibleIndex() - 1);
246+
int endParagraphIndex = Math.min(codeArea.getParagraphs().size() - 1, virtualFlow.getLastVisibleIndex());
246247
for (int i = startParagraphIndex; i <= endParagraphIndex; i++)
247248
codeArea.recreateParagraphGraphic(i);
248249
}
@@ -529,15 +530,17 @@ public CodeArea getCodeArea() {
529530
*
530531
* @return List of text nodes in the paragraph.
531532
*/
533+
@Nonnull
532534
public List<Text> getTextNodes(int paragraph) {
533535
// Get the cell from the given paragraph. It should exist since we're
534536
// initializing a paragraph graphic for it.
535-
Cell<?, ?> cell = virtualCellList.get(paragraph);
536-
if (cell == null) return Collections.emptyList();
537+
Optional<Cell<?, ?>> cell = virtualCellList.getIfMemoized(paragraph);
538+
if (cell.isEmpty())
539+
return Collections.emptyList();
537540

538541
// ParagraphBox is private in RichTextFX, but we just need to get the children so
539542
// casting to region suffices.
540-
Region paragraphBox = (Region) cell.getNode();
543+
Region paragraphBox = (Region) cell.get().getNode();
541544
ObservableList<Node> paragraphBoxChildren = paragraphBox.getChildrenUnmodifiable();
542545

543546
if (paragraphBoxChildren.isEmpty())

recaf-ui/src/main/java/software/coley/recaf/ui/pane/editing/assembler/ControlFlowLines.java

Lines changed: 46 additions & 128 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22

33
import atlantafx.base.controls.Spacer;
44
import it.unimi.dsi.fastutil.ints.Int2IntArrayMap;
5-
import it.unimi.dsi.fastutil.ints.Int2ObjectArrayMap;
6-
import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
75
import jakarta.annotation.Nonnull;
86
import jakarta.enterprise.context.Dependent;
97
import jakarta.inject.Inject;
@@ -16,7 +14,6 @@
1614
import javafx.scene.effect.ColorAdjust;
1715
import javafx.scene.effect.Effect;
1816
import javafx.scene.effect.Glow;
19-
import javafx.scene.image.WritableImage;
2017
import javafx.scene.layout.StackPane;
2118
import javafx.scene.paint.Color;
2219
import javafx.util.Duration;
@@ -34,7 +31,6 @@
3431
import software.coley.bentofx.control.canvas.PixelCanvas;
3532
import software.coley.bentofx.control.canvas.PixelPainter;
3633
import software.coley.bentofx.control.canvas.PixelPainterIntArgb;
37-
import software.coley.collections.Unchecked;
3834
import software.coley.collections.box.Box;
3935
import software.coley.collections.box.IntBox;
4036
import software.coley.observables.AbstractObservable;
@@ -47,9 +43,8 @@
4743
import software.coley.recaf.ui.control.richtext.linegraphics.AbstractTextBoundLineGraphicFactory;
4844
import software.coley.recaf.ui.control.richtext.linegraphics.LineContainer;
4945
import software.coley.recaf.util.Colors;
50-
import software.coley.recaf.util.DesktopUtil;
5146
import software.coley.recaf.util.FxThreadUtil;
52-
import software.coley.recaf.util.NumberUtil;
47+
import software.coley.recaf.util.threading.ThreadUtil;
5348

5449
import java.util.ArrayList;
5550
import java.util.Collections;
@@ -60,6 +55,7 @@
6055
import java.util.Map;
6156
import java.util.Objects;
6257
import java.util.Set;
58+
import java.util.concurrent.CompletableFuture;
6359
import java.util.function.BiConsumer;
6460
import java.util.function.Consumer;
6561

@@ -166,55 +162,59 @@ protected void onPipelineOutputUpdate() {
166162
* Caret pos change.
167163
*/
168164
private void onCaretMove(@Nonnull Change<Integer> caretChange) {
169-
try {
170-
// Find selected instruction (can be null)
171-
Box<ASTInstruction> selected = extracted();
172-
173-
// Check if the selection was a label or supported instruction.
174-
ASTInstruction current = selected.get();
175-
boolean hasSelection = false;
176-
if (current instanceof ASTLabel) {
177-
hasSelection = true;
178-
} else if (current != null) {
179-
String insnName = current.identifier().content();
180-
List<ASTElement> arguments = current.arguments();
181-
if (!arguments.isEmpty() && FLOW_INSN_SET.contains(insnName) || SWITCH_INSNS.contains(insnName)) {
165+
// Find the selected element off of the FX thread, then update our selection and line draw states on the FX thread.
166+
CompletableFuture.supplyAsync(this::findSelected, ThreadUtil.executor()).thenAcceptAsync(selected -> {
167+
try {
168+
// Check if the selection was a label or supported instruction.
169+
ASTInstruction current = selected.get();
170+
boolean hasSelection = false;
171+
if (current instanceof ASTLabel) {
182172
hasSelection = true;
173+
} else if (current != null) {
174+
String insnName = current.identifier().content();
175+
List<ASTElement> arguments = current.arguments();
176+
if (!arguments.isEmpty() && FLOW_INSN_SET.contains(insnName) || SWITCH_INSNS.contains(insnName)) {
177+
hasSelection = true;
178+
}
183179
}
180+
currentInstructionSelection.setValue(current);
181+
drawLines.setValue(hasSelection);
182+
} catch (Throwable t) {
183+
logger.error("Error updating control flow line targets", t);
184184
}
185-
currentInstructionSelection.setValue(current);
186-
drawLines.setValue(hasSelection);
187-
} catch (Throwable t) {
188-
logger.error("Error updating control flow line targets", t);
189-
}
185+
}, FxThreadUtil.executor());
190186
}
191187

192188
@Nonnull
193-
private Box<ASTInstruction> extracted() {
189+
private Box<ASTInstruction> findSelected() {
194190
Box<ASTInstruction> selected = new Box<>();
195-
int pos = editor.getCodeArea().getCaretPosition();
196-
int line = editor.getCodeArea().getCurrentParagraph() + 1;
197-
for (ASTElement element : astElements) {
198-
if (element == null)
199-
continue;
200-
if (element.range().within(pos)) {
201-
element.walk(ast -> {
202-
if (ast instanceof ASTInstruction instruction) {
203-
Location location = ast.location();
204-
if (location != null && location.line() == line)
205-
selected.set(instruction);
206-
else {
207-
String identifier = instruction.identifier().content();
208-
if (("tableswitch".equals(identifier)
209-
|| "lookupswitch".equals(identifier))
210-
&& ast.range().within(pos)) {
191+
try {
192+
int pos = editor.getCodeArea().getCaretPosition();
193+
int line = editor.getCodeArea().getCurrentParagraph() + 1;
194+
for (ASTElement element : astElements) {
195+
if (element == null)
196+
continue;
197+
if (element.range().within(pos)) {
198+
element.walk(ast -> {
199+
if (ast instanceof ASTInstruction instruction) {
200+
Location location = ast.location();
201+
if (location != null && location.line() == line)
211202
selected.set(instruction);
203+
else {
204+
String identifier = instruction.identifier().content();
205+
if (("tableswitch".equals(identifier)
206+
|| "lookupswitch".equals(identifier))
207+
&& ast.range().within(pos)) {
208+
selected.set(instruction);
209+
}
212210
}
213211
}
214-
}
215-
return !selected.isSet();
216-
});
212+
return !selected.isSet();
213+
});
214+
}
217215
}
216+
} catch (Throwable t) {
217+
logger.warn("Error finding selected AST element", t);
218218
}
219219
return selected;
220220
}
@@ -336,7 +336,6 @@ private class ControlFlowLineFactory extends AbstractTextBoundLineGraphicFactory
336336
private static final int MASK_SOUTH = 2;
337337
private static final int MASK_EAST = 4;
338338
private final ArrayList<ASTElement> switchDestinations = new ArrayList<>(64);
339-
private final Int2ObjectMap<ImageRecycler> recyclers = new Int2ObjectArrayMap<>();
340339
private final int[] offsets = new int[containerWidth];
341340
private final long rainbowHueRotationDurationMillis = 3000;
342341
private final PixelPainter<?> pixelPainter = new PixelPainterIntArgb();
@@ -409,10 +408,11 @@ public void apply(@Nonnull StackPane container, int paragraph) {
409408

410409
// If we've gotten to this point we will need a canvas to draw the lines on.
411410
if (canvas == null) {
412-
canvas = new CachingPixelCanvas(pixelPainter, (int) width, (int) height);
411+
canvas = new PixelCanvas(pixelPainter, (int) width, (int) height);
413412
canvas.setManaged(false);
414413
canvas.setMouseTransparent(true);
415414
canvas.resize(width, height);
415+
canvas.clear();
416416

417417
// Setup canvas styling for the render mode.
418418
var renderMode = config.getRenderMode().getValue();
@@ -531,7 +531,6 @@ public void apply(@Nonnull StackPane container, int paragraph) {
531531
}
532532

533533
public void cleanup() {
534-
recyclers.clear();
535534
switchDestinations.clear();
536535
}
537536

@@ -578,87 +577,6 @@ protected void interpolate(double frac) {
578577
}
579578
};
580579
}
581-
582-
/**
583-
* @param width
584-
* Image width.
585-
* @param height
586-
* Image height.
587-
*
588-
* @return An image recycler for the given dimensions.
589-
*/
590-
@Nonnull
591-
private ImageRecycler getRecycler(int width, int height) {
592-
int key = width << 16 | height;
593-
return recyclers.computeIfAbsent(key, i -> new ImageRecycler(width, height));
594-
}
595-
596-
/**
597-
* Ring buffer of {@link WritableImage} keyed to a specific width/height.
598-
*/
599-
private static class ImageRecycler {
600-
private static final int MAX_IMAGE_BUFFER;
601-
private final List<WritableImage> images = new ArrayList<>(MAX_IMAGE_BUFFER);
602-
private final int width, height;
603-
private int index;
604-
605-
static {
606-
// Each row is ~18px so if we divide the screen height by that amount we should
607-
// get the number of images needed to not visually show that they're recycled.
608-
//
609-
// Of course, we want to have some leeway, so we'll round ~18px down a bit.
610-
// And if the UI scale property is assigned, we'll also accommodate for that.
611-
Number scale = Unchecked.getOr(() -> NumberUtil.parse(System.getProperty("sun.java2d.uiScale", "1.0")), 1);
612-
double scaledHeight = DesktopUtil.getLargestScreenSize().height * scale.doubleValue();
613-
final double rowHeight = 15D;
614-
final double minRows = 72; // I have ~60 rows on a 1080p monitor so this is probably common enough fallback.
615-
MAX_IMAGE_BUFFER = (int) Math.max(minRows, scaledHeight / rowHeight);
616-
}
617-
618-
/**
619-
* @param width
620-
* Width of images to create.
621-
* @param height
622-
* Height of images to create.
623-
*/
624-
public ImageRecycler(int width, int height) {
625-
this.width = width;
626-
this.height = height;
627-
}
628-
629-
/**
630-
* @return Next available image.
631-
*/
632-
@Nonnull
633-
public WritableImage next() {
634-
if (index >= MAX_IMAGE_BUFFER)
635-
index = 0;
636-
WritableImage image;
637-
if (index >= images.size()) {
638-
image = new WritableImage(width, height);
639-
images.add(image);
640-
} else {
641-
image = images.get(index);
642-
}
643-
index++;
644-
return image;
645-
}
646-
}
647-
648-
/**
649-
* Canvas implementation that recycles backing images via {@link ImageRecycler}.
650-
*/
651-
private class CachingPixelCanvas extends PixelCanvas {
652-
public CachingPixelCanvas(@Nonnull PixelPainter<?> pixelPainter, int width, int height) {
653-
super(pixelPainter, width, height);
654-
}
655-
656-
@Nonnull
657-
@Override
658-
protected WritableImage newImage(int width, int height) {
659-
return getRecycler(width, height).next();
660-
}
661-
}
662580
}
663581

664582
@SuppressWarnings("rawtypes")

0 commit comments

Comments
 (0)