Replacing custom logic in Help>Search results to disable icons, with Image constructor call

[arunjose696]

Summary

For the results displayed in Help>Search, Previously, icons for disabled search hits were created by custom pixel-level manipulation using ImageData. This approach, at high zoom levels (e.g., 225%), causes icons to appear slightly distorted, as scaling of ImageData is a destructive operation. This logic has now been replaced by a call to the Image constructor with the flag set to disabled.

---

Screenshots at 225%

Current Behaviour

With This Change

---

Steps to reproduce

Testing this change directly in the runtime workspace is complicated due to conditional logic that disables icons only when the corresponding search hit is considered a potential hit — and a potential hit is defined as one without any filters applied.

To simulate the change for the purposes of this PR, the following line can be temporarily modified:

- boolean isPotentialHit = (hit instanceof SearchHit && ((SearchHit) hit).isPotentialHit());
+ boolean isPotentialHit = true;

File: EngineResultSection.java

---

Related Issue

Contributes to: vi-eclipse/Eclipse-Platform#199

[github-actions]

Test Results

 1 947 files  ±0   1 947 suites  ±0   1h 51m 33s ⏱️ + 24m 22s
 4 718 tests ±0   4 694 ✅ +1   24 💤 ±0  0 ❌  - 1 
14 154 runs  ±0  13 987 ✅ +1  167 💤 ±0  0 ❌  - 1 

Results for commit 597d7d1. ± Comparison against base commit dc7434e.

♻️ This comment has been updated with latest results.

[HeikoKlare]

I winder why SWT.IMAGE_DISABLE was not used from the beginning (as it seems to have already existed when the code was introduced). The result of the disablement is obviously a bit different from the existing visualization. I am not sure whether it's acceptable to change it. Couldn't we just keep the existing visualization in a HiDPI-aware way? For example: create an image based on an ImageGcDrawer, which, on demand, draws the original image onto the GC in the requested size and then adapts the image data (in the same ways as now) in a postProcess() implementation of the ImageGcDrawer.

[akoch-yatta]

I wonder why SWT.IMAGE_DISABLE was not used from the beginning (as it seems to have already existed when the code was introduced). The result of the disablement is obviously a bit different from the existing visualization. I am not sure whether it's acceptable to change it.

I don't see a good reason to not simplify it. Neither in the original ticket, nor the commit nor the code/javadoc I see a proper explanation, why this look was necessary. UX wise I would argue, if you use the exact image to visualize an active entry, you should use the disabled image to visualize an disabled entry. But therefor I would propose to use SWT.IMAGE_DISABLE instead of SWT.IMAGE_GRAY. This looks more similar.

Couldn't we just keep the existing visualization in a HiDPI-aware way? For example: create an image based on an ImageGcDrawer, which, on demand, draws the original image onto the GC in the requested size and then adapts the image data (in the same ways as now) in a postProcess() implementation of the ImageGcDrawer.

I would assume that would work, yes, but as stated above I would propose to use IMAGE_DISABLE as it looks good to me and doesn't have this overcomplicated pixel manipulation logic.

[HeikoKlare]

I don't see a good reason to not simplify it. Neither in the original ticket, nor the commit nor the code/javadoc I see a proper explanation, why this look was necessary. UX wise I would argue, if you use the exact image to visualize an active entry, you should use the disabled image to visualize an disabled entry. But therefor I would propose to use SWT.IMAGE_DISABLE instead of SWT.IMAGE_GRAY. This looks more similar.

I agree that we lack proper knowledge of why it was done like this. But note that this code is not about showing a disabled entry but about showing a potential search match. In my understanding, exact search matches are shown with the original image and _ potential_ search matches are shown with a grayed and slightly transparent one. The risk I see with using the disabled (or grayed) image is that in case the shown image is already gray, you will not see a different. You could of course argue that the argument holds for every use case of the disabled/grayed image options but the difference I see is that in this case the given image is "dynamic", i.e., one may hook any image in here for which you need to find a representation that looks different to indicate the potential match. In the examples we have seen, always the default "help search icon" is used, but in my understanding this icon can also be customized:

eclipse.platform/ua/org.eclipse.help.ui/src/org/eclipse/help/ui/internal/views/EngineTypeDescriptor.java

Lines 56 to 68 in a0e851b

	public Image getIconImage() {
	if (image!=null) {
	return image;
	}
	String icon = config.getAttribute(IHelpUIConstants.ATT_ICON);
	if (icon!=null) {
	String bundleId = config.getContributor().getName();
	HelpUIResources.getImageDescriptor(bundleId, icon);
	return HelpUIResources.getImage(icon);
	}
	image = HelpUIResources.getImage(IHelpUIConstants.IMAGE_HELP_SEARCH);
	return image;
	}

My guess it that that's the reason why this more complex manipulation using transparency was used as it will produce a distinguishable result for every kind of input image (other than with the disabled/grayed options). But I may also be wrong. And it may also be fine to just assume that whatever image may be passed in, the gray/disabled option is even sufficient (in particular since the algorithm for calculating the disabled image has just been improved).

@akurtakov do you have any knowledge whether it this pixel manipulation to create an image for showing potential help search results is necessary or whether we may just move to simply using the disabled option of SWT for creating an image to indicate a potential search match?

[arunjose696]

@akurtakov, Could you share your insights on whether the pixel-level manipulation for creating the “potential hit” disabled icons is still necessary? Or would it be okay to simplify by just using SWT’s disabled image flag as proposed?

[akurtakov]

TBH, it's first time I see this code. For me this PR is just fine as the difference is not that big and majority of the users would not make a difference unless they are explicitly pointed to the subtle detail.
Go on with this patch (when build succeeds and you consider it ready) and if someone wants to make this more understandable there are other paths to follow (e.g. decorate the icon with some "question" symbol). But this is not a requirement for this PR.

[akurtakov]

Just as a pointer if someone is interested in pursue such idea more https://www.google.com/url?sa=t&source=web&rct=j&opi=89978449&url=https://help.eclipse.org/latest/topic/org.eclipse.platform.doc.isv/reference/api/org/eclipse/jface/viewers/DecorationOverlayIcon.html

[arunjose696]

I get this error in my CI builds, is this some flaky test?

/home/runner/work/eclipse.platform/eclipse.platform/resources/tests/org.eclipse.core.tests.resources/target/work/data/40569c14aa58001017708a9b9a6d685f/.project
java.nio.file.FileAlreadyExistsException: /home/runner/work/eclipse.platform/eclipse.platform/resources/tests/org.eclipse.core.tests.resources/target/work/data/40569c14aa58001017708a9b9a6d685f/.project
	at java.base/sun.nio.fs.UnixException.translateToIOException(UnixException.java:94)
	at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:106)
	at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:111)
	at java.base/sun.nio.fs.UnixFileSystem.copyFile(UnixFileSystem.java:668)
	at java.base/sun.nio.fs.UnixFileSystem.copy(UnixFileSystem.java:1075)
	at java.base/sun.nio.fs.UnixFileSystemProvider.copy(UnixFileSystemProvider.java:300)
	at java.base/java.nio.file.Files.copy(Files.java:1305)
	at org.eclipse.core.tests.resources.NatureTest.testBug297871(NatureTest.java:280)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)

!ENTRY org.eclipse.core.tests.resources 1 0 2025-07-04 07:39:56.196
!MESSAGE [testBug297871(org.eclipse.core.tests.resources.NatureTest)] setUp

!ENTRY org.eclipse.core.resources 2 2 2025-07-04 07:39:56.201
!MESSAGE Save operation warnings.
!SUBENTRY 1 org.eclipse.core.resources 2 234 2025-07-04 07:39:56.201
!MESSAGE The project description file (.project) for '40569c14aa58001017708a9b9a6d685f' was missing.  This file contains important information about the project.  A new project description file has been created, but some information about the project may have been lost.

[HeikoKlare]

Thank you for the valuable feedback! Then let's go with the proposal.