Adding disable/gray image props in ImageData when rescaled

[ShahzaibIbrahim]

Disabled image do not work when rescaled. Writing a new method copyImageDataForDisabledOrGrayImages and calling it before init image in win32_getHandle.

HOW TO TEST

Put the following snippet into the snippet folder to test the effect:

/*******************************************************************************
 * Copyright (c) 2015, 2018 IBM Corporation and others.
 *
 * This program and the accompanying materials
 * are made available under the terms of the Eclipse Public License 2.0
 * which accompanies this distribution, and is available at
 * https://www.eclipse.org/legal/epl-2.0/
 *
 * SPDX-License-Identifier: EPL-2.0
 *
 * Contributors:
 *     IBM Corporation - initial API and implementation
 *******************************************************************************/
package org.eclipse.swt.snippets;

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

public class Snippet382 {
	private static final String IMAGE_100 = "eclipse16.png";
	private static final String IMAGE_150 = "eclipse24.png";
	private static final String IMAGE_200 = "eclipse32.png";
	private static final String IMAGES_ROOT = "bin/org/eclipse/swt/snippets/";

	private static final String IMAGE_PATH_100 = IMAGES_ROOT + IMAGE_100;
	private static final String IMAGE_PATH_150 = IMAGES_ROOT + IMAGE_150;
	private static final String IMAGE_PATH_200 = IMAGES_ROOT + IMAGE_200;

	public static void main (String [] args) {
		final ImageFileNameProvider filenameProvider = zoom -> {
			switch (zoom) {
			case 100:
				return IMAGE_PATH_100;
			case 150:
				return IMAGE_PATH_150;
			case 200:
				return IMAGE_PATH_200;
			default:
				return null;
			}
		};
		final ImageDataProvider imageDataProvider = zoom -> {
			switch (zoom) {
			case 100:
				return new ImageData (IMAGE_PATH_100);
			case 150:
				return new ImageData (IMAGE_PATH_150);
			case 200:
				return new ImageData (IMAGE_PATH_200);
			default:
				return null;
			}
		};

		final Display display = new Display ();
		final Shell shell = new Shell (display);
		shell.setText("Snippet367");
		shell.setLayout (new GridLayout (3, false));
		Listener l = new Listener() {
			@Override
			public void handleEvent(Event e)  {
				if (e.type == SWT.Paint) {
					GC mainGC = e.gc;
					GCData gcData = mainGC.getGCData();
					final Image imageWithFileNameProvider = new Image (display, filenameProvider);
					final Image disabledImageWithFileNameProvider = new Image (display,imageWithFileNameProvider, SWT.IMAGE_DISABLE);
					final Image greyImageWithFileNameProvider = new Image (display,imageWithFileNameProvider, SWT.IMAGE_GRAY);

					final Image imageWithDataProvider = new Image (display, imageDataProvider);
					final Image disabledImageWithDataProvider = new Image (display,imageWithDataProvider, SWT.IMAGE_DISABLE);
					final Image greyImageWithDataProvider = new Image (display,imageWithDataProvider, SWT.IMAGE_GRAY);

					final Image imageWithData = new Image (display, IMAGE_PATH_100);
					final Image disabledImageWithData = new Image (display,imageWithData, SWT.IMAGE_DISABLE);
					final Image greyImageWithData = new Image (display,imageWithData, SWT.IMAGE_GRAY);

					try {
						drawImages(mainGC, gcData, "Normal",40, imageWithFileNameProvider);
						drawImages(mainGC, gcData, "Disabled",80, disabledImageWithFileNameProvider);
						drawImages(mainGC, gcData, "Greyed",120, greyImageWithFileNameProvider);

						drawImages(mainGC, gcData, "Normal",160, imageWithDataProvider);
						drawImages(mainGC, gcData, "Disabled",200, disabledImageWithDataProvider);
						drawImages(mainGC, gcData, "Greyed",240, greyImageWithDataProvider);

						drawImages(mainGC, gcData, "Normal",280, imageWithDataProvider);
						drawImages(mainGC, gcData, "Disabled",320, disabledImageWithData);
						drawImages(mainGC, gcData, "Greyed",360, greyImageWithData);
					} finally {
						mainGC.dispose ();
					}
				}
			}

			private void drawImages(GC mainGC, GCData gcData, String text, int y, final Image imageWithFileNameProvider) {
				gcData.nativeZoom = 100;
				mainGC.drawText(text, 0, y);
				mainGC.drawImage(imageWithFileNameProvider, 50, y);
				gcData.nativeZoom = 150;
				mainGC.drawImage(imageWithFileNameProvider, 100, y);
				gcData.nativeZoom = 200;
				mainGC.drawImage(imageWithFileNameProvider, 150, y/2);
			}
		};
		shell.addListener(SWT.Paint, l);




		shell.setSize(600, 600);
		shell.open ();
		while (!shell.isDisposed ()) {
			if (!display.readAndDispatch ()) display.sleep ();
		}
		display.dispose ();
	}

}

[github-actions]

Test Results

   478 files  ±0     478 suites  ±0   7m 55s ⏱️ +22s
 4 147 tests ±0   4 139 ✅ ±0   8 💤 ±0  0 ❌ ±0 
16 354 runs  ±0  16 262 ✅ ±0  92 💤 ±0  0 ❌ ±0 

Results for commit fefe52f. ± Comparison against base commit c3f7474.

♻️ This comment has been updated with latest results.

[fedejeanne]

I tested it and it has the expected behavior ✔️

There's only one small typo (missing "e") and I would like to have the snippet also contributed. You need to set the name of the dialog properly though: shell.setText("Snippet382");

[fedejeanne]

I corrected some stuff and created a new screenshot without borders that you can use:

[HeikoKlare]

From a functional perspective, the changes look sound to me. Still the change seem to introduce some unnecessary complexity (or I do not understand the necessity for the complexity so far).

[HeikoKlare]

Thank you for the recent improvements. The PR looks much better now. Changes are easy to comprehend and look sound. I only have one comment remaining w.r.t. documentation, but since it is about a private method that is easy to comprehend by its name and implementation, I propose to simply remove the potentially irritating documentation.

[iloveeclipse]

Commit ba97568 broke compilation of snippets project on Linux. Please next time exclude platform specific sources in the Snippets project from classpath on the platforms where the source can't compile.

I've created #1372.

@HeikoKlare: I wonder if we can make the org.eclipse.swt.snippets compilable with Tycho (with some extra config?) that would apply OS specific .classpath fixes to .classpath. I'm not a maven expert, but it would work with good old ant for sure :-)

[laeubi]

I once opened some requests to JDT to support class folders with different settings so one not need copy around classpath settings or other hacks:

• [JEP 238] Multirelease support eclipse-jdt/eclipse.jdt.core#265
• Computation of Exclude/Include patterns in a non-persitent way by an extension eclipse-jdt/eclipse.jdt.core#642

Also see this related Tycho issues:

• Support compile code for each environment separately eclipse-tycho/tycho#632
• Add support for building a specific platform eclipse-tycho/tycho#2256
• Support class-path exclusions eclipse-tycho/tycho#253

In the end such issues are that seldom that no one really cared to work on that.

[HeikoKlare]

Thank you for the fix, @iloveeclipse! I missed that the snippet uses OS-specific functionality. I guess that it also breaks on MacOS. I will check that as a follow-up.

It would of course be great to have automated checks for this kind of problems in the build / Tycho. But I am not sure whether it is worth the effort (as indicated by Christoph's comment) or if it would not be even better to introduce some (better) way of declaring and statically checking (already in the IDE) what is part of the SWT API. E.g., having warnings for access of classes/methods marked as @noreference would have made the problem obvious in the IDE (by a warning marker) and in the builds (by introducing an additional warning).

[HeikoKlare]

MacOS is also affected. I've created #1373 to fix that.

[laeubi]

E.g., having warnings for access of classes/methods marked as @noreference would have made the problem obvious in the IDE (by a warning marker) and in the builds (by introducing an additional warning).

I think this would usually happen if we enable API analysis, but here again, the question is how much effort one want to invest compared to how often are snippets added.

I usually simply have an own project in my workspace named "swt-test" that I use to write a snippet then I copy it over (e.g. for a bugreport or for permanent usage), that works "good enough" at least :-)

[HeikoKlare]

Fully agree that it's a matter of ROI and if this was only relevant for snippets, I agree that only low effort would be worth the benefit in the rather seldom case of having changes/additions to snippets.

I have to admit that I just understood @iloveeclipse's comment completely: the snippet project is not part of the Tycho build at all. I was not aware of that. Doing that should be quite easy and thus a quick win (maybe as an intermediate solution until the requests mentioned by @laeubi for JDT/Tycho have been processed). I will have a look at that.

[HeikoKlare]

I have to admit that I just understood @\iloveeclipse's comment completely: the snippet project is not part of the Tycho build at all. I was not aware of that. Doing that should be quite easy and thus a quick win (maybe as an intermediate solution until the requests mentioned by @\laeubi for JDT/Tycho have been processed). I will have a look at that.

Works fine, but requires at least:

• Support class-path exclusions eclipse-tycho/tycho#253

@laeubi can you estimate how difficult it would be to implement that in Tycho?