Skip to content

Use the existing image to initialize the GC in ImageGcDrawerWrapper #2238

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Use the existing image to initialize the GC in ImageGcDrawerWrapper #2238

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f256493
Use the existing image to initialize the GC in ImageGcDrawerWrapper
ShahzaibIbrahim Jun 16, 2025
24fbd72
Reinitializing image with image data after post process
ShahzaibIbrahim Jun 18, 2025
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
1 change: 1 addition & 0 deletions bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/graphics/ImageGcDrawer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ public interface ImageGcDrawer {
* @param imageData The resulting ImageData after <code>drawOn</code> was called
*/
default void postProcess(ImageData imageData) {
imageData.data = null;
}

/**
Expand Down
105 changes: 55 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 @@ -1911,6 +1911,47 @@ protected final ImageHandle newImageHandle(ImageData data, int zoom) {
return init(data, zoom);
}
}

protected final ImageHandle createBaseHandle(int zoom, int width, int height) {
long handle = initBaseHandle(zoom, width, height);
ImageHandle imageHandle = new ImageHandle(handle, zoom);
zoomLevelToImageHandle.put(zoom, imageHandle);
return imageHandle;
}


private long initBaseHandle(int zoom, int width, int height) {
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;
}
}

private class ExistingImageHandleProviderWrapper extends AbstractImageProviderWrapper {
Expand Down Expand Up @@ -2121,59 +2162,22 @@ protected Rectangle getBounds(int zoom) {
@Override
ImageData newImageData(int zoom) {
if (zoomLevelToImageHandle.isEmpty()) {
return createBaseHandle(zoom).getImageData();
baseZoom = zoom;
return createBaseHandle(zoom, width, height).getImageData();
}
return getScaledImageData(zoom);
}

@Override
protected ImageHandle newImageHandle(int zoom) {
if (zoomLevelToImageHandle.isEmpty()) {
return createBaseHandle(zoom);
baseZoom = zoom;
return createBaseHandle(zoom, width, height);
}
return super.newImageHandle(zoom);
}

private ImageHandle createBaseHandle(int zoom) {
long handle = initBaseHandle(zoom);
baseZoom = zoom;
ImageHandle imageHandle = new ImageHandle(handle, zoom);
zoomLevelToImageHandle.put(zoom, imageHandle);
return imageHandle;
}

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) {
Expand Down Expand Up @@ -2541,28 +2545,29 @@ ImageData newImageData(int zoom) {
protected ImageHandle newImageHandle(int zoom) {
currentZoom = zoom;
int gcStyle = drawer.getGcStyle();
Image image;
if ((gcStyle & SWT.TRANSPARENT) != 0) {
int scaledHeight = DPIUtil.scaleUp(height, zoom);
int scaledWidth = DPIUtil.scaleUp(width, zoom);
/* Create a 24 bit image data with alpha channel */
final ImageData resultData = new ImageData (scaledWidth, scaledHeight, 24, new PaletteData (0xFF, 0xFF00, 0xFF0000));
resultData.alphaData = new byte [scaledWidth * scaledHeight];
image = new Image(device, resultData, zoom);
init(resultData, zoom);
} else {
image = new Image(device, width, height);
}
GC gc = new GC(new DrawableWrapper(image, zoom), gcStyle);
createBaseHandle(zoom, width, height);
};
GC gc = new GC(new DrawableWrapper(Image.this, zoom), gcStyle);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we then remove the DrawableWrapper and just call?

Suggested change
GC gc = new GC(new DrawableWrapper(Image.this, zoom), gcStyle);
GC gc = new GC(Image.this, gcStyle);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's a scenario where it's needed when another GC can be called over this image from the outside. I am not 100% of the scenario but maybe @akoch-yatta can explain it bit better on why it was used in the first place.

try {
gc.data.nativeZoom = zoom;
drawer.drawOn(gc, width, height);
ImageData imageData = image.getImageMetadata(zoom).getImageData();
ImageData imageData = Image.this.getImageMetadata(zoom).getImageData();
drawer.postProcess(imageData);
ImageData newData = adaptImageDataIfDisabledOrGray(imageData);
return init(newData, zoom);
if(imageData.data != null) {
zoomLevelToImageHandle.get(zoom).destroy();
init(imageData, zoom);
}
Comment on lines +2562 to +2567
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here I want initialize the imageData only if postProcess is called. That's why I have a default implementation that nullify the imageData in order to know here if it was not implemented. I know it's not a clean solution and I am open to hear suggestion.

My suggestions:

  1. Have some field to track if postProcess was called.
  2. Always call init regardless of postProcess calling.

@HeikoKlare

return zoomLevelToImageHandle.get(zoom);
} finally {
gc.dispose();
image.dispose();
}
}

Expand Down
33 changes: 33 additions & 0 deletions ...wt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_Image.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1075,6 +1075,39 @@ public void test_imageDataSameViaProviderAndSimpleData() {
dataImage.dispose();
}

@Test
public void test_ImageWithImageGcDrawerWrapper() {
Image image = new Image(display, 16, 16);
GC gc = new GC(image);
gc.setBackground(display.getSystemColor(SWT.COLOR_RED));
gc.setForeground(display.getSystemColor(SWT.COLOR_RED));
gc.fillRectangle(0, 0, 16, 16);
gc.dispose();
image.getImageData().transparentPixel = display.getSystemColor(SWT.COLOR_BLACK).getAlpha();
ImageData imageDataForSimpleImageWithGcOperation = image.getImageData();

final ImageGcDrawer imageGcDrawerWithPostProcess = new ImageGcDrawer() {
@Override
public void drawOn(GC gc, int width, int height) {
gc.setBackground(display.getSystemColor(SWT.COLOR_RED));
gc.setForeground(display.getSystemColor(SWT.COLOR_RED));
gc.fillRectangle(0, 0, 16, 16);
}

@Override
public void postProcess(ImageData imageData) {
imageData.transparentPixel = display.getSystemColor(SWT.COLOR_BLACK).getAlpha();
}
};
Image imageWithGCDrawerWrapper = new Image(display, imageGcDrawerWithPostProcess, 16, 16);
ImageData imageDataForImageWithGcDrawer = imageWithGCDrawerWrapper.getImageData();

assertEquals(0, imageDataComparator().compare(imageDataForSimpleImageWithGcOperation, imageDataForImageWithGcDrawer));

image.dispose();
imageWithGCDrawerWrapper.dispose();
}


private Comparator<ImageData> imageDataComparator() {
return Comparator.<ImageData>comparingInt(d -> d.width) //
Expand Down
Loading