Bug 575033 - [GTK] Image of disabled ToolItem is not always greyed-out
Summary: [GTK] Image of disabled ToolItem is not always greyed-out
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: SWT (show other bugs)
Version: 4.18   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: 4.21 M2   Edit
Assignee: Simeon Andreev CLA
QA Contact:
URL:
Whiteboard:
Keywords: regression
Depends on:
Blocks:
 
Reported: 2021-07-26 07:06 EDT by Simeon Andreev CLA
Modified: 2021-07-28 03:35 EDT (History)
3 users (show)

See Also:


Attachments
Showcasing ToolItem icons with Eclipse 4.16. (10.78 KB, image/png)
2021-07-26 08:08 EDT, Simeon Andreev CLA
no flags Details
Showcasing ToolItem icons with Eclipse 4.21. (9.84 KB, image/png)
2021-07-26 08:10 EDT, Simeon Andreev CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Simeon Andreev CLA 2021-07-26 07:06:36 EDT
Snippet to reproduce:

	public static void main(String[] args) {
		Display display = new Display();
		Shell shell = new Shell(display);
		ToolBar toolBar = new ToolBar(shell, SWT.FLAT | SWT.WRAP | SWT.RIGHT);

		ToolItem item1 = new ToolItem(toolBar, SWT.PUSH);
		item1.setText("enabled");
		Image icon1 = display.getSystemImage(SWT.ICON_QUESTION);
		item1.setImage(icon1);

		ToolItem item2 = new ToolItem(toolBar, SWT.PUSH);
		item2.setText("disabled");
		Image icon2 = display.getSystemImage(SWT.ICON_QUESTION);
		item2.setEnabled(false);
		item2.setImage(icon2);

		toolBar.pack();

		shell.addListener(SWT.Resize, new Listener() {
			public void handleEvent(Event event) {
				Rectangle clientArea = shell.getClientArea();
				toolBar.setSize(toolBar.computeSize(clientArea.width, SWT.DEFAULT));
			}
		});

		shell.setSize(350, 150);
		shell.open();

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

		display.dispose();
	}

The fix in bug 568771 doesn't cover the case where a tool item is disabled first, and then its image is set. The regression is from bug 564097.
Comment 1 Eclipse Genie CLA 2021-07-26 07:39:07 EDT
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/183377
Comment 2 Simeon Andreev CLA 2021-07-26 08:08:31 EDT
Created attachment 286831 [details]
Showcasing ToolItem icons with Eclipse 4.16.
Comment 3 Simeon Andreev CLA 2021-07-26 08:10:09 EDT
Created attachment 286832 [details]
Showcasing ToolItem icons with Eclipse 4.21.

See more problems with the fix from bug 568771:

1. Disabled icon passed to ToolItem is not greyed out in Eclipse 4.21, but is in Eclipse 4.16 (expected behaviour).
2. Visual fidelity of greying an image out with the added SWT code is not the same as the greying out that GTK+ does for pixbuf. In fact, its much lower than the GTK+ quality.

Andrey, are we dealing with those as well?
Comment 4 Andrey Loskutov CLA 2021-07-26 08:18:49 EDT
(In reply to Simeon Andreev from comment #3)
> Created attachment 286832 [details]
> Showcasing ToolItem icons with Eclipse 4.21.
> 
> See more problems with the fix from bug 568771:
> 
> 1. Disabled icon passed to ToolItem is not greyed out in Eclipse 4.21, but
> is in Eclipse 4.16 (expected behaviour).

This is this bug.

> 2. Visual fidelity of greying an image out with the added SWT code is not
> the same as the greying out that GTK+ does for pixbuf. In fact, its much
> lower than the GTK+ quality.
> 
> Andrey, are we dealing with those as well?

For point 2 please create new issue "SWT disabled icons are of low quality".

If we are going to apply similar fixes for Button/Menu (and it looks like we will), we should afterwards try to improve the quality of disabled icons. 

Probably some common static method can be used to convert given icon to grayscale version, and the same method referenced in all three classes (even if it renders something very poor today).

It looks to me as if the SWT disabled icons were created with 16 colors (or very reduced) palette in SWT, while GTK version seem to use more colors palette for grayed icons.
Comment 5 Simeon Andreev CLA 2021-07-26 08:23:50 EDT
> This is this bug.

No, this is not "the bug". ToolItem allows setting an extra disabled image. The fix for bug 568771 doesn't ensure the disabled image is greyed out.

This doesn't relate to the problem from the description: a disabled ToolItem *without* a disabled image.
Comment 6 Andrey Loskutov CLA 2021-07-26 08:26:36 EDT
(In reply to Simeon Andreev from comment #5)
> > This is this bug.
> 
> No, this is not "the bug". ToolItem allows setting an extra disabled image.
> The fix for bug 568771 doesn't ensure the disabled image is greyed out.
> 
> This doesn't relate to the problem from the description: a disabled ToolItem
> *without* a disabled image.

That was not clear for me from the description / bug title. Please create another bug and update title to reflect which case is handled here exactly.
Comment 8 Andrey Loskutov CLA 2021-07-28 03:35:26 EDT
Works as expected in I20210727-1800, except the image quality, which will be addressed by bug 575069.