Skip to content

A strict check 200% image data is double the size of 100% image data #2249

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 1 commit into
base: master
Choose a base branch
from
Open

A strict check 200% image data is double the size of 100% image data #2249

Show file tree
Hide file tree
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
7 changes: 6 additions & 1 deletion bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/internal/DPIUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,12 @@ private static ImageData autoScaleImageData (Device device, final ImageData imag
int scaledHeight = Math.round (height * scaleFactor);
boolean useSmoothScaling = isSmoothScalingEnabled() && imageData.getTransparencyType() != SWT.TRANSPARENCY_MASK;
if (useSmoothScaling) {
Image original = new Image (device, (ImageDataProvider) zoom -> imageData);
Image original = new Image(device, (ImageDataProvider) zoom -> {
if (zoom == 100) {
return imageData;
}
return null;
});
Comment on lines +304 to +309
Copy link
Contributor

Choose a reason for hiding this comment

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

is zoom == 100 always correct, is it? Will it always be fetched for 100 and then scaled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ImageData is only requested for 100 zoom, so only 100 is required

eclipse.platform.swt/bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/internal/DPIUtil.java

Line 323 in 085c1c6

ImageData result = resultImage.getImageData (100);

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah okay, then please put the 100 in a common local variable to use in both places, so it is not changed independently

ImageGcDrawer drawer = new ImageGcDrawer() {
@Override
public void drawOn(GC gc, int imageWidth, int imageHeight) {
Expand Down
19 changes: 19 additions & 0 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 @@ -610,10 +610,29 @@ public Image(Device device, ImageDataProvider imageDataProvider) {
SWT.error(SWT.ERROR_INVALID_ARGUMENT, null,
": ImageDataProvider [" + imageDataProvider + "] returns null ImageData at 100% zoom.");
}
if (Device.strictChecks) {
validateLinearScaling(imageDataProvider);
}
init();
this.device.registerResourceWithZoomSupport(this);
}

private void validateLinearScaling(ImageDataProvider provider) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This method seems tightly coupled using a lot of magic numbers. Maybe adapt it a little?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using magic numbers for 100 and 200 seems unavoidable, have converted them to final variables, if they are any better

final int zoom100 = 100;
final int zoom200 = 200;
final int scaleFactor = zoom200 / zoom100;
ImageData data100 = provider.getImageData(zoom100);
ImageData data200 = provider.getImageData(zoom200);

if (data200 == null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why not check for data100 as well?

Copy link
Contributor Author

@arunjose696 arunjose696 Jun 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

data200 may be null in a valid ImageDataProvider implementation which should pass the strict check(in this check we just make sure we dont throw exceptions if data200 is null). But data100 is guaranteed to exist and will have already been validated before this line is reached, so no additional checks are necessary here.

return;
}

if (data200.width != scaleFactor * data100.width || data200.height != scaleFactor * data100.height) {
SWT.error(SWT.ERROR_INVALID_ARGUMENT, null, "ImageData should be linearly scaled across zooms.");
}
}

/**
* The provided ImageGcDrawer will be called on demand whenever a new variant of the
* Image for an additional zoom is required. Depending on the OS-specific implementation
Expand Down
17 changes: 11 additions & 6 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 @@ -1031,13 +1031,18 @@ public void test_imageDataIsCached() {
public void test_imageDataSameViaDifferentProviders() {
assumeFalse("Cocoa generates inconsistent image data", SwtTestUtil.isCocoa);
String imagePath = getPath("collapseall.png");
ImageFileNameProvider imageFileNameProvider = __ -> {
return imagePath;
ImageFileNameProvider imageFileNameProvider = zoom -> {
if (zoom == 100)
return imagePath;
else
return null;
};
ImageDataProvider dataProvider = __ -> {
try (InputStream imageStream = Files.newInputStream(Path.of(imagePath))) {
return new ImageData(imageStream);
} catch (IOException e) {
ImageDataProvider dataProvider = zoom -> {
if (zoom == 100) {
try (InputStream imageStream = Files.newInputStream(Path.of(imagePath))) {
return new ImageData(imageStream);
} catch (IOException e) {
}
}
return null;
};
Expand Down
22 changes: 22 additions & 0 deletions ...ests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_ImageData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.fail;

import java.io.IOException;
import java.io.InputStream;
Expand All @@ -32,8 +33,10 @@

import org.eclipse.swt.SWT;
import org.eclipse.swt.SWTException;
import org.eclipse.swt.graphics.GC;
import org.eclipse.swt.graphics.Image;
import org.eclipse.swt.graphics.ImageData;
import org.eclipse.swt.graphics.ImageDataProvider;
import org.eclipse.swt.graphics.PaletteData;
import org.eclipse.swt.graphics.RGB;
import org.eclipse.swt.tests.graphics.ImageDataTestHelper;
Expand Down Expand Up @@ -799,6 +802,25 @@ public void test_setAlphaIII() {
assertSWTProblem("Incorrect exception thrown for putWidth < 0", SWT.ERROR_INVALID_ARGUMENT, ex);
}

@Test
public void test_drawImageFailsWithNonScalingImageDataProviderWhenstrictCheckEnabled() {
Display display = Display.getDefault();
GC gc = new GC(display);
try {
ImageDataProvider wrongDataProvider = (zoom) -> new ImageData(16, 16, 32, new PaletteData());
Image image = new Image(display, wrongDataProvider);
gc.drawImage(image, 0, 0, 16, 16, 0, 0, 16, 16);
image.dispose();
if (System.getProperty("org.eclipse.swt.internal.enableStrictChecks") != null) {
fail("Expected an exception due to non-linearly scaled image data provider");
}
} catch (IllegalArgumentException | SWTException e) {
} finally {
gc.dispose();
display.dispose();
}
}

@Test
public void test_setPixelIII() {
int value;
Expand Down
16 changes: 12 additions & 4 deletions .../JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_internal_SVGRasterizer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,23 @@ public void test_ConstructorLorg_eclipse_swt_graphics_Device_ImageFileNameProvid
@Test
public void test_ConstructorLorg_eclipse_swt_graphics_Device_ImageDataProvider() {
ImageDataProvider validImageDataProvider = zoom -> {
String fileName = "collapseall.svg";
return new ImageData(getPath(fileName));
if (zoom == 100) {
String fileName = "collapseall.svg";
return new ImageData(getPath(fileName));
} else {
return null;
}
};
Image image = new Image(Display.getDefault(), validImageDataProvider);
image.dispose();

ImageDataProvider corruptImageDataProvider = zoom -> {
String fileName = "corrupt.svg";
return new ImageData(getPath(fileName));
if (zoom == 100) {
String fileName = "corrupt.svg";
return new ImageData(getPath(fileName));
} else {
return null;
}
};
SWTException e = assertThrows(SWTException.class,
() -> new Image(Display.getDefault(), corruptImageDataProvider));
Expand Down
Loading