Bug 475784: half transparent composite as overlay composite not work

with gtk3

ATTEMPT #2: fix regression from original patch. Instead of keeping the
NO_BACKGROUND flag, cache it in Control and remove it as intended in
Composite.checkStyle().

Original commit message:
We have to handle some corner case drawing situations, since
Control.setRegion() now uses Cairo entirely. In particular, we need to
handle the case where a user is using an SWT.NO_BACKGROUND Composite as
an overlay. This means the Composite has a region set, and a
paintListener attached to perform custom drawing.

Previously, Control.setRegion() "physically" manipulate the Composite's
GdkWindow to prevent any drawing outside the given region. Since Cairo
is now responsible for drawing the set region we have to handle this
geometry ourselves.

This patch fixes numerous issues all rolled into one. First, it adds a
field in GCData which corresponds to the Cairo region. This is set in
gtk_draw() so that GC.fill* methods will clip themselves accordingly and
not fill where they aren't supposed to.

Second, it checks for a PaintListener, NO_BACKGROUND, and a region being
set. If these conditions are met, we draw a transparent background in
Control.drawBackground() to ensure the region and custom drawing
succeeds.

Last, we copy the region at the Cairo level to ensure there are no
crashes when calling setRegion(region) followed by region.dispose().

Tested on GTK3.22 using the snippet attached and a child Eclipse. No
AllNonBrowser JUnit tests fail.

Change-Id: I6a6245052e9756d6c9bdd0e46ef20a74413acc6b
Signed-off-by: Eric Williams <ericwill@redhat.com>

11 files changed, 123 insertions, 8 deletions

diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/cairo/library/cairo.c b/bundles/org.eclipse.swt/Eclipse SWT PI/cairo/library/cairo.c
index dc39bccf20..9733cccad0 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT PI/cairo/library/cairo.c
+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/cairo/library/cairo.c
@@ -875,6 +875,26 @@ JNIEXPORT jboolean JNICALL Cairo_NATIVE(_1cairo_1region_1contains_1point)
 }
 #endif
 
+#ifndef NO__1cairo_1region_1copy
+JNIEXPORT jintLong JNICALL Cairo_NATIVE(_1cairo_1region_1copy)
+	(JNIEnv *env, jclass that, jintLong arg0)
+{
+	jintLong rc = 0;
+	Cairo_NATIVE_ENTER(env, that, _1cairo_1region_1copy_FUNC);
+/*
+	rc = (jintLong)cairo_region_copy(arg0);
+*/
+	{
+		Cairo_LOAD_FUNCTION(fp, cairo_region_copy)
+		if (fp) {
+			rc = (jintLong)((jintLong (CALLING_CONVENTION*)(jintLong))fp)(arg0);
+		}
+	}
+	Cairo_NATIVE_EXIT(env, that, _1cairo_1region_1copy_FUNC);
+	return rc;
+}
+#endif
+
 #ifndef NO__1cairo_1region_1get_1rectangle
 JNIEXPORT void JNICALL Cairo_NATIVE(_1cairo_1region_1get_1rectangle)
 	(JNIEnv *env, jclass that, jintLong arg0, jint arg1, jintLong arg2)
diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/cairo/library/cairo_custom.h b/bundles/org.eclipse.swt/Eclipse SWT PI/cairo/library/cairo_custom.h
index 2554ab6068..fecaef595a 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT PI/cairo/library/cairo_custom.h
+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/cairo/library/cairo_custom.h
@@ -32,6 +32,7 @@
 #define cairo_ps_surface_set_size_LIB LIB_CAIRO
 #define cairo_region_num_rectangles_LIB LIB_CAIRO
 #define cairo_region_contains_point_LIB LIB_CAIRO
+#define cairo_region_copy_LIB LIB_CAIRO
 #define cairo_region_get_rectangle_LIB LIB_CAIRO
 #define cairo_surface_set_device_scale_LIB LIB_CAIRO
 #define cairo_surface_get_device_scale_LIB LIB_CAIRO
diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/cairo/library/cairo_stats.c b/bundles/org.eclipse.swt/Eclipse SWT PI/cairo/library/cairo_stats.c
index 68d31d2c6a..4153701dea 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT PI/cairo/library/cairo_stats.c
+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/cairo/library/cairo_stats.c
@@ -97,6 +97,7 @@ char * Cairo_nativeFunctionNames[] = {
 	"_1cairo_1rectangle",
 	"_1cairo_1reference",
 	"_1cairo_1region_1contains_1point",
+	"_1cairo_1region_1copy",
 	"_1cairo_1region_1get_1rectangle",
 	"_1cairo_1region_1num_1rectangles",
 	"_1cairo_1reset_1clip",
diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/cairo/library/cairo_stats.h b/bundles/org.eclipse.swt/Eclipse SWT PI/cairo/library/cairo_stats.h
index 13f3005169..1eae730340 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT PI/cairo/library/cairo_stats.h
+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/cairo/library/cairo_stats.h
@@ -107,6 +107,7 @@ typedef enum {
 	_1cairo_1rectangle_FUNC,
 	_1cairo_1reference_FUNC,
 	_1cairo_1region_1contains_1point_FUNC,
+	_1cairo_1region_1copy_FUNC,
 	_1cairo_1region_1get_1rectangle_FUNC,
 	_1cairo_1region_1num_1rectangles_FUNC,
 	_1cairo_1reset_1clip_FUNC,
diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/cairo/library/cairo_structs.c b/bundles/org.eclipse.swt/Eclipse SWT PI/cairo/library/cairo_structs.c
index a54a37e80d..6f5afd45c9 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT PI/cairo/library/cairo_structs.c
+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/cairo/library/cairo_structs.c
@@ -15,7 +15,7 @@
  *
  * IBM
  * -  Binding to permit interfacing between Cairo and SWT
- * -  Copyright (C) 2005, 2016 IBM Corp.  All Rights Reserved.
+ * -  Copyright (C) 2005, 2018 IBM Corp.  All Rights Reserved.
  *
  * ***** END LICENSE BLOCK ***** */
 
diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/cairo/library/cairo_structs.h b/bundles/org.eclipse.swt/Eclipse SWT PI/cairo/library/cairo_structs.h
index 6ffd32cb80..5faf6e80d8 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT PI/cairo/library/cairo_structs.h
+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/cairo/library/cairo_structs.h
@@ -15,7 +15,7 @@
  *
  * IBM
  * -  Binding to permit interfacing between Cairo and SWT
- * -  Copyright (C) 2005, 2016 IBM Corp.  All Rights Reserved.
+ * -  Copyright (C) 2005, 2018 IBM Corp.  All Rights Reserved.
  *
  * ***** END LICENSE BLOCK ***** */
 
diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/cairo/org/eclipse/swt/internal/cairo/Cairo.java b/bundles/org.eclipse.swt/Eclipse SWT PI/cairo/org/eclipse/swt/internal/cairo/Cairo.java
index 27918325ef..2af4e020fa 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT PI/cairo/org/eclipse/swt/internal/cairo/Cairo.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/cairo/org/eclipse/swt/internal/cairo/Cairo.java
@@ -1276,6 +1276,16 @@ public static final int cairo_region_num_rectangles(long /*int*/ region) {
 	}
 }
 /** @method flags=dynamic */
+public static final native long /*int*/ _cairo_region_copy(long /*int*/ region);
+public static final long /*int*/ cairo_region_copy(long /*int*/ region) {
+	lock.lock();
+	try {
+		 return _cairo_region_copy(region);
+	} finally {
+		lock.unlock();
+	}
+}
+/** @method flags=dynamic */
 public static final native boolean _cairo_region_contains_point(long /*int*/ region, int x, int y);
 public static final boolean cairo_region_contains_point(long /*int*/ region, int x, int y) {
 	lock.lock();
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/GC.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/GC.java
index 127e1e216e..7a4642f963 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/GC.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/GC.java
@@ -206,6 +206,23 @@ static int checkStyle (int style) {
 }
 
 /**
+ * Convenience method that applies a region to the Control using cairo_clip.
+ *
+ * @param cairo the cairo context to apply the region to
+ */
+void cairoClipRegion (long /*int*/ cairo) {
+	if (cairo == 0) return;
+	GdkRectangle rect = new GdkRectangle ();
+	GDK.gdk_cairo_get_clip_rectangle (cairo, rect);
+	long /*int*/ regionHandle = data.regionSet;
+	long /*int*/ actualRegion = GDK.gdk_region_rectangle(rect);
+	GDK.gdk_region_subtract(actualRegion, regionHandle);
+	GDK.gdk_cairo_region(cairo, actualRegion);
+	Cairo.cairo_clip(cairo);
+	Cairo.cairo_paint(cairo);
+}
+
+/**
  * Invokes platform specific functionality to wrap a graphics context.
  * <p>
  * <b>IMPORTANT:</b> This method is <em>not</em> part of the public
@@ -1741,7 +1758,11 @@ void fillRectangleInPixels(int x, int y, int width, int height) {
 		height = -height;
 	}
 	long /*int*/ cairo = data.cairo;
-	Cairo.cairo_rectangle(cairo, x, y, width, height);
+	if (data.regionSet != 0) {
+		cairoClipRegion(cairo);
+	} else {
+		Cairo.cairo_rectangle(cairo, x, y, width, height);
+	}
 	Cairo.cairo_fill(cairo);
 }
 
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/GCData.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/GCData.java
index b6e8b7a799..f9fa5a7efe 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/GCData.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/GCData.java
@@ -69,7 +69,7 @@ public final class GCData {
 	public int interpolation = SWT.DEFAULT;
 	public Image image;
 
-	public long /*int*/ clipRgn, context, layout, damageRgn, drawable, cairo;
+	public long /*int*/ clipRgn, context, layout, damageRgn, drawable, cairo, regionSet;
 	public double cairoXoffset, cairoYoffset;
 	public boolean disposeCairo;
 	public double[] identity, clippingTransform;
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 cf786af88b..6aebd3ac7b 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
@@ -124,6 +124,13 @@ Composite () {
  */
 public Composite (Composite parent, int style) {
 	super (parent, checkStyle (style));
+	/*
+	 * Cache the NO_BACKGROUND flag for use in the Cairo setRegion()
+	 * implementation. Only relevant to GTK3.10+, see bug 475784.
+	 */
+	if (GTK.GTK_VERSION >= OS.VERSION(3, 10, 0) && (style & SWT.NO_BACKGROUND) != 0) {
+		cachedNoBackground = true;
+	}
 }
 
 static int checkStyle (int style) {
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 ed9b3ae3f4..05167c35ac 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
@@ -60,6 +60,11 @@ public abstract class Control extends Widget implements Drawable {
 	Font font;
 	Region region;
 	long /*int*/ eventRegion;
+	/**
+	 * The handle to the Region, which is neccessary in the case
+	 * that <code>region</code> is disposed before this Control.
+	 */
+	long /*int*/ regionHandle;
 	String toolTipText;
 	Object layoutData;
 	Accessible accessible;
@@ -67,6 +72,12 @@ public abstract class Control extends Widget implements Drawable {
 	String cssBackground, cssForeground = " ";
 	boolean drawRegion;
 	/**
+	 * Cache the NO_BACKGROUND flag as it gets removed automatically in
+	 * Composite. Only relevant for GTK3.10+ as we need it for Cairo setRegion()
+	 * functionality. See bug 475784.
+	 */
+	boolean cachedNoBackground;
+	/**
 	 * Point for storing the (x, y) coordinate of the last input (click/scroll/etc.).
 	 * This is useful for checking input event coordinates in methods that act on input,
 	 * but do not receive coordinates (like gtk_clicked, for example). See bug 529431.
@@ -172,6 +183,13 @@ void drawBackground (Control control, long /*int*/ window, long /*int*/ region,
 
 void drawBackground (Control control, long /*int*/ window, long /*int*/ cr, long /*int*/ region, int x, int y, int width, int height) {
 	long /*int*/ cairo = cr != 0 ? cr : GDK.gdk_cairo_create(window);
+	/*
+	 * It's possible that a client is using an SWT.NO_BACKGROUND Composite with custom painting
+	 * and a region to provide "overlay" functionality. In this case we don't want to paint
+	 * any background color, as it will likely break desired behavior. The fix is to paint
+	 * this Control as transparent. See bug 475784.
+	 */
+	boolean noBackgroundRegion = drawRegion && hooks(SWT.Paint) && cachedNoBackground;
 	if (cairo == 0) error (SWT.ERROR_NO_HANDLES);
 	if (region != 0) {
 		GDK.gdk_cairo_region(cairo, region);
@@ -195,7 +213,11 @@ void drawBackground (Control control, long /*int*/ window, long /*int*/ cr, long
 		GdkRGBA rgba;
 		if (GTK.GTK3) {
 			rgba = control.getBackgroundGdkRGBA();
-			Cairo.cairo_set_source_rgba (cairo, rgba.red, rgba.green, rgba.blue, rgba.alpha);
+			if (noBackgroundRegion) {
+				Cairo.cairo_set_source_rgba (cairo, 0.0, 0.0, 0.0, 0.0);
+			} else {
+				Cairo.cairo_set_source_rgba (cairo, rgba.red, rgba.green, rgba.blue, rgba.alpha);
+			}
 		} else {
 			GdkColor color = control.getBackgroundGdkColor ();
 			Cairo.cairo_set_source_rgba_compatibility (cairo, color);
@@ -1376,6 +1398,11 @@ public void setRegion (Region region) {
 		GDK.gdk_window_shape_combine_region (window, shape_region, 0, 0);
 	} else {
 		drawRegion = (this.region != null && this.region.handle != 0);
+		if (drawRegion) {
+			cairoCopyRegion(this.region);
+		} else {
+			cairoDisposeRegion();
+		}
 		GTK.gtk_widget_queue_draw(topHandle());
 	}
 }
@@ -3580,6 +3607,24 @@ long /*int*/ gtk_event_after (long /*int*/ widget, long /*int*/ gdkEvent) {
 }
 
 /**
+ * Copies the region at the Cairo level, as we need to re-use these resources
+ * after the Region object is disposed.
+ *
+ * @param region the Region object to copy to this Control
+ */
+void cairoCopyRegion (Region region) {
+	if (region == null || region.isDisposed() || region.handle == 0) return;
+	regionHandle = Cairo.cairo_region_copy(region.handle);
+	return;
+}
+
+void cairoDisposeRegion () {
+	if (regionHandle != 0) GDK.gdk_region_destroy(regionHandle);
+	if (eventRegion != 0) GDK.gdk_region_destroy(eventRegion);
+	regionHandle = 0;
+	eventRegion = 0;
+}
+/**
  * Convenience method that applies a region to the Control using cairo_clip.
  *
  * @param cairo the cairo context to apply the region to
@@ -3587,13 +3632,16 @@ long /*int*/ gtk_event_after (long /*int*/ widget, long /*int*/ gdkEvent) {
 void cairoClipRegion (long /*int*/ cairo) {
 	GdkRectangle rect = new GdkRectangle ();
 	GDK.gdk_cairo_get_clip_rectangle (cairo, rect);
-	long /*int*/ regionHandle = this.region.handle;
-	GdkRectangle regionRect = new GdkRectangle();
+	long /*int*/ regionHandle = this.regionHandle;
+	// Disposal check just in case
+	if (regionHandle == 0) {
+		drawRegion = false;
+		return;
+	}
 	/*
 	 * These gdk_region_* functions actually map to the proper
 	 * cairo_* functions in os.h.
 	 */
-	GDK.gdk_region_get_clipbox(regionHandle, regionRect);
 	long /*int*/ actualRegion = GDK.gdk_region_rectangle(rect);
 	GDK.gdk_region_subtract(actualRegion, regionHandle);
 	// Draw the Shell bg using cairo, only if it's a different color
@@ -3631,6 +3679,11 @@ long /*int*/ gtk_draw (long /*int*/ widget, long /*int*/ cairo) {
 	if ((style & SWT.MIRRORED) != 0) eventBounds.x = DPIUtil.autoScaleDown (getClientWidth ()) - eventBounds.width - eventBounds.x;
 	event.setBounds (eventBounds);
 	GCData data = new GCData ();
+	/*
+	 * Pass the region into the GCData so that GC.fill* methods can be aware of the region
+	 * and clip themselves accordingly. Only relevant on GTK3.10+, see bug 475784.
+	 */
+	if (drawRegion) data.regionSet = eventRegion;
 //	data.damageRgn = gdkEvent.region;
 	if (GTK.GTK_VERSION <= OS.VERSION (3, 9, 0) || (GTK.GTK_VERSION >= OS.VERSION (3, 14, 0) && OS.CAIRO_CONTEXT_REUSE)) {
 		data.cairo = cairo;
@@ -4310,6 +4363,7 @@ void releaseHandle () {
 	super.releaseHandle ();
 	fixedHandle = 0;
 	parent = null;
+	cairoDisposeRegion();
 }
 
 @Override