This is an automated email from the ASF dual-hosted git repository.
mattcasters pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/hop.git
The following commit(s) were added to refs/heads/main by this push:
new 76f36991d8 harden getting the session display, fixes #8248 (#8271)
76f36991d8 is described below
commit 76f36991d82759a0d933571be31bcdbcb04b4277
Author: Hans Van Akelyen <[email protected]>
AuthorDate: Sat Sep 5 22:07:37 2026 +0200
harden getting the session display, fixes #8248 (#8271)
minor improvements
---
.../apache/hop/lint/LintCanvasOverlayRefresh.java | 3 +-
.../apache/hop/lint/LintProblemsBarManager.java | 13 +-
.../org/apache/hop/lint/LintStatusFilePainter.java | 221 ++++++++++++++++-----
.../hop/lint/LintStatusBadgeCompositeTest.java | 155 +++++++++++++++
.../apache/hop/testing/gui/TestingGuiPlugin.java | 6 +-
.../hop/testing/gui/TestingGuiPluginTweakTest.java | 34 ++++
.../hop/ui/hopgui/BackgroundThreadFacadeImpl.java | 13 +-
.../ui/hopgui/BackgroundThreadFacadeImplTest.java | 27 ++-
.../org/apache/hop/core/SwtUniversalImage.java | 2 +-
.../org/apache/hop/ui/core/gui/HopNamespace.java | 5 +-
.../org/apache/hop/ui/core/widget/TreeMemory.java | 3 +-
.../org/apache/hop/ui/hopgui/SessionDisplay.java | 77 +++++++
.../hop/ui/hopgui/shared/SashFormMemory.java | 3 +-
.../apache/hop/ui/hopgui/SessionDisplayTest.java | 88 ++++++++
14 files changed, 587 insertions(+), 63 deletions(-)
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintCanvasOverlayRefresh.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintCanvasOverlayRefresh.java
index d54b1e7439..d423ffcf8e 100644
---
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintCanvasOverlayRefresh.java
+++
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintCanvasOverlayRefresh.java
@@ -20,6 +20,7 @@ import java.util.Collections;
import java.util.Set;
import java.util.WeakHashMap;
import org.apache.hop.ui.hopgui.HopGui;
+import org.apache.hop.ui.hopgui.SessionDisplay;
import org.apache.hop.ui.hopgui.file.IHopFileTypeHandler;
import org.apache.hop.ui.hopgui.file.shared.HopGuiAbstractGraph;
import org.apache.hop.ui.hopgui.perspective.TabItemHandler;
@@ -68,7 +69,7 @@ public final class LintCanvasOverlayRefresh {
display = hopGui.getShell().getDisplay();
}
if (display == null) {
- display = Display.getCurrent();
+ display = SessionDisplay.current();
}
if (display == null || display.isDisposed()) {
return;
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintProblemsBarManager.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintProblemsBarManager.java
index 640b4012be..90409d01b5 100644
---
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintProblemsBarManager.java
+++
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintProblemsBarManager.java
@@ -22,6 +22,7 @@ import java.util.concurrent.ConcurrentHashMap;
import org.apache.hop.core.logging.ILogChannel;
import org.apache.hop.core.logging.LogChannel;
import org.apache.hop.ui.hopgui.HopGui;
+import org.apache.hop.ui.hopgui.SessionDisplay;
import org.apache.hop.ui.hopgui.file.shared.HopGuiAbstractGraph;
import org.eclipse.swt.widgets.Display;
@@ -138,7 +139,7 @@ public class LintProblemsBarManager {
}
// This touches SWT widgets, so make sure it runs on the UI thread
regardless of which
// thread the caller is on (background lint threads call this too).
- if (Display.getCurrent() == null) {
+ if (SessionDisplay.current() == null) {
Display display = sessionDisplay();
if (display == null || display.isDisposed()) {
log.logDetailed(
@@ -174,7 +175,7 @@ public class LintProblemsBarManager {
"Gave up syncing the Problems tab for " + filePath + ": no editor
found in time");
return;
}
- Display display = Display.getCurrent();
+ Display display = SessionDisplay.current();
if (display == null || display.isDisposed()) {
return;
}
@@ -184,9 +185,9 @@ public class LintProblemsBarManager {
/**
* The display to get onto the UI thread with.
*
- * <p>An editor we already know about answers first: {@code
Display.getDefault()} only knows the
- * session bound to the calling thread, which is nothing at all on a thread
that was started
- * without one - and in Hop Web the wrong session's display would be worse
than none.
+ * <p>An editor we already know about answers first: the default display
only knows the session
+ * bound to the calling thread, which is nothing at all on a thread that was
started without one -
+ * and in Hop Web the wrong session's display would be worse than none.
*/
private Display sessionDisplay() {
for (HopGuiAbstractGraph graph : graphsById.values()) {
@@ -197,7 +198,7 @@ public class LintProblemsBarManager {
}
}
}
- return Display.getDefault();
+ return SessionDisplay.currentOrDefault();
}
public void refreshAllOpenEditors() {
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintStatusFilePainter.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintStatusFilePainter.java
index 60043a9839..e018463d2c 100644
---
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintStatusFilePainter.java
+++
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintStatusFilePainter.java
@@ -20,15 +20,20 @@ import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.TimeUnit;
+import org.apache.hop.core.SwtUniversalImage;
import org.apache.hop.core.logging.ILogChannel;
import org.apache.hop.core.logging.LogChannel;
import org.apache.hop.core.util.Utils;
import org.apache.hop.ui.core.gui.GuiResource;
import org.apache.hop.ui.hopgui.HopGui;
+import org.apache.hop.ui.hopgui.SessionDisplay;
import org.apache.hop.ui.hopgui.perspective.explorer.ExplorerPerspective;
import
org.apache.hop.ui.hopgui.perspective.explorer.IExplorerFilePaintListener;
import org.eclipse.swt.SWT;
import org.eclipse.swt.graphics.Image;
+import org.eclipse.swt.graphics.ImageData;
+import org.eclipse.swt.graphics.PaletteData;
+import org.eclipse.swt.graphics.RGB;
import org.eclipse.swt.widgets.Display;
import org.eclipse.swt.widgets.Tree;
import org.eclipse.swt.widgets.TreeItem;
@@ -60,8 +65,8 @@ public class LintStatusFilePainter implements
IExplorerFilePaintListener {
/**
* The display of the GUI this painter belongs to, read when it is built on
the UI thread.
*
- * <p>{@code Display.getDefault()} answers for the session bound to the
calling thread, and the
- * results this repaints for arrive on lint threads that may have none.
+ * <p>The default display answers for the session bound to the calling
thread, and the results
+ * this repaints for arrive on lint threads that may have none.
*/
private final Display display;
@@ -77,7 +82,7 @@ public class LintStatusFilePainter implements
IExplorerFilePaintListener {
}
public LintStatusFilePainter() {
- this.display = Display.getCurrent() != null ? Display.getCurrent() :
Display.getDefault();
+ this.display = SessionDisplay.currentOrDefault();
updateFileStatusCache();
LintResultsManager.getInstance()
@@ -99,8 +104,11 @@ public class LintStatusFilePainter implements
IExplorerFilePaintListener {
* do: RWT only draws on a control, so the very first call failed and the
Explorer showed no lint
* status at all. Hop ships the three icons as SVG, and {@link GuiResource}
loads and caches them
* per session, which also settles who disposes them - not us.
+ *
+ * <p>Asked for at the size it will occupy, so the SVG is rasterized at that
size rather than a
+ * bigger bitmap being resampled down into it - resampling is what costs an
icon its edges.
*/
- private Image badgeIcon(LintStatus status) {
+ private Image badgeIcon(LintStatus status, int size) {
String location =
switch (status) {
case ERROR -> "ui/images/error.svg";
@@ -112,7 +120,7 @@ public class LintStatusFilePainter implements
IExplorerFilePaintListener {
return null;
}
try {
- return GuiResource.getInstance().getImage(location, BADGE_SIZE,
BADGE_SIZE);
+ return GuiResource.getInstance().getImage(location, size, size);
} catch (Exception e) {
log.logDetailed("No lint status icon available for " + status + ": " +
e.getMessage());
return null;
@@ -181,21 +189,17 @@ public class LintStatusFilePainter implements
IExplorerFilePaintListener {
}
}
- Image badge = badgeIcon(status);
- if (badge == null || badge.isDisposed()) {
- return;
- }
switch (status) {
case ERROR:
- addOverlayIcon(treeItem, badge, status);
+ addOverlayIcon(treeItem, status);
addLintTooltip(treeItem, name, "Linter errors");
break;
case WARNING:
- addOverlayIcon(treeItem, badge, status);
+ addOverlayIcon(treeItem, status);
addLintTooltip(treeItem, name, "Linter warnings");
break;
case CLEAN:
- addOverlayIcon(treeItem, badge, status);
+ addOverlayIcon(treeItem, status);
addLintTooltip(treeItem, name, "No linter issues");
break;
default:
@@ -263,7 +267,7 @@ public class LintStatusFilePainter implements
IExplorerFilePaintListener {
return LintStatus.UNKNOWN;
}
- private void addOverlayIcon(TreeItem treeItem, Image lintIcon, LintStatus
status) {
+ private void addOverlayIcon(TreeItem treeItem, LintStatus status) {
try {
// Already showing this exact status for this item -> nothing to do
(avoids re-compositing
// on every paint, which previously leaked a new Image each time and
crashed the GUI).
@@ -282,22 +286,31 @@ public class LintStatusFilePainter implements
IExplorerFilePaintListener {
}
}
- if (base == null || base.isDisposed()) {
+ boolean noBase = base == null || base.isDisposed();
+ org.eclipse.swt.graphics.Rectangle bounds = noBase ? null :
base.getBounds();
+ // Rasterize the badge at the size it will occupy on this icon, so
nothing is resampled.
+ Image lintIcon =
+ badgeIcon(
+ status, bounds == null ? BADGE_SIZE : badgeSizeFor(bounds.width,
bounds.height));
+ if (lintIcon == null || lintIcon.isDisposed()) {
+ return;
+ }
+
+ if (bounds == null) {
treeItem.setImage(lintIcon);
treeItem.setData(APPLIED_STATUS_KEY, status);
return;
}
- org.eclipse.swt.graphics.Rectangle bounds = base.getBounds();
if (bounds.width > 100 || bounds.height > 100) {
treeItem.setImage(lintIcon);
treeItem.setData(APPLIED_STATUS_KEY, status);
return;
}
- // No composite (Hop Web cannot draw one): leave the file's own icon
alone. The item's
- // colour already says what the status is, and replacing the icon with a
bare badge would
- // cost more than it tells.
+ // Without a composite, leave the file's own icon alone. The item's
colour already says
+ // what the status is, and replacing the icon with a bare badge would
cost more than it
+ // tells.
Image compositeIcon = getOrCreateComposite(base, lintIcon);
if (compositeIcon != null) {
treeItem.setImage(compositeIcon);
@@ -305,7 +318,11 @@ public class LintStatusFilePainter implements
IExplorerFilePaintListener {
treeItem.setData(APPLIED_STATUS_KEY, status);
} catch (Exception e) {
log.logError("Error creating overlay icon: " + e.getMessage(), e);
- treeItem.setImage(lintIcon);
+ // The bare badge still says what the status is, which beats leaving the
item unmarked.
+ Image fallback = badgeIcon(status, BADGE_SIZE);
+ if (fallback != null && !fallback.isDisposed()) {
+ treeItem.setImage(fallback);
+ }
}
}
@@ -323,45 +340,157 @@ public class LintStatusFilePainter implements
IExplorerFilePaintListener {
return composite;
}
+ /**
+ * The file's own icon with the lint badge in its bottom right corner.
+ *
+ * <p>Composited pixel by pixel rather than with a {@code GC}. RWT resolves
both the device and
+ * the drawing delegate of a {@code GC} from its drawable and understands
only a Control or a
+ * Device, so a {@code GC} on an Image is left with neither: the drawing
fails with a
+ * NullPointerException, and disposing it then fails with "A factory-created
resource cannot be
+ * disposed", which is the exception that reaches the log. Working on the
{@link ImageData} of
+ * both icons needs no drawing surface at all, so Hop Web gets the same
badges as the desktop.
+ */
private Image createCompositeIcon(Image originalIcon, Image lintIcon) {
try {
if (display == null || display.isDisposed()) {
return null;
}
-
- org.eclipse.swt.graphics.Rectangle originalBounds =
originalIcon.getBounds();
- org.eclipse.swt.graphics.Rectangle lintBounds = lintIcon.getBounds();
-
- Image composite = new Image(display, originalBounds.width,
originalBounds.height);
- org.eclipse.swt.graphics.GC gc = new
org.eclipse.swt.graphics.GC(composite);
- try {
- gc.setBackground(display.getSystemColor(SWT.COLOR_WIDGET_BACKGROUND));
- gc.fillRectangle(0, 0, originalBounds.width, originalBounds.height);
- gc.drawImage(originalIcon, 0, 0);
-
- int badgeSize = Math.min(10, Math.min(originalBounds.width / 2,
originalBounds.height / 2));
- int lintX = originalBounds.width - badgeSize - 1;
- int lintY = originalBounds.height - badgeSize - 1;
- gc.drawImage(
- lintIcon,
- 0,
- 0,
- lintBounds.width,
- lintBounds.height,
- lintX,
- lintY,
- badgeSize,
- badgeSize);
- return composite;
- } finally {
- gc.dispose();
+ ImageData baseData = SwtUniversalImage.getImageDataAtZoom(originalIcon,
100);
+ if (baseData == null) {
+ return null;
}
+ int badgeSize = badgeSizeFor(baseData.width, baseData.height);
+ if (badgeSize <= 0) {
+ return null;
+ }
+ // Composited again for every zoom the platform asks for, out of what
both icons themselves
+ // have at that zoom. Handing over the 100% pixels alone and letting SWT
raster-scale them up
+ // is what leaves icons blurry on a HiDPI screen - the very thing
createDpiAwareImage exists
+ // to avoid.
+ return SwtUniversalImage.createDpiAwareImage(
+ display,
+ zoom ->
+ withBadge(
+ SwtUniversalImage.getImageDataAtZoom(originalIcon, zoom),
+ SwtUniversalImage.getImageDataAtZoom(lintIcon, zoom),
+ SwtUniversalImage.pixelSize(badgeSize, zoom),
+ SwtUniversalImage.pixelSize(1, zoom)));
} catch (Exception e) {
log.logError("Error creating composite icon: " + e.getMessage(), e);
return null;
}
}
+ /** A corner mark on an icon this size: half its width at most, and never
more than 10px. */
+ private static int badgeSizeFor(int width, int height) {
+ return Math.min(10, Math.min(width / 2, height / 2));
+ }
+
+ /**
+ * The base icon with the badge scaled into its bottom right corner, blended
over whatever the
+ * base has there rather than punched through it, so a badge with soft edges
does not leave a hard
+ * outline. What the base leaves transparent stays transparent: the tree
paints its own background
+ * behind the icon.
+ *
+ * <p>Sizes are in the pixels of the icons handed in, so that the same badge
lands in the same
+ * place whichever zoom these pixels came from.
+ */
+ static ImageData withBadge(ImageData baseData, ImageData badgeData, int
badgeSize, int margin) {
+ ImageData composite = withPerPixelAlpha(baseData);
+ ImageData scaled = badgeData.scaledTo(badgeSize, badgeSize);
+ ImageData scaledMask = transparencyMask(scaled);
+ int offsetX = composite.width - badgeSize - margin;
+ int offsetY = composite.height - badgeSize - margin;
+
+ for (int y = 0; y < badgeSize; y++) {
+ int targetY = offsetY + y;
+ if (targetY < 0 || targetY >= composite.height) {
+ continue;
+ }
+ for (int x = 0; x < badgeSize; x++) {
+ int targetX = offsetX + x;
+ if (targetX < 0 || targetX >= composite.width) {
+ continue;
+ }
+ int overAlpha = alphaAt(scaled, scaledMask, x, y);
+ if (overAlpha == 0) {
+ continue;
+ }
+ RGB over = scaled.palette.getRGB(scaled.getPixel(x, y));
+ if (overAlpha == 255) {
+ composite.setPixel(targetX, targetY,
composite.palette.getPixel(over));
+ composite.setAlpha(targetX, targetY, 255);
+ continue;
+ }
+ RGB under = composite.palette.getRGB(composite.getPixel(targetX,
targetY));
+ int underAlpha = composite.getAlpha(targetX, targetY);
+ int outAlpha = overAlpha + underAlpha * (255 - overAlpha) / 255;
+ composite.setPixel(
+ targetX,
+ targetY,
+ composite.palette.getPixel(blend(over, under, overAlpha,
underAlpha)));
+ composite.setAlpha(targetX, targetY, outAlpha);
+ }
+ }
+ return composite;
+ }
+
+ /**
+ * The same picture in the one shape we can composite into: direct colour
with an alpha value per
+ * pixel. An icon can express its transparency in any of several ways and
only this one can be
+ * written back to, so the base is read through {@link #alphaAt} and
rewritten as this.
+ */
+ private static ImageData withPerPixelAlpha(ImageData source) {
+ ImageData copy =
+ new ImageData(source.width, source.height, 24, new
PaletteData(0xFF0000, 0xFF00, 0xFF));
+ copy.alphaData = new byte[source.width * source.height];
+ ImageData mask = transparencyMask(source);
+ for (int y = 0; y < source.height; y++) {
+ for (int x = 0; x < source.width; x++) {
+ RGB rgb = source.palette.getRGB(source.getPixel(x, y));
+ copy.setPixel(x, y, copy.palette.getPixel(rgb));
+ copy.setAlpha(x, y, alphaAt(source, mask, x, y));
+ }
+ }
+ return copy;
+ }
+
+ /** The 1-bit mask of an icon that carries one (ICO, BMP), or null - read
once, not per pixel. */
+ private static ImageData transparencyMask(ImageData data) {
+ return data.maskData == null ? null : data.getTransparencyMask();
+ }
+
+ /** How opaque one pixel is, whichever of the four ways the icon says so. */
+ private static int alphaAt(ImageData data, ImageData mask, int x, int y) {
+ if (mask != null && mask.getPixel(x, y) == 0) {
+ return 0;
+ }
+ if (data.transparentPixel != -1 && data.getPixel(x, y) ==
data.transparentPixel) {
+ return 0;
+ }
+ if (data.alphaData != null) {
+ return data.getAlpha(x, y);
+ }
+ return data.alpha == -1 ? 255 : data.alpha;
+ }
+
+ /** Source-over: the colour left when {@code over} is laid on {@code under}.
*/
+ private static RGB blend(RGB over, RGB under, int overAlpha, int underAlpha)
{
+ return new RGB(
+ channel(over.red, under.red, overAlpha, underAlpha),
+ channel(over.green, under.green, overAlpha, underAlpha),
+ channel(over.blue, under.blue, overAlpha, underAlpha));
+ }
+
+ private static int channel(int over, int under, int overAlpha, int
underAlpha) {
+ int outAlpha = overAlpha + underAlpha * (255 - overAlpha) / 255;
+ if (outAlpha == 0) {
+ return 0;
+ }
+ int weighted = over * overAlpha * 255 + under * underAlpha * (255 -
overAlpha);
+ return Math.min(255, weighted / (outAlpha * 255));
+ }
+
private void addLintTooltip(TreeItem treeItem, String fileName, String
lintStatus) {
try {
treeItem.setData("lintTooltip", fileName + " - " + lintStatus);
diff --git
a/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintStatusBadgeCompositeTest.java
b/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintStatusBadgeCompositeTest.java
new file mode 100644
index 0000000000..d25fb1290a
--- /dev/null
+++
b/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintStatusBadgeCompositeTest.java
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hop.lint;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import org.eclipse.swt.graphics.ImageData;
+import org.eclipse.swt.graphics.PaletteData;
+import org.eclipse.swt.graphics.RGB;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+
+/**
+ * The Explorer badges a file's own icon with its lint status. RWT cannot draw
onto an Image - a
+ * {@code GC} built on one has neither a device nor a drawing delegate - so
the badge is composited
+ * from the {@link ImageData} of both icons, which needs no drawing surface
and works the same on
+ * the desktop and in Hop Web.
+ */
+class LintStatusBadgeCompositeTest {
+
+ private static final RGB BASE_COLOR = new RGB(10, 20, 30);
+ private static final RGB BADGE_COLOR = new RGB(200, 100, 50);
+
+ /** A 16x16 icon of one colour, opaque, with an alpha channel - what an SVG
icon loads as. */
+ private static ImageData icon(int size, RGB color, int alpha) {
+ ImageData data = new ImageData(size, size, 24, new PaletteData(0xFF0000,
0xFF00, 0xFF));
+ data.alphaData = new byte[size * size];
+ for (int y = 0; y < size; y++) {
+ for (int x = 0; x < size; x++) {
+ data.setPixel(x, y, data.palette.getPixel(color));
+ data.setAlpha(x, y, alpha);
+ }
+ }
+ return data;
+ }
+
+ private static RGB colorAt(ImageData data, int x, int y) {
+ return data.palette.getRGB(data.getPixel(x, y));
+ }
+
+ @Test
+ @DisplayName("the badge lands in the bottom right corner and leaves the rest
of the icon alone")
+ void badgeIsCompositedIntoTheCorner() {
+ ImageData composite =
+ LintStatusFilePainter.withBadge(icon(16, BASE_COLOR, 255), icon(8,
BADGE_COLOR, 255), 8, 1);
+
+ assertEquals(16, composite.width);
+ assertEquals(16, composite.height);
+ // One pixel of margin is kept, so the very last column and row stay the
base icon.
+ assertEquals(BADGE_COLOR, colorAt(composite, 14, 14));
+ assertEquals(BASE_COLOR, colorAt(composite, 15, 15));
+ assertEquals(BASE_COLOR, colorAt(composite, 0, 0));
+ assertEquals(BASE_COLOR, colorAt(composite, 6, 6));
+ }
+
+ @Test
+ @DisplayName("what the badge leaves transparent shows the icon underneath")
+ void fullyTransparentBadgePixelsChangeNothing() {
+ ImageData composite =
+ LintStatusFilePainter.withBadge(icon(16, BASE_COLOR, 255), icon(8,
BADGE_COLOR, 0), 8, 1);
+
+ assertEquals(BASE_COLOR, colorAt(composite, 14, 14));
+ assertEquals(255, composite.getAlpha(14, 14));
+ }
+
+ @Test
+ @DisplayName("what the icon leaves transparent stays transparent")
+ void theIconsOwnTransparencySurvives() {
+ ImageData composite =
+ LintStatusFilePainter.withBadge(icon(16, BASE_COLOR, 0), icon(8,
BADGE_COLOR, 255), 8, 1);
+
+ assertEquals(0, composite.getAlpha(0, 0));
+ // Where the badge is opaque it wins outright, transparent base or not.
+ assertEquals(255, composite.getAlpha(14, 14));
+ assertEquals(BADGE_COLOR, colorAt(composite, 14, 14));
+ }
+
+ /** Palette icons say "transparent" with a pixel value rather than an alpha
channel. */
+ @Test
+ @DisplayName("a palette icon's transparent pixel is read as transparent, not
as its colour")
+ void transparentPixelIsHonoured() {
+ PaletteData palette = new PaletteData(BASE_COLOR, BADGE_COLOR);
+ ImageData base = new ImageData(16, 16, 8, palette);
+ base.transparentPixel = 1;
+ for (int y = 0; y < 16; y++) {
+ for (int x = 0; x < 16; x++) {
+ base.setPixel(x, y, x == 0 && y == 0 ? 1 : 0);
+ }
+ }
+
+ ImageData composite = LintStatusFilePainter.withBadge(base, icon(8,
BADGE_COLOR, 255), 8, 1);
+
+ assertEquals(0, composite.getAlpha(0, 0));
+ assertEquals(255, composite.getAlpha(1, 1));
+ assertEquals(BASE_COLOR, colorAt(composite, 1, 1));
+ }
+
+ /**
+ * The platform asks for the composite again at each zoom it paints at, so
the same badge is built
+ * from bigger pixels rather than by scaling up the 100% one - that is what
keeps a HiDPI icon
+ * sharp. Everything scales together, the margin included, so the badge
lands in the same place on
+ * the icon at every zoom.
+ */
+ @Test
+ @DisplayName("at 200% everything is twice the size and the badge sits in the
same place")
+ void geometryScalesWithTheZoom() {
+ ImageData at100 =
+ LintStatusFilePainter.withBadge(icon(16, BASE_COLOR, 255), icon(8,
BADGE_COLOR, 255), 8, 1);
+ ImageData at200 =
+ LintStatusFilePainter.withBadge(
+ icon(32, BASE_COLOR, 255), icon(16, BADGE_COLOR, 255), 16, 2);
+
+ assertEquals(32, at200.width);
+ // The badge corner of the 100% icon, at twice the scale, is still the
badge corner.
+ assertEquals(BADGE_COLOR, colorAt(at100, 7, 7));
+ assertEquals(BADGE_COLOR, colorAt(at200, 14, 14));
+ // And so is the last pixel before the margin.
+ assertEquals(BADGE_COLOR, colorAt(at100, 14, 14));
+ assertEquals(BADGE_COLOR, colorAt(at200, 29, 29));
+ // The margin itself stays the base icon at both zooms.
+ assertEquals(BASE_COLOR, colorAt(at100, 15, 15));
+ assertEquals(BASE_COLOR, colorAt(at200, 30, 30));
+ assertEquals(BASE_COLOR, colorAt(at200, 31, 31));
+ }
+
+ @Test
+ @DisplayName("a half transparent badge blends with the icon rather than
replacing it")
+ void partialAlphaBlends() {
+ ImageData composite =
+ LintStatusFilePainter.withBadge(
+ icon(16, new RGB(0, 0, 0), 255), icon(8, new RGB(255, 255, 255),
128), 8, 1);
+
+ // Half of white over black, opaque either way.
+ RGB blended = colorAt(composite, 14, 14);
+ assertEquals(128, blended.red);
+ assertEquals(128, blended.green);
+ assertEquals(128, blended.blue);
+ assertEquals(255, composite.getAlpha(14, 14));
+ }
+}
diff --git
a/plugins/misc/testing/src/main/java/org/apache/hop/testing/gui/TestingGuiPlugin.java
b/plugins/misc/testing/src/main/java/org/apache/hop/testing/gui/TestingGuiPlugin.java
index 61437fb05d..c8d8a3b534 100644
---
a/plugins/misc/testing/src/main/java/org/apache/hop/testing/gui/TestingGuiPlugin.java
+++
b/plugins/misc/testing/src/main/java/org/apache/hop/testing/gui/TestingGuiPlugin.java
@@ -78,6 +78,7 @@ import org.apache.hop.ui.core.widget.ColumnInfo;
import org.apache.hop.ui.core.widget.ComboFilterPopup;
import org.apache.hop.ui.core.widget.TableView;
import org.apache.hop.ui.hopgui.HopGui;
+import org.apache.hop.ui.hopgui.SessionDisplay;
import org.apache.hop.ui.hopgui.file.IHopFileTypeHandler;
import org.apache.hop.ui.hopgui.file.pipeline.HopGuiPipelineGraph;
import org.apache.hop.ui.hopgui.file.pipeline.context.HopGuiPipelineContext;
@@ -92,7 +93,6 @@ import org.eclipse.swt.SWT;
import org.eclipse.swt.SWTException;
import org.eclipse.swt.widgets.Combo;
import org.eclipse.swt.widgets.Control;
-import org.eclipse.swt.widgets.Display;
import org.eclipse.swt.widgets.Shell;
import org.eclipse.swt.widgets.TableItem;
@@ -1402,7 +1402,7 @@ public class TestingGuiPlugin {
*/
public static HopGuiPipelineGraph getPipelineGraph(PipelineMeta
pipelineMeta) {
// Tab / graph lookup may touch SWT widgets; only safe on the UI thread
(issue #7896).
- if (Display.getCurrent() == null) {
+ if (SessionDisplay.current() == null) {
return null;
}
try {
@@ -1441,7 +1441,7 @@ public class TestingGuiPlugin {
return null;
}
// Same rule as getPipelineGraph: never access HopGui/SWT from a worker
thread (issue #7896).
- if (Display.getCurrent() == null) {
+ if (SessionDisplay.current() == null) {
return null;
}
Map<String, Object> stateMap = getStateMap(pipelineMeta);
diff --git
a/plugins/misc/testing/src/test/java/org/apache/hop/testing/gui/TestingGuiPluginTweakTest.java
b/plugins/misc/testing/src/test/java/org/apache/hop/testing/gui/TestingGuiPluginTweakTest.java
index 64da9ee26d..697e06a56f 100644
---
a/plugins/misc/testing/src/test/java/org/apache/hop/testing/gui/TestingGuiPluginTweakTest.java
+++
b/plugins/misc/testing/src/test/java/org/apache/hop/testing/gui/TestingGuiPluginTweakTest.java
@@ -21,14 +21,18 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mockStatic;
import java.util.List;
+import org.apache.hop.core.Const;
import org.apache.hop.pipeline.PipelineMeta;
import org.apache.hop.pipeline.transform.TransformMeta;
import org.apache.hop.testing.PipelineTweak;
import org.apache.hop.testing.PipelineUnitTest;
import org.apache.hop.testing.PipelineUnitTestTweak;
+import org.eclipse.swt.widgets.Display;
import org.junit.jupiter.api.Test;
+import org.mockito.MockedStatic;
/**
* Unit tests for multi-transform unit-test tweak application (issue #2742).
Covers pure helpers in
@@ -87,6 +91,36 @@ class TestingGuiPluginTweakTest {
assertNull(TestingGuiPlugin.getStateMap(new PipelineMeta()));
}
+ /**
+ * In Hop Web the guard itself used to be the failure (issue #8248):
background work carries a RAP
+ * session over to the thread that runs it, and once that session has been
destroyed RWT throws
+ * from {@code Display.getCurrent()} rather than answering "no display". The
exception escaped
+ * into GetFields, which logged "Error calling extension point
'GetFieldsExtension'" for every
+ * transform. A thread whose session is gone is a thread with no unit test.
+ */
+ @Test
+ void getCurrentUnitTestReturnsNullWhenTheSessionIsGone() {
+ String runtime = System.getProperty(Const.HOP_PLATFORM_RUNTIME);
+ System.setProperty(Const.HOP_PLATFORM_RUNTIME, "GUI");
+ try (MockedStatic<Display> display = mockStatic(Display.class)) {
+ display
+ .when(Display::getCurrent)
+ .thenThrow(
+ new NullPointerException(
+ "Cannot invoke
\"org.eclipse.rap.rwt.service.UISession.getAttribute(String)\""
+ + " because \"uiSession\" is null"));
+
+ assertNull(TestingGuiPlugin.getCurrentUnitTest(new PipelineMeta()));
+ assertNull(TestingGuiPlugin.getStateMap(new PipelineMeta()));
+ } finally {
+ if (runtime == null) {
+ System.clearProperty(Const.HOP_PLATFORM_RUNTIME);
+ } else {
+ System.setProperty(Const.HOP_PLATFORM_RUNTIME, runtime);
+ }
+ }
+ }
+
@Test
void applyTweakEnableAddsBypass() {
PipelineUnitTest unitTest = new PipelineUnitTest();
diff --git
a/rap/src/main/java/org/apache/hop/ui/hopgui/BackgroundThreadFacadeImpl.java
b/rap/src/main/java/org/apache/hop/ui/hopgui/BackgroundThreadFacadeImpl.java
index 9e4efd74f8..aa8c96ffb9 100644
--- a/rap/src/main/java/org/apache/hop/ui/hopgui/BackgroundThreadFacadeImpl.java
+++ b/rap/src/main/java/org/apache/hop/ui/hopgui/BackgroundThreadFacadeImpl.java
@@ -35,7 +35,18 @@ public class BackgroundThreadFacadeImpl extends
BackgroundThreadFacade {
}
// exec() binds the session to whatever thread calls it, which is the
point here: the work
// itself runs exactly as before, only now with a session to ask.
- return () -> uiSession.exec(runnable);
+ return () -> {
+ if (!uiSession.isBound()) {
+ // The session ended between scheduling this work and running it - the
browser closed, the
+ // user logged out, the session timed out. Binding it anyway leaves
the thread with a
+ // context that resolves to nothing, which is worse than no context at
all: RWT reads it
+ // back unchecked and even asking whether this thread has a display
then fails with a
+ // NullPointerException (issue #8248). The work itself never needed
the session.
+ runnable.run();
+ return;
+ }
+ uiSession.exec(runnable);
+ };
}
/** The session serving this thread, or null when there is none to carry
over. */
diff --git
a/rap/src/test/java/org/apache/hop/ui/hopgui/BackgroundThreadFacadeImplTest.java
b/rap/src/test/java/org/apache/hop/ui/hopgui/BackgroundThreadFacadeImplTest.java
index 853f0b75c8..4552f50094 100644
---
a/rap/src/test/java/org/apache/hop/ui/hopgui/BackgroundThreadFacadeImplTest.java
+++
b/rap/src/test/java/org/apache/hop/ui/hopgui/BackgroundThreadFacadeImplTest.java
@@ -20,8 +20,10 @@ package org.apache.hop.ui.hopgui;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockStatic;
+import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
+import static org.mockito.Mockito.when;
import org.eclipse.rap.rwt.RWT;
import org.eclipse.rap.rwt.service.UISession;
@@ -40,6 +42,7 @@ class BackgroundThreadFacadeImplTest {
@DisplayName("work started from a session runs with that session bound")
void carriesTheSessionOverToTheBackgroundThread() {
UISession uiSession = mock(UISession.class);
+ when(uiSession.isBound()).thenReturn(true);
Runnable work = mock(Runnable.class);
try (MockedStatic<RWT> rwt = mockStatic(RWT.class)) {
@@ -48,7 +51,7 @@ class BackgroundThreadFacadeImplTest {
Runnable bound = new BackgroundThreadFacadeImpl().bindInternal(work);
// The session is read here, on the thread that starts the work; the
work itself waits.
- verifyNoInteractions(uiSession, work);
+ verifyNoInteractions(work);
bound.run();
@@ -57,6 +60,28 @@ class BackgroundThreadFacadeImplTest {
}
}
+ /**
+ * A session that ended while the work was queued must not be made current:
RWT hands out a
+ * context that resolves to no session, and reading it back fails with a
NullPointerException as
+ * far away as {@code Display.getCurrent()} (issue #8248). The work still
runs.
+ */
+ @Test
+ @DisplayName("a session that ended in the meantime is not carried over")
+ void runsWithoutASessionThatDiedBeforeTheWorkStarted() {
+ UISession uiSession = mock(UISession.class);
+ when(uiSession.isBound()).thenReturn(false);
+ Runnable work = mock(Runnable.class);
+
+ try (MockedStatic<RWT> rwt = mockStatic(RWT.class)) {
+ rwt.when(() -> RWT.getUISession()).thenReturn(uiSession);
+
+ new BackgroundThreadFacadeImpl().bindInternal(work).run();
+
+ verify(work).run();
+ verify(uiSession, never()).exec(work);
+ }
+ }
+
@Test
@DisplayName("without a session there is nothing to carry over")
void leavesTheWorkAloneOutsideASession() {
diff --git a/ui/src/main/java/org/apache/hop/core/SwtUniversalImage.java
b/ui/src/main/java/org/apache/hop/core/SwtUniversalImage.java
index e2804fe4fe..750ee54344 100644
--- a/ui/src/main/java/org/apache/hop/core/SwtUniversalImage.java
+++ b/ui/src/main/java/org/apache/hop/core/SwtUniversalImage.java
@@ -133,7 +133,7 @@ public abstract class SwtUniversalImage {
* Pixel size of a logical extent at an SWT zoom percentage. Must be linear
({@code 200} → {@code
* 2 * 100}) to satisfy the desktop ImageDataProvider contract.
*/
- static int pixelSize(int logical, int zoom) {
+ public static int pixelSize(int logical, int zoom) {
return Math.max(1, logical * zoom / 100);
}
diff --git a/ui/src/main/java/org/apache/hop/ui/core/gui/HopNamespace.java
b/ui/src/main/java/org/apache/hop/ui/core/gui/HopNamespace.java
index f3d71258fe..501b616e16 100644
--- a/ui/src/main/java/org/apache/hop/ui/core/gui/HopNamespace.java
+++ b/ui/src/main/java/org/apache/hop/ui/core/gui/HopNamespace.java
@@ -22,6 +22,7 @@ import java.util.concurrent.ConcurrentHashMap;
import org.apache.hop.core.Const;
import org.apache.hop.core.exception.HopRuntimeException;
import org.apache.hop.core.util.Utils;
+import org.apache.hop.ui.hopgui.SessionDisplay;
import org.eclipse.swt.SWT;
import org.eclipse.swt.widgets.Display;
@@ -97,7 +98,7 @@ public class HopNamespace {
/** The namespace of the session on this thread, or null. Only call with a
user interface. */
private static String namespaceOfCurrentDisplay() {
- Display display = Display.getCurrent();
+ Display display = SessionDisplay.current();
if (display == null || display.isDisposed()) {
return null;
}
@@ -106,7 +107,7 @@ public class HopNamespace {
/** Remember the namespace for the session on this thread. Only call with a
user interface. */
private static void rememberForCurrentDisplay(String namespace) {
- Display display = Display.getCurrent();
+ Display display = SessionDisplay.current();
if (display == null || display.isDisposed()) {
return;
}
diff --git a/ui/src/main/java/org/apache/hop/ui/core/widget/TreeMemory.java
b/ui/src/main/java/org/apache/hop/ui/core/widget/TreeMemory.java
index d762498a40..8fe7bd04e9 100644
--- a/ui/src/main/java/org/apache/hop/ui/core/widget/TreeMemory.java
+++ b/ui/src/main/java/org/apache/hop/ui/core/widget/TreeMemory.java
@@ -21,6 +21,7 @@ import java.util.Hashtable;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import org.apache.hop.ui.core.ConstUi;
+import org.apache.hop.ui.hopgui.SessionDisplay;
import org.eclipse.swt.SWT;
import org.eclipse.swt.events.TreeEvent;
import org.eclipse.swt.events.TreeListener;
@@ -40,7 +41,7 @@ public class TreeMemory {
private Map<TreeMemoryEntry, Boolean> map;
public static final TreeMemory getInstance() {
- Display display = Display.getCurrent();
+ Display display = SessionDisplay.current();
if (display != null && !display.isDisposed()) {
return BY_DISPLAY.computeIfAbsent(
display,
diff --git a/ui/src/main/java/org/apache/hop/ui/hopgui/SessionDisplay.java
b/ui/src/main/java/org/apache/hop/ui/hopgui/SessionDisplay.java
new file mode 100644
index 0000000000..e2f0deb912
--- /dev/null
+++ b/ui/src/main/java/org/apache/hop/ui/hopgui/SessionDisplay.java
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hop.ui.hopgui;
+
+import org.eclipse.swt.widgets.Display;
+
+/**
+ * Answers the question "does this thread have a display to work with?"
without ever throwing.
+ *
+ * <p>{@code Display.getCurrent()} is the usual way to ask, and on the desktop
it is a plain
+ * thread-to-display lookup that cannot fail. In Hop Web it is not: RWT
resolves the display through
+ * the RAP session bound to the calling thread, so asking the question reaches
into that session.
+ *
+ * <p>{@link BackgroundThreadFacade} deliberately binds a session to the
threads that run background
+ * work, so those threads do have one to reach into. A session that has since
been destroyed - the
+ * browser closed, the user logged out, the session timed out - is still bound
to the thread but no
+ * longer resolves, and RWT dereferences it unchecked: the guard meant to keep
background threads
+ * away from the UI is where the {@code NullPointerException} comes from
instead (issue #8248,
+ * following #7896 / #7897).
+ *
+ * <p>A thread whose session died is a thread with no display, which is what
these methods answer.
+ */
+public class SessionDisplay {
+
+ private SessionDisplay() {
+ // Utility class
+ }
+
+ /**
+ * The display this thread is the user interface thread for.
+ *
+ * @return the display, or null when this thread has none - including when
the session that would
+ * have provided it is gone
+ */
+ public static Display current() {
+ try {
+ return Display.getCurrent();
+ } catch (RuntimeException e) {
+ return null;
+ }
+ }
+
+ /**
+ * The display of this thread, falling back to the default display.
+ *
+ * <p>For callers that want a display to schedule work on rather than an
answer about the calling
+ * thread. The fallback resolves through the same session, so it is guarded
too.
+ *
+ * @return a usable display, or null when there is none
+ */
+ public static Display currentOrDefault() {
+ Display display = current();
+ if (display != null) {
+ return display;
+ }
+ try {
+ return Display.getDefault();
+ } catch (RuntimeException e) {
+ return null;
+ }
+ }
+}
diff --git
a/ui/src/main/java/org/apache/hop/ui/hopgui/shared/SashFormMemory.java
b/ui/src/main/java/org/apache/hop/ui/hopgui/shared/SashFormMemory.java
index 502f6ad4aa..77e6c7f43b 100644
--- a/ui/src/main/java/org/apache/hop/ui/hopgui/shared/SashFormMemory.java
+++ b/ui/src/main/java/org/apache/hop/ui/hopgui/shared/SashFormMemory.java
@@ -27,6 +27,7 @@ import org.apache.hop.core.logging.LogChannel;
import org.apache.hop.history.AuditList;
import org.apache.hop.history.AuditManager;
import org.apache.hop.ui.hopgui.HopGui;
+import org.apache.hop.ui.hopgui.SessionDisplay;
import org.eclipse.swt.SWT;
import org.eclipse.swt.custom.SashForm;
import org.eclipse.swt.widgets.Control;
@@ -155,7 +156,7 @@ public final class SashFormMemory {
* startup).
*/
public static void resetAll() {
- Display display = Display.getCurrent();
+ Display display = SessionDisplay.current();
if (display == null || display.isDisposed()) {
return;
}
diff --git a/ui/src/test/java/org/apache/hop/ui/hopgui/SessionDisplayTest.java
b/ui/src/test/java/org/apache/hop/ui/hopgui/SessionDisplayTest.java
new file mode 100644
index 0000000000..79ef812863
--- /dev/null
+++ b/ui/src/test/java/org/apache/hop/ui/hopgui/SessionDisplayTest.java
@@ -0,0 +1,88 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hop.ui.hopgui;
+
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.mockStatic;
+
+import org.eclipse.swt.widgets.Display;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+import org.mockito.MockedStatic;
+
+/**
+ * In Hop Web, asking whether this thread has a display goes through the RAP
session bound to it.
+ * When that session has been destroyed - the browser closed, the user logged
out, the session timed
+ * out - RWT dereferences it unchecked and the question itself throws (issue
#8248). The answer for
+ * such a thread is "no display", not an exception thrown out of a guard
clause.
+ */
+class SessionDisplayTest {
+
+ /** What RWT throws out of {@code LifeCycleUtil.getSessionDisplay} for a
destroyed session. */
+ private static NullPointerException deadSession() {
+ return new NullPointerException(
+ "Cannot invoke
\"org.eclipse.rap.rwt.service.UISession.getAttribute(String)\""
+ + " because \"uiSession\" is null");
+ }
+
+ @Test
+ @DisplayName("a thread whose session is gone has no display, rather than an
exception")
+ void currentAnswersNullWhenTheSessionIsGone() {
+ try (MockedStatic<Display> display = mockStatic(Display.class)) {
+ display.when(Display::getCurrent).thenThrow(deadSession());
+
+ assertNull(SessionDisplay.current());
+ }
+ }
+
+ @Test
+ @DisplayName("the display of a live session is handed back unchanged")
+ void currentAnswersTheDisplayOfTheThread() {
+ Display sessionDisplay = mock(Display.class);
+ try (MockedStatic<Display> display = mockStatic(Display.class)) {
+ display.when(Display::getCurrent).thenReturn(sessionDisplay);
+
+ assertSame(sessionDisplay, SessionDisplay.current());
+ }
+ }
+
+ @Test
+ @DisplayName("the fallback to the default display is guarded the same way")
+ void currentOrDefaultAnswersNullWhenNeitherResolves() {
+ try (MockedStatic<Display> display = mockStatic(Display.class)) {
+ display.when(Display::getCurrent).thenThrow(deadSession());
+ display.when(Display::getDefault).thenThrow(deadSession());
+
+ assertNull(SessionDisplay.currentOrDefault());
+ }
+ }
+
+ @Test
+ @DisplayName("a thread without a display of its own falls back to the
default one")
+ void currentOrDefaultFallsBackToTheDefaultDisplay() {
+ Display defaultDisplay = mock(Display.class);
+ try (MockedStatic<Display> display = mockStatic(Display.class)) {
+ display.when(Display::getCurrent).thenReturn(null);
+ display.when(Display::getDefault).thenReturn(defaultDisplay);
+
+ assertSame(defaultDisplay, SessionDisplay.currentOrDefault());
+ }
+ }
+}
