[Refactoring] Streamline calls to Image#adaptImageDataIfDisabledOrGray()

[ShahzaibIbrahim]

Enabling a consistent pattern to apply disablement or graying of an Image. They are applied whenever a handle is being created from init method. Ensuring streamline usage and avoid double calls to this method.

How to test

• Run the Snippet382 with following VM Argument: swt.autoScale.updateOnRuntime=true
• Move the shell between different zoom levels (100,125,150,175,200,250)
• The snippet tests different kind of image providers to create an Image.
• Expected result would be that we don't lose disabled or grayed image attributes.

---

Sanity test for the Cursor:

import org.eclipse.swt.*;
import org.eclipse.swt.graphics.*;
import org.eclipse.swt.widgets.*;

public class CustomCursorExample {
    public static void main(String[] args) {
        Display display = new Display();
        Shell shell = new Shell(display);
        shell.setText("Custom Cursor Example");
        shell.setSize(400, 300);

        // Load an image to use as the cursor
        Cursor customCursor = null;
        try {
        	ImageData imageData = new ImageData ("bin/org/eclipse/swt/snippets/eclipse16.png");
        	Image normalImage = new Image (display, imageData);
        	Image disabledImage = new Image (display, normalImage, SWT.IMAGE_DISABLE);

            Point hotspot = new Point(8, 8);

            customCursor = new Cursor(display, disabledImage.getImageData(), hotspot.x, hotspot.y);

            // Apply it to the shell
            shell.setCursor(customCursor);

        } catch (Exception e) {
            e.printStackTrace();
        }
        Label label = new Label(shell, SWT.NONE);
        label.setText("Hover here to see the custom cursor!");
        label.setBounds(50, 100, 300, 20);

        shell.open();
        while (!shell.isDisposed()) {
            if (!display.readAndDispatch()) display.sleep();
        }

        // Clean up resources
        if (customCursor != null) customCursor.dispose();
        display.dispose();
    }
}

The cursor should be disabled using this snippet.

[ShahzaibIbrahim]

@copilot could you review it first.

[github-actions]

Test Results

  118 files  ±0    118 suites  ±0   17m 59s ⏱️ +11s
4 651 tests ±0  4 633 ✅  - 1  18 💤 +1  0 ❌ ±0 
  330 runs  ±0    326 ✅ ±0   4 💤 ±0  0 ❌ ±0 

Results for commit f6ec60e. ± Comparison against base commit 9aa5074.

This pull request skips 1 test.

org.eclipse.swt.tests.junit.Test_org_eclipse_swt_browser_Browser ‑ test_setUrl_remote_with_post

♻️ This comment has been updated with latest results.

[akoch-yatta]

One remark: as the change seems to affect Cursor as well - Is there a Snippet/test case or could you provide one to test correctness for Cursor?

[ShahzaibIbrahim]

One remark: as the change seems to affect Cursor as well - Is there a Snippet/test case or could you provide one to test correctness for Cursor?

I have added a sanity test for disabled cursor in the description.

[HeikoKlare]

This PR seems to currently mix up two changes: one is streamlining calls to adaptImageDataIfDisabledOrGray in the sense that there should be a common concept when to call it consistently (without any behavioral changes). Another is applying this method where it (erronoeously? on purpose?) was not applied before. The latter affects Icon-type images, for which it seems as if the adaptation was not always applied.
Can we split the PR up accordingly and, in particular, how can we see/test the changed behavior for Icon-type images?

[ShahzaibIbrahim]

@HeikoKlare there seems to be a confusion. For Icon it was working even before. Although during all these refactoring the signature for org.eclipse.swt.graphics.Image.init(Device, ImageData, int) has changed which accepts the style flag and org.eclipse.swt.graphics.Image.initIcon(Device, ImageData, ImageData, int) method which is called from cursor now need to pass the style flag (which could be null since the image for that now can be initialized directly with disabled or gray flag.

The short answer is the flag was applied even before this change for the images of type Icon (can be tested through Snippet382)

I have made a small change which will reflect previous behavior as is for the icons after refactoring.

[HeikoKlare]

The short answer is the flag was applied even before this change for the images of type Icon (can be tested through Snippet382)

When running Snippet382, neither of the initIconHandle methods seems to be called. I've set breakpoints in both of them and none of them was triggered.

I went through the call sequences a bit to see if I miss something and found that init() (where the adaptation will be moved to by this PR) is executed quite often (obviously for every handle to be created). In many cases, a temporary handle will be created first (just for the sake of generating image data) and the extracted image data will later be used to create an actual handle to be used. So can't we improve the implementation to only do the adaptation when the initial image data / handle is created and no repeated applications of the adaptation logic to the same data / handle happen?

[ShahzaibIbrahim]

When running Snippet382, neither of the initIconHandle methods seems to be called. I've set breakpoints in both of them and none of them was triggered.

import org.eclipse.swt.*;
import org.eclipse.swt.graphics.*;
import org.eclipse.swt.widgets.*;

public class Snippet70 {

public static void main (String [] args) {
	System.setProperty("swt.autoScale.updateOnRuntime", "true");
	Display display = new Display ();
	Color red = display.getSystemColor (SWT.COLOR_RED);
	Color white = display.getSystemColor (SWT.COLOR_WHITE);
	Color black = display.getSystemColor (SWT.COLOR_BLACK);

	Image image = new Image (display, 20, 20);
	GC gc = new GC (image);
	gc.setBackground (red);
	gc.fillRectangle (5, 5, 10, 10);
	gc.dispose ();
	ImageData imageData = image.getImageData ();

	PaletteData palette = new PaletteData (new RGB (0, 0, 0),new RGB (0xFF, 0xFF, 0xFF));
	ImageData maskData = new ImageData (20, 20, 1, palette);
	Image mask = new Image (display, maskData);
	gc = new GC (mask);
	gc.setBackground (black);
	gc.fillRectangle (0, 0, 20, 20);
	gc.setBackground (white);
	gc.fillRectangle (5, 5, 10, 10);
	gc.dispose ();
	maskData = mask.getImageData ();

	Image icon = new Image (display, imageData, maskData);
	Image disabledIcon = new Image(display, icon, SWT.IMAGE_DISABLE);
	Image grayedIcon = new Image(display, icon, SWT.IMAGE_GRAY);
	Shell shell = new Shell (display);
	shell.setText("Snippet 70");
	Button button = new Button (shell, SWT.PUSH);
	button.setImage (disabledIcon);
	button.setSize (60, 60);

	Button button2 = new Button (shell, SWT.PUSH);
	button2.setImage (grayedIcon);
	button2.setSize (60, 60);
	shell.open ();
	while (!shell.isDisposed ()) {
		if (!display.readAndDispatch ()) display.sleep ();
	}
	icon.dispose ();
	disabledIcon.dispose ();
	grayedIcon.dispose();
	image.dispose ();
	mask.dispose ();
	display.dispose ();
}
}

This snippet can test the icon disablement and grayness. Initially I though we had an example in 382.