From 654f8d8fb3bca32929cac26b6d989aa28af60d82 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Sat, 30 May 2026 10:33:07 +0300 Subject: [PATCH] Dedupe simulator paint-scope leak warnings (#5102) The paint-scope checks added for #5058 fire on every offending repaint, so framework components that routinely leak harmless state (BGPainter, Toolbar, Button, TextArea, the simulator's own SelectedComponentGlassPane) flooded the EDT log at ~60Hz and burned CPU formatting it. Track a (owner-class, leak-kinds) signature per warning and only emit the first occurrence. Auto-restore still runs every time so on-device behavior is unchanged. Also fix SelectedComponentGlassPane to actually save/restore color and alpha around its highlight fill. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../impl/javase/ComponentTreeInspector.java | 16 ++-- .../com/codename1/impl/javase/JavaSEPort.java | 33 +++++++- .../codename1/impl/javase/PaintScopeTest.java | 76 +++++++++++++++++++ 3 files changed, 115 insertions(+), 10 deletions(-) diff --git a/Ports/JavaSE/src/com/codename1/impl/javase/ComponentTreeInspector.java b/Ports/JavaSE/src/com/codename1/impl/javase/ComponentTreeInspector.java index fd67e2d343..e2463f15ce 100644 --- a/Ports/JavaSE/src/com/codename1/impl/javase/ComponentTreeInspector.java +++ b/Ports/JavaSE/src/com/codename1/impl/javase/ComponentTreeInspector.java @@ -74,12 +74,16 @@ public SelectedComponentGlassPane(Component cmp) { @Override public void paint(Graphics g, Rectangle rect) { - g.setAlpha(30); - g.setColor(0xff0000); - - g.fillRect(cmp.getAbsoluteX(), cmp.getAbsoluteY(), cmp.getWidth(), cmp.getHeight()); - - g.setAlpha(255); + int oldColor = g.getColor(); + int oldAlpha = g.getAlpha(); + try { + g.setAlpha(30); + g.setColor(0xff0000); + g.fillRect(cmp.getAbsoluteX(), cmp.getAbsoluteY(), cmp.getWidth(), cmp.getHeight()); + } finally { + g.setAlpha(oldAlpha); + g.setColor(oldColor); + } } } diff --git a/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java b/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java index fdded362d6..b6ddd5543a 100644 --- a/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java +++ b/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java @@ -8892,6 +8892,13 @@ private static final class PaintScopeSnapshot { private final boolean paintScopeChecksEnabled = !Boolean.getBoolean("cn1.disable.paint.scope.checks"); + /// Tracks `(owner-class, leak-kinds)` signatures already reported so a + /// leaky component logs once per session instead of every repaint + /// (issue #5102). The set is unbounded but its growth is bounded by the + /// number of distinct leak shapes in the running app, which is small. + private final java.util.Set reportedPaintScopeLeaks = + java.util.Collections.synchronizedSet(new java.util.HashSet()); + private int clipStackDepth(Object graphics) { if (graphics instanceof NativeScreenGraphics) { return ((NativeScreenGraphics) graphics).clipStack.size(); @@ -8965,11 +8972,13 @@ public void endPaintScope(Object graphics, Object owner) { } Graphics2D g2d = getGraphics(graphics); StringBuilder diffs = null; + StringBuilder kinds = null; int depthNow = clipStackDepth(graphics); if (depthNow != snap.clipStackDepth) { diffs = appendDiff(diffs, "clip stack depth " + snap.clipStackDepth + " -> " + depthNow + " (unmatched pushClip/popClip)"); + kinds = appendDiff(kinds, "clipStackDepth"); trimClipStack(graphics, snap.clipStackDepth); } // Transform must be restored before the clip comparison: getClip() @@ -8978,31 +8987,43 @@ public void endPaintScope(Object graphics, Object owner) { if (!g2d.getTransform().equals(snap.transform)) { diffs = appendDiff(diffs, "transform not restored " + snap.transform + " -> " + g2d.getTransform()); + kinds = appendDiff(kinds, "transform"); g2d.setTransform(snap.transform); } if (!sameShape(g2d.getClip(), snap.clip)) { diffs = appendDiff(diffs, "clip rect not restored"); + kinds = appendDiff(kinds, "clipRect"); g2d.setClip(snap.clip); } if (!equalsNullSafe(g2d.getColor(), snap.color)) { diffs = appendDiff(diffs, "color not restored " + snap.color + " -> " + g2d.getColor()); + kinds = appendDiff(kinds, "color"); g2d.setColor(snap.color); } if (!equalsNullSafe(g2d.getFont(), snap.font)) { diffs = appendDiff(diffs, "font not restored"); + kinds = appendDiff(kinds, "font"); g2d.setFont(snap.font); } if (!equalsNullSafe(g2d.getComposite(), snap.composite)) { diffs = appendDiff(diffs, "composite/alpha not restored"); + kinds = appendDiff(kinds, "composite"); g2d.setComposite(snap.composite); } if (diffs != null) { - Log.p("paint-scope: " + describe(snap.owner) - + " did not restore Graphics state: " + diffs - + ". State has been auto-restored for the simulator; on device " - + "this would persist into the next paint (see issue #5058)."); + // Dedup per (owner-class, leak-kinds): a leaky component fires every + // paint, which used to flood the EDT log (issue #5102). One warning + // per signature is enough -- the diff text is identical anyway. + String signature = ownerClass(snap.owner) + "|" + kinds; + if (reportedPaintScopeLeaks.add(signature)) { + Log.p("paint-scope: " + describe(snap.owner) + + " did not restore Graphics state: " + diffs + + ". State has been auto-restored for the simulator; on device " + + "this would persist into the next paint (see issue #5058)." + + " Further occurrences of this leak shape are suppressed."); + } } } @@ -9013,6 +9034,10 @@ private static StringBuilder appendDiff(StringBuilder b, String diff) { return b.append("; ").append(diff); } + private static String ownerClass(Object owner) { + return owner == null ? "" : owner.getClass().getName(); + } + private static boolean equalsNullSafe(Object a, Object b) { return a == null ? b == null : a.equals(b); } diff --git a/maven/javase/src/test/java/com/codename1/impl/javase/PaintScopeTest.java b/maven/javase/src/test/java/com/codename1/impl/javase/PaintScopeTest.java index b5330f8e26..dd1b6ed773 100644 --- a/maven/javase/src/test/java/com/codename1/impl/javase/PaintScopeTest.java +++ b/maven/javase/src/test/java/com/codename1/impl/javase/PaintScopeTest.java @@ -22,6 +22,7 @@ */ package com.codename1.impl.javase; +import com.codename1.io.Log; import com.codename1.testing.junit.CodenameOneTest; import org.junit.jupiter.api.Test; @@ -31,6 +32,8 @@ import java.awt.Graphics2D; import java.awt.geom.AffineTransform; import java.awt.image.BufferedImage; +import java.util.ArrayList; +import java.util.List; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -147,6 +150,79 @@ public void cleanScopeProducesNoChange() { } } + /// Capturing Log used by the dedup test below. It installs itself in + /// `install()`, restores the previous instance in `restore()`, and records + /// only `paint-scope:` lines so unrelated output stays out of the way. + private static final class CapturingLog extends Log { + final List messages = new ArrayList<>(); + private Log previous; + void install() { + previous = Log.getInstance(); + Log.install(this); + } + void restore() { + Log.install(previous); + } + @Override + protected void print(String text, int level) { + if (text != null && text.startsWith("paint-scope:")) { + messages.add(text); + } + } + } + + @Test + public void repeatedLeakLogsOncePerSignature() { + // Issue #5102: a component that leaks state every paint used to flood + // the EDT log -- one warning per repaint at ~60Hz. The dedup keeps the + // diagnostic value (first occurrence reported) while collapsing the + // tail of identical reports. + BufferedImage img = new BufferedImage(10, 10, BufferedImage.TYPE_INT_ARGB); + Graphics2D g = img.createGraphics(); + CapturingLog log = new CapturingLog(); + log.install(); + try { + // Class with a distinct fully-qualified name so this test's + // signature is unique even if other tests run in the same JVM and + // pollute the shared dedup set on JavaSEPort.instance. + class LeakySignatureA_5102 {} + class LeakySignatureB_5102 {} + Object ownerA = new LeakySignatureA_5102(); + Object ownerB = new LeakySignatureB_5102(); + + for (int i = 0; i < 5; i++) { + port().beginPaintScope(g, ownerA); + g.setColor(Color.MAGENTA); + port().endPaintScope(g, ownerA); + } + + int afterA = log.messages.size(); + assertEquals(1, afterA, + "repeating the same (owner-class, leak-kind) must log only once"); + + // A different owner class with the same leak kind is a new + // signature -- we still want one warning so devs see it. + port().beginPaintScope(g, ownerB); + g.setColor(Color.MAGENTA); + port().endPaintScope(g, ownerB); + assertEquals(2, log.messages.size(), + "a different owner class must produce its own warning"); + + // Even after dedup the auto-restore must keep working -- the log + // suppression must not gate the state-fix-up. + Color initialColor = g.getColor(); + port().beginPaintScope(g, ownerA); + g.setColor(Color.GREEN); + port().endPaintScope(g, ownerA); + assertEquals(initialColor, g.getColor(), + "auto-restore must run even when the warning is suppressed"); + } finally { + port().disposeGraphics(g); + g.dispose(); + log.restore(); + } + } + @Test public void nestedScopesLifoBalance() { BufferedImage img = new BufferedImage(10, 10, BufferedImage.TYPE_INT_ARGB);