Skip to content

[win32] Create image handles on demand for width/height based constructors #1884

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 14, 2025
Merged

[win32] Create image handles on demand for width/height based constructors #1884

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
166 changes: 116 additions & 50 deletions bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,11 +178,8 @@ public Image(Device device, int width, int height) {

private Image(Device device, int width, int height, int nativeZoom) {
super(device);
initialNativeZoom = nativeZoom;
final int zoom = getZoom();
width = DPIUtil.scaleUp (width, zoom);
height = DPIUtil.scaleUp (height, zoom);
init(width, height);
this.initialNativeZoom = nativeZoom;
this.imageProvider = new PlainImageProviderWrapper(width, height);
init();
this.device.registerResourceWithZoomSupport(this);
}
Expand Down Expand Up @@ -331,8 +328,7 @@ public Image(Device device, Rectangle bounds) {
super(device);
if (bounds == null) SWT.error(SWT.ERROR_NULL_ARGUMENT);
initialNativeZoom = DPIUtil.getNativeDeviceZoom();
bounds = DPIUtil.scaleUp (bounds, getZoom());
init(bounds.width, bounds.height);
this.imageProvider = new PlainImageProviderWrapper(bounds.width, bounds.height);
init();
this.device.registerResourceWithZoomSupport(this);
}
Expand Down Expand Up @@ -1406,6 +1402,10 @@ public ImageData getImageData (int zoom) {
return imageProvider.getImageData(zoom);
}

return getScaledImageData(zoom);
}

private ImageData getScaledImageData (int zoom) {
// if a GC is initialized with an Image (memGC != null), the image data must not be resized, because it would
// be a destructive operation. Therefor, always the current image data must be returned
if (memGC != null) {
Expand All @@ -1416,6 +1416,7 @@ public ImageData getImageData (int zoom) {
return DPIUtil.scaleImageData (device, getImageMetadata(closestZoom).getImageData(), zoom, closestZoom);
}


/**
* Returns an <code>ImageData</code> based on the receiver.
* Modifications made to this <code>ImageData</code> will not
Expand Down Expand Up @@ -1458,39 +1459,6 @@ public int hashCode () {
return (int)win32_getHandle(this, getZoom());
}

void init(int width, int height) {
if (width <= 0 || height <= 0) {
SWT.error (SWT.ERROR_INVALID_ARGUMENT);
}
type = SWT.BITMAP;
long hDC = device.internal_new_GC(null);
ImageHandle imageMetadata = new ImageHandle(OS.CreateCompatibleBitmap(hDC, width, height), getZoom());
/*
* Feature in Windows. CreateCompatibleBitmap() may fail
* for large images. The fix is to create a DIB section
* in that case.
*/
if (imageMetadata.handle == 0) {
int bits = OS.GetDeviceCaps(hDC, OS.BITSPIXEL);
int planes = OS.GetDeviceCaps(hDC, OS.PLANES);
int depth = bits * planes;
if (depth < 16) depth = 16;
if (depth > 24) depth = 24;
imageMetadata = new ImageHandle(createDIB(width, height, depth), getZoom());
}
if (imageMetadata.handle != 0) {
long memDC = OS.CreateCompatibleDC(hDC);
long hOldBitmap = OS.SelectObject(memDC, imageMetadata.handle);
OS.PatBlt(memDC, 0, 0, width, height, OS.PATCOPY);
OS.SelectObject(memDC, hOldBitmap);
OS.DeleteDC(memDC);
}
device.internal_dispose_GC(hDC, null);
if (imageMetadata.handle == 0) {
SWT.error(SWT.ERROR_NO_HANDLES, null, device.getLastError());
}
}

static long createDIB(int width, int height, int depth) {
BITMAPINFOHEADER bmiHeader = new BITMAPINFOHEADER();
bmiHeader.biSize = BITMAPINFOHEADER.sizeof;
Expand Down Expand Up @@ -2082,13 +2050,114 @@ public static Image win32_new(Device device, int type, long handle, int nativeZo
}

private abstract class AbstractImageProviderWrapper {
abstract Object getProvider();
protected abstract Rectangle getBounds(int zoom);
abstract ImageData getImageData(int zoom);
abstract ImageHandle getImageMetadata(int zoom);
abstract AbstractImageProviderWrapper createCopy(Image image);
abstract boolean isDisposed();

protected void destroy() {
}
}

private class PlainImageProviderWrapper extends AbstractImageProviderWrapper {
private boolean isDestroyed;
private final int width;
private final int height;

PlainImageProviderWrapper(int width, int height) {
if (width <= 0 || height <= 0) {
SWT.error (SWT.ERROR_INVALID_ARGUMENT);
}
this.width = width;
this.height = height;
type = SWT.BITMAP;
}

@Override
protected Rectangle getBounds(int zoom) {
Rectangle rectangle = new Rectangle(0, 0, width, height);
return DPIUtil.scaleUp(rectangle, zoom);
}

@Override
ImageData getImageData(int zoom) {
if (zoomLevelToImageHandle.isEmpty() || zoomLevelToImageHandle.containsKey(zoom)) {
return getImageMetadata(zoom).getImageData();
}

return getScaledImageData(zoom);
}

@Override
ImageHandle getImageMetadata(int zoom) {
Copy link
Contributor

@HeikoKlare HeikoKlare Mar 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am always a bit confused by this getImageMetadata method. Isn't this actually what we want to have:

  • A provider-specific createImageHandleIfMissing()
  • A gerneric getOrCreateImageHandle() directly in Image, which creates the image handle if missing (via above) and then just returns the value from the zoomLevelToImageHandle map

As far as I can see, that's what each of the implementations does.

This is not necessarily something to improve in this PR, but since I stumped upon it again in this PR, I wanted to take the chance to ask.

Copy link
Contributor Author

@akoch-yatta akoch-yatta Mar 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree. One issue is, that most handles are created implicitly in the different init methods. When we have transformed all constructors to use the wrappers and the handles are created more explicitly, we can probably drag out some common logic back into Image

if (zoomLevelToImageHandle.isEmpty()) {
long handle = initBaseHandle(zoom);
ImageHandle imageHandle = new ImageHandle(handle, zoom);
zoomLevelToImageHandle.put(zoom, imageHandle);
return imageHandle;
} else if(zoomLevelToImageHandle.containsKey(zoom)) {
return zoomLevelToImageHandle.get(zoom);
} else {
ImageData resizedData = getImageData(zoom);
ImageData newData = adaptImageDataIfDisabledOrGray(resizedData);
init(newData, zoom);
return zoomLevelToImageHandle.get(zoom);
}
}

private long initBaseHandle(int zoom) {
if (isDisposed()) SWT.error(SWT.ERROR_GRAPHIC_DISPOSED);
int scaledWidth = DPIUtil.scaleUp (width, zoom);
int scaledHeight = DPIUtil.scaleUp (height, zoom);
long hDC = device.internal_new_GC(null);
long newHandle = OS.CreateCompatibleBitmap(hDC, scaledWidth, scaledHeight);
/*
* Feature in Windows. CreateCompatibleBitmap() may fail
* for large images. The fix is to create a DIB section
* in that case.
*/
if (newHandle == 0) {
int bits = OS.GetDeviceCaps(hDC, OS.BITSPIXEL);
int planes = OS.GetDeviceCaps(hDC, OS.PLANES);
int depth = bits * planes;
if (depth < 16) depth = 16;
if (depth > 24) depth = 24;
newHandle = createDIB(scaledWidth, scaledHeight, depth);
}
if (newHandle != 0) {
long memDC = OS.CreateCompatibleDC(hDC);
long hOldBitmap = OS.SelectObject(memDC, newHandle);
OS.PatBlt(memDC, 0, 0, scaledWidth, scaledHeight, OS.PATCOPY);
OS.SelectObject(memDC, hOldBitmap);
OS.DeleteDC(memDC);
}
device.internal_dispose_GC(hDC, null);
if (newHandle == 0) {
SWT.error(SWT.ERROR_NO_HANDLES, null, device.getLastError());
}
return newHandle;
}

@Override
AbstractImageProviderWrapper createCopy(Image image) {
return image.new PlainImageProviderWrapper(width, height);
}

@Override
protected void destroy() {
this.isDestroyed = true;
}

@Override
boolean isDisposed() {
return isDestroyed;
}
}

private abstract class DynamicImageProviderWrapper extends AbstractImageProviderWrapper {
abstract Object getProvider();

protected void checkProvider(Object provider, Class<?> expectedClass) {
if (provider == null) SWT.error(SWT.ERROR_NULL_ARGUMENT);
if (!expectedClass.isAssignableFrom(provider.getClass())) SWT.error(SWT.ERROR_INVALID_ARGUMENT);
Expand All @@ -2101,15 +2170,12 @@ public int hashCode() {

@Override
public boolean equals(Object otherProvider) {
return otherProvider instanceof AbstractImageProviderWrapper aip //
&& getProvider().equals(aip.getProvider());
}

protected void destroy() {
return otherProvider instanceof DynamicImageProviderWrapper aip
&& Objects.equals(getProvider(), aip.getProvider());
}
}

private class ImageFileNameProviderWrapper extends AbstractImageProviderWrapper {
private class ImageFileNameProviderWrapper extends DynamicImageProviderWrapper {

/**
* ImageFileNameProvider to provide file names at various Zoom levels
Expand Down Expand Up @@ -2175,7 +2241,7 @@ ImageFileNameProviderWrapper createCopy(Image image) {
}
}

private class ImageDataProviderWrapper extends AbstractImageProviderWrapper {
private class ImageDataProviderWrapper extends DynamicImageProviderWrapper {

/**
* ImageDataProvider to provide ImageData at various Zoom levels
Expand Down Expand Up @@ -2225,13 +2291,13 @@ ImageDataProviderWrapper createCopy(Image image) {
}
}

private class ImageGcDrawerWrapper extends AbstractImageProviderWrapper {
private class ImageGcDrawerWrapper extends DynamicImageProviderWrapper {
private ImageGcDrawer drawer;
private int width;
private int height;
private boolean isDestroyed;

public ImageGcDrawerWrapper(ImageGcDrawer imageGcDrawer, int width, int height) {
ImageGcDrawerWrapper(ImageGcDrawer imageGcDrawer, int width, int height) {
checkProvider(imageGcDrawer, ImageGcDrawer.class);
this.drawer = imageGcDrawer;
this.width = width;
Expand Down
Loading