From 4b0412b94e47be8f2c5480568a36ec71809a148f Mon Sep 17 00:00:00 2001
From: Eric Williams
Date: Fri, 23 Mar 2018 16:12:53 -0400
Subject: Bug 531928: [GTK3] Table editing is broken
The fix for bug 511133 introduced some logic which raises/lowers the
GdkWindows of widgets who are children of Table or Tree. Unforunately,
this breaks certain cases such as Tables/Trees that have more than one
child widget.
Additionally, changing the drawing handle from fixedHandle to handle
meant that some child widgets were not getting their draw events
properly. According to GTK, child widgets with non-native windows should
receive draw events from their parent containers using
gtk_container_propagate_draw(). This patch follows this advice and
connects a Table/Tree's fixedHandle to the draw signal. The incoming
draw events to that fixedHandle are then propagated to the respective
child widgets using gtk_container_propagate_draw().
A reproducer snippet is attached. This patch also fixes a regression
from bug 511133 which caused the file permissions table (right click
file -> properties -> resources) to be empty/contain only one checkbox.
Change-Id: I645a04df6773372aa27a360d3df9a3521437828f
Signed-off-by: Eric Williams <ericwill@redhat.com>---
.../Eclipse SWT PI/gtk/library/os.c | 18 ++++
.../Eclipse SWT PI/gtk/library/os_stats.c | 1 +
.../Eclipse SWT PI/gtk/library/os_stats.h | 1 +
.../gtk/org/eclipse/swt/internal/gtk/OS.java | 15 ++++
.../gtk/org/eclipse/swt/widgets/Composite.java | 44 ++++++++++
.../gtk/org/eclipse/swt/widgets/Control.java | 14 ----
.../gtk/org/eclipse/swt/widgets/Table.java | 32 ++++---
.../gtk/org/eclipse/swt/widgets/TableItem.java | 3 -
.../gtk/org/eclipse/swt/widgets/Tree.java | 32 ++++---
.../gtk/org/eclipse/swt/widgets/TreeItem.java | 3 -
.../Bug531928_TableTreeEditorVisibility.java | 98 ++++++++++++++++++++++
11 files changed, 219 insertions(+), 42 deletions(-)
create mode 100644 tests/org.eclipse.swt.tests.gtk/ManualTests/org/eclipse/swt/tests/gtk/snippets/Bug531928_TableTreeEditorVisibility.java
diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os.c b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os.c
index cb6b841..5fc92d0 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os.c
+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os.c
@@ -8497,6 +8497,24 @@ JNIEXPORT jintLong JNICALL OS_NATIVE(_1gtk_1container_1get_1children)
}
#endif
+#ifndef NO__1gtk_1container_1propagate_1draw
+JNIEXPORT void JNICALL OS_NATIVE(_1gtk_1container_1propagate_1draw)
+ (JNIEnv *env, jclass that, jintLong arg0, jintLong arg1, jintLong arg2)
+{
+ OS_NATIVE_ENTER(env, that, _1gtk_1container_1propagate_1draw_FUNC);
+/*
+ gtk_container_propagate_draw((GtkContainer *)arg0, (GtkWidget *)arg1, (cairo_t *)arg2);
+*/
+ {
+ OS_LOAD_FUNCTION(fp, gtk_container_propagate_draw)
+ if (fp) {
+ ((void (CALLING_CONVENTION*)(GtkContainer *, GtkWidget *, cairo_t *))fp)((GtkContainer *)arg0, (GtkWidget *)arg1, (cairo_t *)arg2);
+ }
+ }
+ OS_NATIVE_EXIT(env, that, _1gtk_1container_1propagate_1draw_FUNC);
+}
+#endif
+
#ifndef NO__1gtk_1container_1remove
JNIEXPORT void JNICALL OS_NATIVE(_1gtk_1container_1remove)
(JNIEnv *env, jclass that, jintLong arg0, jintLong arg1)
diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.c b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.c
index ebfe993..98f36e6 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.c
+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.c
@@ -680,6 +680,7 @@ char * GTK_nativeFunctionNames[] = {
"_1gtk_1container_1forall",
"_1gtk_1container_1get_1border_1width",
"_1gtk_1container_1get_1children",
+ "_1gtk_1container_1propagate_1draw",
"_1gtk_1container_1remove",
"_1gtk_1container_1set_1border_1width",
"_1gtk_1css_1provider_1get_1named",
diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.h b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.h
index f707a39..97f4d14 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.h
+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.h
@@ -690,6 +690,7 @@ typedef enum {
_1gtk_1container_1forall_FUNC,
_1gtk_1container_1get_1border_1width_FUNC,
_1gtk_1container_1get_1children_FUNC,
+ _1gtk_1container_1propagate_1draw_FUNC,
_1gtk_1container_1remove_FUNC,
_1gtk_1container_1set_1border_1width_FUNC,
_1gtk_1css_1provider_1get_1named_FUNC,
diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/OS.java b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/OS.java
index cc80140..78ba8d3 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/OS.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/OS.java
@@ -6915,6 +6915,21 @@ public class GTK extends OS {
lock.unlock();
}
}
+ /**
+ * @method flags=dynamic
+ * @param container cast=(GtkContainer *)
+ * @param child cast=(GtkWidget *)
+ * @param cairo cast=(cairo_t *)
+ */
+ public static final native void _gtk_container_propagate_draw(long /*int*/ container, long /*int*/ child, long /*int*/ cairo);
+ public static final void gtk_container_propagate_draw(long /*int*/ container, long /*int*/ child, long /*int*/ cairo) {
+ lock.lock();
+ try {
+ _gtk_container_propagate_draw(container, child, cairo);
+ } finally {
+ lock.unlock();
+ }
+ }
/** @param container cast=(GtkContainer *) */
public static final native int _gtk_container_get_border_width(long /*int*/ container);
public static final int gtk_container_get_border_width(long /*int*/ container) {
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Composite.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Composite.java
index 57bbf52..bd253c4 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Composite.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Composite.java
@@ -1393,6 +1393,50 @@ void printWidget (GC gc, long /*int*/ drawable, int depth, int x, int y) {
newClip.dispose ();
}
+/**
+ * Connects this widget's fixedHandle to the "draw" signal.<br>
+ * NOTE: only the "draw" (EXPOSE) signal is connected, not EXPOSE_EVENT_INVERSE.
+ */
+void connectFixedHandleDraw () {
+ long /*int*/ paintHandle = fixedHandle;
+ int paintMask = OS.GDK_EXPOSURE_MASK;
+ OS.gtk_widget_add_events (paintHandle, paintMask);
+
+ OS.g_signal_connect_closure_by_id (paintHandle, display.signalIds [DRAW], 0, display.getClosure (DRAW), true);
+}
+
+/**
+ * <p>Propagates draw events from a parent container to its children using
+ * gtk_container_propagate_draw(). This method only works if the fixedHandle
+ * has been connected to the "draw" signal, and only propagates draw events
+ * to other siblings of handle (i.e. other children of fixedHandle, but not
+ * handle itself).</p>
+ *
+ * <p>It's useful to propagate draw events to other child widgets for things
+ * like Table/Tree editors, or other scenarios where a widget is a child of
+ * a non-standard container widget (i.e., not a direct child of a Composite).</p>
+ *
+ * @param container the parent container, i.e. fixedHandle
+ * @param cairo the cairo context provided by GTK
+ */
+void propagateDraw (long /*int*/ container, long /*int*/ cairo) {
+ if (container == fixedHandle && OS.GTK3) {
+ long /*int*/ list = OS.gtk_container_get_children (container);
+ long /*int*/ temp = list;
+ while (temp != 0) {
+ long /*int*/ child = OS.g_list_data (temp);
+ if (child != 0) {
+ Widget widget = display.getWidget (child);
+ if (widget != this) {
+ OS.gtk_container_propagate_draw(container, child, cairo);
+ }
+ }
+ temp = OS.g_list_next (temp);
+ }
+ OS.g_list_free (list);
+ }
+}
+
@Override
void redrawChildren () {
super.redrawChildren ();
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Control.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Control.java
index 7971ff6..f075de8 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Control.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Control.java
@@ -5537,13 +5537,6 @@ public void setVisible (boolean visible) {
state &= ~HIDDEN;
if ((state & (ZERO_WIDTH | ZERO_HEIGHT)) == 0) {
if (enableWindow != 0) OS.gdk_window_show_unraised (enableWindow);
- /*
- * Raise this widget's GdkWindow if the reparentOnVisibility
- * flag has been set and visible is true. See bug 511133.
- */
- if (reparentOnVisibility && OS.GTK3) {
- OS.gdk_window_raise(gtk_widget_get_window(topHandle));
- }
OS.gtk_widget_show (topHandle);
}
} else {
@@ -5580,13 +5573,6 @@ public void setVisible (boolean visible) {
OS.gtk_widget_hide (topHandle);
if (isDisposed ()) return;
if (enableWindow != 0) OS.gdk_window_hide (enableWindow);
- /*
- * Lower this widget's GdkWindow if the reparentOnVisibility
- * flag has been set and visible is false. See bug 511133.
- */
- if (reparentOnVisibility && OS.GTK3) {
- OS.gdk_window_lower(gtk_widget_get_window(topHandle));
- }
sendEvent (SWT.Hide);
}
}
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Table.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Table.java
index a64bdfc..dff41bd 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Table.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Table.java
@@ -85,7 +85,7 @@ public class Table extends Composite {
GdkRGBA background, foreground;
Color headerBackground, headerForeground;
String headerCSSBackground, headerCSSForeground;
- boolean ownerDraw, ignoreSize, ignoreAccessibility, pixbufSizeSet;
+ boolean ownerDraw, ignoreSize, ignoreAccessibility, pixbufSizeSet, hasChildren;
int maxWidth = 0;
int topIndex;
double cachedAdjustment, currentAdjustment;
@@ -3653,18 +3653,18 @@ void setParentGdkWindow (Control child) {
long /*int*/ parentGdkWindow = eventWindow ();
OS.gtk_widget_set_parent_window (child.topHandle(), parentGdkWindow);
/*
- * Feature in GTK3: all children of Table have their GdkWindows
- * re-parented so they are siblings of the parent Table
- * (i.e. on the same level in the z-order).
+ * Feature in GTK3: non-native GdkWindows are not drawn implicitly
+ * as of GTK3.10+. It is the client's responsibility to propagate draw
+ * events to these windows in the "draw" signal handler.
*
- * To fix table editing in GTK3: raise/lower the
- * GdkWindow of these child widgets to make them visible when
- * setVisible() is called on them. This ensures they are properly
- * drawn when setVisible(true) is called, and properly hidden
- * when setVisible(false) is called. See bug 511133.
+ * This change breaks table editing on GTK3.10+, as the table editor
+ * widgets no longer receive draw signals. The fix is to connect the
+ * Table's fixedHandle to the draw signal, and propagate the draw
+ * signal using gtk_container_propagate_draw(). See bug 531928.
*/
- if (child != null && OS.GTK3) {
- child.reparentOnVisibility = true;
+ if (OS.GTK_VERSION >= OS.VERSION(3, 10, 0)) {
+ hasChildren = true;
+ connectFixedHandleDraw();
}
}
@@ -4080,6 +4080,16 @@ void updateScrollBarValue (ScrollBar bar) {
@Override
long /*int*/ windowProc (long /*int*/ handle, long /*int*/ arg0, long /*int*/ user_data) {
switch ((int)/*64*/user_data) {
+ case EXPOSE_EVENT: {
+ /*
+ * If this Table has any child widgets, propagate the draw signal
+ * to them using gtk_container_propagate_draw(). See bug 531928.
+ */
+ if (OS.GTK_VERSION >= OS.VERSION(3, 10, 0) && hasChildren) {
+ propagateDraw(handle, arg0);
+ }
+ break;
+ }
case EXPOSE_EVENT_INVERSE: {
/*
* Feature in GTK. When the GtkTreeView has no items it does not propagate
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TableItem.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TableItem.java
index a1d3b89..c6031d2 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TableItem.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TableItem.java
@@ -331,9 +331,6 @@ Rectangle getBoundsinPixels () {
}
int width = OS.gtk_tree_view_column_get_visible (column) ? rect.width + 1 : 0;
Rectangle r = new Rectangle (rect.x, rect.y, width, rect.height + 1);
- if (parent.getHeaderVisible() && OS.GTK_VERSION > OS.VERSION(3, 9, 0)) {
- r.y += parent.getHeaderHeightInPixels();
- }
return r;
}
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Tree.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Tree.java
index 6644346..1842b84 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Tree.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Tree.java
@@ -91,7 +91,7 @@ public class Tree extends Composite {
int drawState, drawFlags;
GdkColor drawForeground;
GdkRGBA background, foreground;
- boolean ownerDraw, ignoreSize, ignoreAccessibility, pixbufSizeSet;
+ boolean ownerDraw, ignoreSize, ignoreAccessibility, pixbufSizeSet, hasChildren;
int pixbufHeight, pixbufWidth;
TreeItem topItem;
double cachedAdjustment, currentAdjustment;
@@ -3658,18 +3658,18 @@ void setParentGdkWindow (Control child) {
long /*int*/ parentGdkWindow = eventWindow ();
OS.gtk_widget_set_parent_window (child.topHandle(), parentGdkWindow);
/*
- * Feature in GTK3: all children of Tree have their GdkWindows
- * re-parented so they are siblings of the parent Tree
- * (i.e. on the same level in the z-order).
+ * Feature in GTK3: non-native GdkWindows are not drawn implicitly
+ * as of GTK3.10+. It is the client's responsibility to propagate draw
+ * events to these windows in the "draw" signal handler.
*
- * To fix table editing in GTK3: raise/lower the
- * GdkWindow of these child widgets to make them visible when
- * setVisible() is called on them. This ensures they are properly
- * drawn when setVisible(true) is called, and properly hidden
- * when setVisible(false) is called. See bug 511133.
+ * This change breaks tree editing on GTK3.10+, as the tree editor
+ * widgets no longer receive draw signals. The fix is to connect the
+ * Tree's fixedHandle to the draw signal, and propagate the draw
+ * signal using gtk_container_propagate_draw(). See bug 531928.
*/
- if (child != null && OS.GTK3) {
- child.reparentOnVisibility = true;
+ if (OS.GTK_VERSION >= OS.VERSION(3, 10, 0)) {
+ hasChildren = true;
+ connectFixedHandleDraw();
}
}
@@ -4010,6 +4010,16 @@ void updateScrollBarValue (ScrollBar bar) {
@Override
long /*int*/ windowProc (long /*int*/ handle, long /*int*/ arg0, long /*int*/ user_data) {
switch ((int)/*64*/user_data) {
+ case EXPOSE_EVENT: {
+ /*
+ * If this Tree has any child widgets, propagate the draw signal
+ * to them using gtk_container_propagate_draw(). See bug 531928.
+ */
+ if (OS.GTK_VERSION >= OS.VERSION(3, 10, 0) && hasChildren) {
+ propagateDraw(handle, arg0);
+ }
+ break;
+ }
case EXPOSE_EVENT_INVERSE: {
/*
* Feature in GTK. When the GtkTreeView has no items it does not propagate
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TreeItem.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TreeItem.java
index 03b847c..66307f8 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TreeItem.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TreeItem.java
@@ -517,9 +517,6 @@ Rectangle getBoundsInPixels () {
}
int width = OS.gtk_tree_view_column_get_visible (column) ? rect.width + 1 : 0;
Rectangle r = new Rectangle (rect.x, rect.y, width, rect.height + 1);
- if (parent.getHeaderVisible() && OS.GTK_VERSION > OS.VERSION(3, 9, 0)) {
- r.y += parent.getHeaderHeightInPixels();
- }
return r;
}
diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_custom.h b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_custom.h
index ddb326e..43acdb3 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_custom.h
+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_custom.h
@@ -255,6 +255,8 @@
#define gdk_cairo_create_LIB LIB_GDK
#define gdk_colormap_alloc_color_LIB LIB_GDK
#define gdk_colormap_free_colors_LIB LIB_GDK
+#define gtk_container_propagate_draw_LIB LIB_GTK
+#define gtk_widget_reparent_LIB LIB_GTK
#define gtk_paint_box_LIB LIB_GTK
#define gtk_paint_handle_LIB LIB_GTK
#define gtk_paint_focus_LIB LIB_GTK