diff --git a/.github/workflows/input-validation.yml b/.github/workflows/input-validation.yml index c929ac50df..117af6f1b7 100644 --- a/.github/workflows/input-validation.yml +++ b/.github/workflows/input-validation.yml @@ -82,24 +82,6 @@ jobs: id: setup_hash run: echo "hash=$(shasum -a 256 scripts/setup-workspace.sh | awk '{print $1}')" >> "$GITHUB_OUTPUT" - - name: Compute CN1 source hash - id: src_hash - run: | - set -euo pipefail - SRC_HASH=$(find CodenameOne/src Ports/iOSPort vm/JavaAPI vm/ByteCodeTranslator Themes native-themes \ - maven/codenameone-maven-plugin/src/main \ - -type f \( -name '*.java' -o -name '*.m' -o -name '*.h' -o -name '*.xml' -o -name '*.properties' -o -name '*.css' \) 2>/dev/null \ - | sort | xargs shasum -a 256 | shasum -a 256 | awk '{print $1}') - POM_HASH=$(find . -name 'pom.xml' -not -path './scripts/*' 2>/dev/null \ - | sort | xargs shasum -a 256 | shasum -a 256 | awk '{print $1}') - SCRIPT_HASH=$(shasum -a 256 \ - scripts/setup-workspace.sh \ - scripts/build-ios-port.sh \ - scripts/build-native-themes.sh \ - .github/workflows/_build-ios-port.yml \ - | shasum -a 256 | awk '{print $1}') - echo "hash=${SRC_HASH:0:16}-${POM_HASH:0:16}-${SCRIPT_HASH:0:16}" >> "$GITHUB_OUTPUT" - - name: Set TMPDIR run: echo "TMPDIR=${{ runner.temp }}" >> $GITHUB_ENV @@ -128,13 +110,18 @@ jobs: cn1-binaries-${{ runner.os }}- - name: Restore built CN1 + iOS port artifacts + # Reuse the exact cache key the build-port job saved against. Locally + # recomputing the hash here has drifted out of sync with + # _build-ios-port.yml before (e.g. svg-transcoder/src/main added on + # one side only), causing a permanent cache miss for any PR that + # touches a file matching this workflow's paths filter. uses: actions/cache/restore@v4 with: path: | ~/.m2/repository/com/codenameone Themes Ports/iOSPort/nativeSources - key: cn1-built-${{ runner.os }}-${{ steps.src_hash.outputs.hash }} + key: ${{ needs.build-port.outputs.cn1_built_cache_key }} fail-on-cache-miss: true - name: Install XcodeGen diff --git a/CodenameOne/src/com/codename1/impl/CodenameOneImplementation.java b/CodenameOne/src/com/codename1/impl/CodenameOneImplementation.java index ff48623f7d..450dfc4fd4 100644 --- a/CodenameOne/src/com/codename1/impl/CodenameOneImplementation.java +++ b/CodenameOne/src/com/codename1/impl/CodenameOneImplementation.java @@ -1963,6 +1963,18 @@ public void popClip(Object graphics) { } + /// Marks the start of a user-overrideable paint scope (e.g. `Component.paint`, + /// `Component.paintBackground`, `Painter.paint`, glass pane). Ports may use this + /// to snapshot graphics state and validate that the scope leaves it unchanged. + /// Default is a no-op so device ports pay zero cost. + public void beginPaintScope(Object graphics, Object owner) { + } + + /// Marks the end of a paint scope opened by `#beginPaintScope`. Must be paired + /// with a `beginPaintScope` call on the same graphics with the same owner. + public void endPaintScope(Object graphics, Object owner) { + } + /// Draws a line between the 2 X/Y coordinates /// /// #### Parameters diff --git a/CodenameOne/src/com/codename1/ui/Component.java b/CodenameOne/src/com/codename1/ui/Component.java index d25adfa0e1..1b2fbfb99a 100644 --- a/CodenameOne/src/com/codename1/ui/Component.java +++ b/CodenameOne/src/com/codename1/ui/Component.java @@ -3001,13 +3001,23 @@ void internalPaintImpl(Graphics g, boolean paintIntersects) { int scrollX = getScrollX(); int scrollY = getScrollY(); g.translate(-scrollX, -scrollY); - paint(g); + Display.impl.beginPaintScope(g.getGraphics(), this); + try { + paint(g); + } finally { + Display.impl.endPaintScope(g.getGraphics(), this); + } g.translate(scrollX, scrollY); if (isScrollVisible) { paintScrollbars(g); } } else { - paint(g); + Display.impl.beginPaintScope(g.getGraphics(), this); + try { + paint(g); + } finally { + Display.impl.endPaintScope(g.getGraphics(), this); + } } if (isBorderPainted()) { paintBorder(g); @@ -3395,9 +3405,19 @@ private void drawPaintersImpl(Graphics g, Component par, Component c, rect.getSize().setWidth(par.getWidth()); rect.getSize().setHeight(par.getHeight()); } - p.paint(g, rect); + Display.impl.beginPaintScope(g.getGraphics(), p); + try { + p.paint(g, rect); + } finally { + Display.impl.endPaintScope(g.getGraphics(), p); + } + } + Display.impl.beginPaintScope(g.getGraphics(), par); + try { + par.paintBackground(g); + } finally { + Display.impl.endPaintScope(g.getGraphics(), par); } - par.paintBackground(g); ((Container) par).paintIntersecting(g, c, x, y, w, h, false, 0); g.translate(-transX, -transY); } @@ -3469,9 +3489,20 @@ private void paintBackgroundImpl(Graphics g) { } } if (getStyle().getBgPainter() != null) { - getStyle().getBgPainter().paint(g, bounds); + Painter bp = getStyle().getBgPainter(); + Display.impl.beginPaintScope(g.getGraphics(), bp); + try { + bp.paint(g, bounds); + } finally { + Display.impl.endPaintScope(g.getGraphics(), bp); + } + } + Display.impl.beginPaintScope(g.getGraphics(), this); + try { + paintBackground(g); + } finally { + Display.impl.endPaintScope(g.getGraphics(), this); } - paintBackground(g); paintRippleEffect(g); } @@ -5086,7 +5117,12 @@ protected Image getDragImage() { g.translate(-getX(), -getY()); paintComponentBackground(g); - paint(g); + Display.impl.beginPaintScope(g.getGraphics(), this); + try { + paint(g); + } finally { + Display.impl.endPaintScope(g.getGraphics(), this); + } if (isBorderPainted()) { paintBorder(g); } @@ -5132,7 +5168,12 @@ public Image toImage() { g.translate(-getX(), -getY()); paintComponentBackground(g); - paint(g); + Display.impl.beginPaintScope(g.getGraphics(), this); + try { + paint(g); + } finally { + Display.impl.endPaintScope(g.getGraphics(), this); + } if (isBorderPainted()) { paintBorder(g); } diff --git a/CodenameOne/src/com/codename1/ui/Form.java b/CodenameOne/src/com/codename1/ui/Form.java index b15167bfdd..6f0b542e52 100644 --- a/CodenameOne/src/com/codename1/ui/Form.java +++ b/CodenameOne/src/com/codename1/ui/Form.java @@ -1123,10 +1123,20 @@ void paintGlassImpl(Graphics g) { int tx = g.getTranslateX(); int ty = g.getTranslateY(); g.translate(-tx, -ty); - glassPane.paint(g, getBounds()); + Display.impl.beginPaintScope(g.getGraphics(), glassPane); + try { + glassPane.paint(g, getBounds()); + } finally { + Display.impl.endPaintScope(g.getGraphics(), glassPane); + } g.translate(tx, ty); } - paintGlass(g); + Display.impl.beginPaintScope(g.getGraphics(), this); + try { + paintGlass(g); + } finally { + Display.impl.endPaintScope(g.getGraphics(), this); + } if (dragged != null && dragged.isDragAndDropInitialized()) { int[] c = g.getClip(); g.setClip(0, 0, getWidth(), getHeight()); diff --git a/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java b/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java index d944c2f660..0ab3e04a36 100644 --- a/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java +++ b/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java @@ -8812,32 +8812,231 @@ public void pushClip(Object graphics) { public void popClip(Object graphics) { checkEDT(); Graphics2D g2d = getGraphics(graphics); - + if ( graphics instanceof NativeScreenGraphics ){ NativeScreenGraphics g = (NativeScreenGraphics)graphics; + if (g.clipStack.isEmpty()) { + throw new IllegalStateException( + "popClip() called with no matching pushClip(). " + + "The clip stack is empty -- popping it would corrupt the graphics state. " + + "This is detected only by the simulator; on devices the same mistake can " + + "manifest as drift or crashes after many frames (see issue #5058)."); + } Shape oldClip = g.clipStack.pop(); - g2d.setClip(oldClip); } else { synchronized(clipStack) { - if (clipStack.containsKey(graphics)) { - Shape oldClip = clipStack.get(graphics).pop(); - if (oldClip != null) { - g2d.setClip(oldClip); - } + LinkedList stack = clipStack.get(graphics); + if (stack == null || stack.isEmpty()) { + throw new IllegalStateException( + "popClip() called with no matching pushClip(). " + + "The clip stack is empty -- popping it would corrupt the graphics state. " + + "This is detected only by the simulator; on devices the same mistake can " + + "manifest as drift or crashes after many frames (see issue #5058)."); + } + Shape oldClip = stack.pop(); + if (oldClip != null) { + g2d.setClip(oldClip); } } } - } private final Map> clipStack = new HashMap>(); - + @Override public void disposeGraphics(Object graphics) { synchronized(clipStack) { clipStack.remove(graphics); } + synchronized(paintScopes) { + paintScopes.remove(graphics); + } + } + + /// Snapshot of Graphics2D state taken when a user paint scope begins. + /// Used by the simulator to detect components/painters that leak state + /// (clip push without pop, dangling translate, alpha change, etc.). + private static final class PaintScopeSnapshot { + final Object owner; + final int clipStackDepth; + final Shape clip; + final AffineTransform transform; + final java.awt.Color color; + final java.awt.Font font; + final java.awt.Composite composite; + PaintScopeSnapshot(Object owner, int clipStackDepth, Shape clip, + AffineTransform transform, java.awt.Color color, + java.awt.Font font, java.awt.Composite composite) { + this.owner = owner; + this.clipStackDepth = clipStackDepth; + this.clip = clip; + this.transform = transform; + this.color = color; + this.font = font; + this.composite = composite; + } + } + + private final Map> paintScopes = + new HashMap>(); + + /// When true (default) the simulator validates that every user paint scope + /// (`Component.paint`, `Component.paintBackground`, `Painter.paint`, + /// `Form.paintGlass`, glass pane) leaves the Graphics in the same state it + /// found it. Set system property `cn1.disable.paint.scope.checks=true` to + /// turn the validation off (the begin/end hooks become no-ops). + private final boolean paintScopeChecksEnabled = + !Boolean.getBoolean("cn1.disable.paint.scope.checks"); + + private int clipStackDepth(Object graphics) { + if (graphics instanceof NativeScreenGraphics) { + return ((NativeScreenGraphics) graphics).clipStack.size(); + } + synchronized (clipStack) { + LinkedList stack = clipStack.get(graphics); + return stack == null ? 0 : stack.size(); + } + } + + private void trimClipStack(Object graphics, int targetDepth) { + if (graphics instanceof NativeScreenGraphics) { + LinkedList stack = ((NativeScreenGraphics) graphics).clipStack; + while (stack.size() > targetDepth) { + stack.pop(); + } + } else { + synchronized (clipStack) { + LinkedList stack = clipStack.get(graphics); + if (stack != null) { + while (stack.size() > targetDepth) { + stack.pop(); + } + } + } + } + } + + @Override + public void beginPaintScope(Object graphics, Object owner) { + if (!paintScopeChecksEnabled) { + return; + } + Graphics2D g2d = getGraphics(graphics); + PaintScopeSnapshot snap = new PaintScopeSnapshot( + owner, + clipStackDepth(graphics), + g2d.getClip(), + new AffineTransform(g2d.getTransform()), + g2d.getColor(), + g2d.getFont(), + g2d.getComposite()); + synchronized (paintScopes) { + LinkedList stack = paintScopes.get(graphics); + if (stack == null) { + stack = new LinkedList(); + paintScopes.put(graphics, stack); + } + stack.push(snap); + } + } + + @Override + public void endPaintScope(Object graphics, Object owner) { + if (!paintScopeChecksEnabled) { + return; + } + PaintScopeSnapshot snap; + synchronized (paintScopes) { + LinkedList stack = paintScopes.get(graphics); + if (stack == null || stack.isEmpty()) { + Log.p("paint-scope: endPaintScope without matching begin for " + + describe(owner)); + return; + } + snap = stack.pop(); + } + if (snap.owner != owner) { + Log.p("paint-scope: mismatched begin/end (begin=" + describe(snap.owner) + + ", end=" + describe(owner) + ")"); + } + Graphics2D g2d = getGraphics(graphics); + StringBuilder diffs = null; + + int depthNow = clipStackDepth(graphics); + if (depthNow != snap.clipStackDepth) { + diffs = appendDiff(diffs, "clip stack depth " + snap.clipStackDepth + + " -> " + depthNow + " (unmatched pushClip/popClip)"); + trimClipStack(graphics, snap.clipStackDepth); + } + // Transform must be restored before the clip comparison: getClip() + // returns the clip in current user-space, so a different transform + // makes the same device-space clip look different. + if (!g2d.getTransform().equals(snap.transform)) { + diffs = appendDiff(diffs, "transform not restored " + snap.transform + + " -> " + g2d.getTransform()); + g2d.setTransform(snap.transform); + } + if (!sameShape(g2d.getClip(), snap.clip)) { + diffs = appendDiff(diffs, "clip rect not restored"); + g2d.setClip(snap.clip); + } + if (!equalsNullSafe(g2d.getColor(), snap.color)) { + diffs = appendDiff(diffs, "color not restored " + snap.color + + " -> " + g2d.getColor()); + g2d.setColor(snap.color); + } + if (!equalsNullSafe(g2d.getFont(), snap.font)) { + diffs = appendDiff(diffs, "font not restored"); + g2d.setFont(snap.font); + } + if (!equalsNullSafe(g2d.getComposite(), snap.composite)) { + diffs = appendDiff(diffs, "composite/alpha not restored"); + 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)."); + } + } + + private static StringBuilder appendDiff(StringBuilder b, String diff) { + if (b == null) { + return new StringBuilder(diff); + } + return b.append("; ").append(diff); + } + + private static boolean equalsNullSafe(Object a, Object b) { + return a == null ? b == null : a.equals(b); + } + + private static boolean sameShape(Shape a, Shape b) { + if (a == b) { + return true; + } + if (a == null || b == null) { + return false; + } + java.awt.Rectangle ra = a.getBounds(); + java.awt.Rectangle rb = b.getBounds(); + return ra.equals(rb); + } + + private static String describe(Object owner) { + if (owner == null) { + return ""; + } + if (owner instanceof com.codename1.ui.Component) { + com.codename1.ui.Component c = (com.codename1.ui.Component) owner; + String name = c.getName(); + return name != null ? c.getClass().getName() + "[" + name + "]" + : c.getClass().getName(); + } + return owner.getClass().getName(); } 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 new file mode 100644 index 0000000000..b5330f8e26 --- /dev/null +++ b/maven/javase/src/test/java/com/codename1/impl/javase/PaintScopeTest.java @@ -0,0 +1,176 @@ +/* + * Copyright (c) 2026, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. + */ +package com.codename1.impl.javase; + +import com.codename1.testing.junit.CodenameOneTest; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledIfSystemProperty; + +import java.awt.Color; +import java.awt.Graphics2D; +import java.awt.geom.AffineTransform; +import java.awt.image.BufferedImage; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/// Verifies the simulator's strict paint-scope checks added for issue #5058: +/// excess `popClip` must throw, and a paint scope that leaks Graphics state +/// must auto-restore the state at end-of-scope. +@CodenameOneTest +@DisabledIfSystemProperty(named = "java.awt.headless", matches = "true") +public class PaintScopeTest { + + private JavaSEPort port() { + return JavaSEPort.instance; + } + + @Test + public void excessPopClipThrows() { + BufferedImage img = new BufferedImage(10, 10, BufferedImage.TYPE_INT_ARGB); + Graphics2D g = img.createGraphics(); + try { + assertThrows(IllegalStateException.class, () -> port().popClip(g)); + } finally { + g.dispose(); + } + } + + @Test + public void overPopAfterMatchingPushThrows() { + BufferedImage img = new BufferedImage(10, 10, BufferedImage.TYPE_INT_ARGB); + Graphics2D g = img.createGraphics(); + try { + port().pushClip(g); + port().popClip(g); + // Second pop has no matching push. + assertThrows(IllegalStateException.class, () -> port().popClip(g)); + } finally { + port().disposeGraphics(g); + g.dispose(); + } + } + + @Test + public void scopeAutoRestoresClipStackDepth() { + BufferedImage img = new BufferedImage(10, 10, BufferedImage.TYPE_INT_ARGB); + Graphics2D g = img.createGraphics(); + try { + Object owner = new Object(); + port().beginPaintScope(g, owner); + port().pushClip(g); + port().pushClip(g); + // Deliberately forget to pop. + port().endPaintScope(g, owner); + // Auto-restore must drain the stack so the next pop fails cleanly + // rather than corrupting subsequent paints (the issue #5058 leak). + assertThrows(IllegalStateException.class, () -> port().popClip(g)); + } finally { + port().disposeGraphics(g); + g.dispose(); + } + } + + @Test + public void scopeAutoRestoresTransformAndColor() { + BufferedImage img = new BufferedImage(10, 10, BufferedImage.TYPE_INT_ARGB); + Graphics2D g = img.createGraphics(); + try { + AffineTransform initialTx = new AffineTransform(g.getTransform()); + Color initialColor = g.getColor(); + + Object owner = new Object(); + port().beginPaintScope(g, owner); + g.translate(50, 50); + g.setColor(Color.MAGENTA); + port().endPaintScope(g, owner); + + assertEquals(initialTx, g.getTransform(), + "transform must be restored to its pre-scope value"); + assertEquals(initialColor, g.getColor(), + "color must be restored to its pre-scope value"); + } finally { + port().disposeGraphics(g); + g.dispose(); + } + } + + @Test + public void cleanScopeProducesNoChange() { + BufferedImage img = new BufferedImage(10, 10, BufferedImage.TYPE_INT_ARGB); + Graphics2D g = img.createGraphics(); + try { + AffineTransform initialTx = new AffineTransform(g.getTransform()); + Color initialColor = g.getColor(); + + Object owner = new Object(); + port().beginPaintScope(g, owner); + // Mimic a well-behaved paint: push, draw, pop -- all balanced. + port().pushClip(g); + g.translate(10, 10); + g.setColor(Color.RED); + g.translate(-10, -10); + g.setColor(initialColor); + port().popClip(g); + port().endPaintScope(g, owner); + + assertEquals(initialTx, g.getTransform()); + assertEquals(initialColor, g.getColor()); + // A balanced scope leaves the stack empty -- a follow-up pop must + // still throw to flag any *new* unbalanced caller. + assertThrows(IllegalStateException.class, () -> port().popClip(g)); + } finally { + port().disposeGraphics(g); + g.dispose(); + } + } + + @Test + public void nestedScopesLifoBalance() { + BufferedImage img = new BufferedImage(10, 10, BufferedImage.TYPE_INT_ARGB); + Graphics2D g = img.createGraphics(); + try { + Object outer = new Object(); + Object inner = new Object(); + port().beginPaintScope(g, outer); + g.translate(5, 5); + port().beginPaintScope(g, inner); + g.translate(7, 7); + // Inner forgets to undo its translate. + port().endPaintScope(g, inner); + // After inner, translate should be back to (5, 5). + assertTrue(g.getTransform().getTranslateX() == 5.0 + && g.getTransform().getTranslateY() == 5.0, + "inner scope auto-restore should leave outer translate intact"); + port().endPaintScope(g, outer); + // After outer, transform should be identity again. + assertTrue(g.getTransform().isIdentity(), + "outer scope auto-restore should reach the original identity"); + } finally { + port().disposeGraphics(g); + g.dispose(); + } + } +}