Community
Participate
Working Groups
The icon of the disabled Button is not disabled anymore. This is a regression from bug 564097, gerrit https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/165440 , commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=a05ee0e05540ae5f068f127d687828dc3cdf0341 Snippet to reproduce below. Click on the second button - the icon of the first button doesn't change to grayed state, only the text. package org.eclipse.swt.snippets; import org.eclipse.swt.*; import org.eclipse.swt.events.*; import org.eclipse.swt.graphics.*; import org.eclipse.swt.layout.*; import org.eclipse.swt.widgets.*; public class Snippet206 { public static void main(String[] args) { Display display = new Display(); Image image = display.getSystemImage(SWT.ICON_QUESTION); Shell shell = new Shell(display); shell.setText("Snippet 206"); shell.setLayout (new GridLayout()); Button button = new Button(shell, SWT.PUSH); button.setImage(image); button.setText("Will be disabled"); Button button2 = new Button(shell, SWT.PUSH); button2.setImage(image); button2.setText("Click me!"); SelectionListener listener = new SelectionListener() { @Override public void widgetSelected(SelectionEvent e) { button.setEnabled(!button.isEnabled()); } @Override public void widgetDefaultSelected(SelectionEvent e) { } }; button2.addSelectionListener(listener ); shell.setSize(300, 300); shell.open(); while (!shell.isDisposed ()) { if (!display.readAndDispatch ()) display.sleep (); } display.dispose (); } }
Looks like the change from gtk_image_set_from_gicon to gtk_image_set_from_surface causes missing state transition from enabled to disabled icon? I don't see other relevant code changes in patch https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/165440 . @Paul, Sravan: could you please check, if we need to use some other feature / API from GTK3 to get the icon automatically disabled, or if we should now create "disposed" icon manually and switch between enabled / disabled image as soon as setEnabled() changes the Button state?
(In reply to Andrey Loskutov from comment #1) > Looks like the change from gtk_image_set_from_gicon to > gtk_image_set_from_surface causes missing state transition from enabled to > disabled icon? I think this is correct, I can reproduce the same behaviour with just GTK3 snippets. GTK+ example using a surface for a button image: #include <gtk/gtk.h> // gcc -g button_image.c `pkg-config --cflags --libs gtk+-3.0` -o ButtonImage && ./ButtonImage static void print_hello (GtkWidget *widget, gpointer data) { g_print ("Hello World\n"); } int main (int argc, char **argv) { gtk_init(&argc, &argv); GtkWidget *window; GtkWidget *button; GtkWidget *button_box; GtkWidget *label; GtkWidget *image; cairo_surface_t *surface; cairo_t *cr; window = gtk_window_new(GTK_WINDOW_TOPLEVEL); g_signal_connect(window, "delete_event", gtk_main_quit, NULL); gtk_window_set_title (GTK_WINDOW (window), "Window"); gtk_window_set_default_size (GTK_WINDOW (window), 200, 200); button = gtk_button_new (); g_signal_connect (button, "clicked", G_CALLBACK (print_hello), NULL); g_signal_connect_swapped (button, "clicked", G_CALLBACK (gtk_widget_destroy), window); button_box = gtk_box_new (GTK_ORIENTATION_HORIZONTAL, 4); label = gtk_label_new ("button"); image = gtk_image_new (); gtk_container_add (GTK_CONTAINER (window), button); gtk_container_add (GTK_CONTAINER (button), button_box); gtk_container_add (GTK_CONTAINER (button_box), image); gtk_container_add (GTK_CONTAINER (button_box), label); surface = cairo_image_surface_create (CAIRO_FORMAT_ARGB32, 32, 32); cr = cairo_create (surface); cairo_set_source_rgb(cr, 1, 0.0, 0.0); cairo_set_line_width(cr, 1); cairo_rectangle(cr, 0, 0, 32, 32); cairo_stroke_preserve(cr); cairo_fill(cr); cairo_destroy (cr); gtk_image_set_from_surface (GTK_IMAGE (image), surface); gtk_widget_set_sensitive (GTK_WIDGET (button), FALSE); gtk_widget_show_all (window); gtk_main(); return 0; } GTK+ example using a pixbuf: #include <gtk/gtk.h> // gcc -g button_image_gicon.c `pkg-config --cflags --libs gtk+-3.0` -o ButtonImageGIcon && ./ButtonImageGIcon static void print_hello (GtkWidget *widget, gpointer data) { g_print ("Hello World\n"); } int main (int argc, char **argv) { gtk_init(&argc, &argv); GtkWidget *window; GtkWidget *button; GtkWidget *button_box; GtkWidget *label; GtkWidget *image; window = gtk_window_new(GTK_WINDOW_TOPLEVEL); g_signal_connect(window, "delete_event", gtk_main_quit, NULL); gtk_window_set_title (GTK_WINDOW (window), "Window"); gtk_window_set_default_size (GTK_WINDOW (window), 200, 200); button = gtk_button_new (); g_signal_connect (button, "clicked", G_CALLBACK (print_hello), NULL); g_signal_connect_swapped (button, "clicked", G_CALLBACK (gtk_widget_destroy), window); button_box = gtk_box_new (GTK_ORIENTATION_HORIZONTAL, 4); label = gtk_label_new ("button"); image = gtk_image_new (); gtk_container_add (GTK_CONTAINER (window), button); gtk_container_add (GTK_CONTAINER (button), button_box); gtk_container_add (GTK_CONTAINER (button_box), image); gtk_container_add (GTK_CONTAINER (button_box), label); GdkPixbuf *pixbuf; GError *error = NULL; pixbuf = gdk_pixbuf_new_from_file ("/home/sandreev/tmp/testicon.png", &error); gtk_image_set_from_gicon (GTK_IMAGE (image), (GIcon *) pixbuf, GTK_ICON_SIZE_BUTTON); gtk_widget_set_sensitive (GTK_WIDGET (button), FALSE); g_object_unref (pixbuf); gtk_widget_show_all (window); gtk_main(); return 0; } ("/home/sandreev/tmp/testicon.png" is a 32x32 pixel red (RGB 255,0,0) square, matching the surface created in the surface based snippet above) From discussions in related SWT tickets, I understood that pixbuf use is deprecated and should be replaced with surface. Sravan, what is your recommendation here? 1. Should we open a bug for GTK+ developers (it seems odd to me, considering how the snippets above/SWT code are structured, that the behaviour with a surface/gicon should be different)? 2. Is there something we can do on the client code side (I'm not aware of being able to provide disabled state icons for buttons)? 3. Should SWT use a different API here when passing the button image to GTK+? I believe a revert of the fix for bug 564097 is a no go here.
Looking at GTK+ code, I can't determine why pixbuf images receive extra handling. Possibly pixbuf implementing the GIcon interface is enough to result in the extra handling (greyed out button image). We did however find extra SWT code for ToolItem (as toolbars still have the expected behaviour for disabled items): org.eclipse.swt.widgets.ToolItem.setEnabled(boolean) /** * Enables the receiver if the argument is <code>true</code>, * and disables it otherwise. * <p> * A disabled control is typically * not selectable from the user interface and draws with an * inactive or "grayed" look. * </p> * * @param enabled the new enabled state * * @exception SWTException <ul> * <li>ERROR_WIDGET_DISPOSED - if the receiver has been disposed</li> * <li>ERROR_THREAD_INVALID_ACCESS - if not called from the thread that created the receiver</li> * </ul> */ public void setEnabled (boolean enabled) { checkWidget(); long topHandle = topHandle(); if (this.enabled == enabled) return; this.enabled = enabled; GTK.gtk_widget_set_sensitive(topHandle, enabled); if (!enabled) { if (disabledImage == null) { if (defaultDisableImage == null && image != null) { defaultDisableImage = new Image(getDisplay(), image, SWT.IMAGE_DISABLE); } _setImage(defaultDisableImage); } else { _setImage(disabledImage); } } if (enabled && image != null) _setImage(image); } The extra code seems to be coming from the fix for bug 568771, which is also a regression from 564097. I.e. it looks like the fix for bug 568771 covered only toolbar button images, but not images. IMO we should apply the same fix to Button.setEnabled(). Sravan, Paul, Andrey? The change is probably simple, looking at: https://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=5b6474909194a96accf81dc255ae51cd4c018c34
(In reply to Simeon Andreev from comment #3) > I.e. it looks like the fix for bug 568771 covered only toolbar button > images, but not images. > > IMO we should apply the same fix to Button.setEnabled(). Sounds like there is no other way to restore previous functionality. Please push gerrit with the proposed patch, we can discuss further details on gerrit.
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/183329
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/183378
MenuItem has the same problem, see bug 575028. The remaining callers of ImageList (changed in bug 564097, introducing the bug) are TrayItem, TreeItem and TableItem. Neither TrayItem nor Tray have methods to disable the widgets. Tree/Table can be disabled, but still store a pixbuf for each cell (on top of the surface). The greying-out of TreeItem/TableItem images is still handled correctly.
Gerrit change https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/183329 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=5588d72f38d979494e55b55c11d41a81d55e3488
Works as expected in I20210727-1800, except the image quality, which will be addressed by 575069.
