391050 – Replace GdkRegion with Cairo methods for GTK + 3.0 and higher

Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 391050 - Replace GdkRegion with Cairo methods for GTK + 3.0 and higher

Summary: Replace GdkRegion with Cairo methods for GTK + 3.0 and higher

Status:	RESOLVED FIXED

Alias:	None

Product:	Platform
Classification:	Eclipse Project
Component:	SWT (show other bugs)
Version:	4.2
Hardware:	PC Linux

Importance:	P3 normal (vote)
Target Milestone:	4.3 M3
Assignee:	Silenio Quarti
QA Contact:

URL:
Whiteboard:
Keywords:

Duplicates (8):	389056 389066 389085 389093 389096 389727 389729 389741 (view as bug list)
Depends on:
Blocks:	340067
	Show dependency tree

Reported:	2012-10-03 13:40 EDT by Anatoly Spektor
Modified:	2012-10-24 14:38 EDT (History)
CC List:	2 users (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Anatoly Spektor

2012-10-03 13:40:26 EDT

To avoid dependency issues I decided to put all GdkRegion  methods in one place.

(I will mark as duplicates all the patches regarding GdkRegion that I opened before this bug)

To Note: As In Gtk3 there is typedef  cairo_rectaingle_int_t GdkRectangle, I have used GdkRectangle for all the operations that required cairo_rectangle_int_t 


Patch can be found here:

http://fedorapeople.org/cgit/aspektor/public_git/eclipse.platform.swt.git/commit/?h=cairo_region_create_64

Comment 1 Anatoly Spektor

2012-10-03 13:41:38 EDT

*** Bug 389056 has been marked as a duplicate of this bug. ***

Comment 2 Anatoly Spektor

2012-10-03 13:42:26 EDT

*** Bug 389096 has been marked as a duplicate of this bug. ***

Comment 3 Anatoly Spektor

2012-10-03 13:42:56 EDT

*** Bug 389066 has been marked as a duplicate of this bug. ***

Comment 4 Anatoly Spektor

2012-10-03 13:43:20 EDT

*** Bug 389741 has been marked as a duplicate of this bug. ***

Comment 5 Anatoly Spektor

2012-10-03 13:43:45 EDT

*** Bug 389729 has been marked as a duplicate of this bug. ***

Comment 6 Anatoly Spektor

2012-10-03 13:44:14 EDT

*** Bug 389727 has been marked as a duplicate of this bug. ***

Comment 7 Anatoly Spektor

2012-10-03 13:44:34 EDT

*** Bug 389085 has been marked as a duplicate of this bug. ***

Comment 8 Anatoly Spektor

2012-10-03 13:45:02 EDT

*** Bug 389093 has been marked as a duplicate of this bug. ***

Comment 9 Alexander Kurtakov

2012-10-03 13:49:50 EDT

Silenio, would you please review this patch? It can't be tested until we can compile against gtk3 but it looks good to me and I think it should work this way.

Comment 10 Silenio Quarti

2012-10-09 11:17:46 EDT

- The new natives in Cairo have to be dynamic
- Region.add(int[]) still uses gtk_region_polygon()
- cairo.c only has the implementation for one native.
- using GdkRectangle as a replacement for cairo_rectangle_int_t should be ok as both structures are exactly the same.  But this is not going to work because the structs helpers for GdkRectangle (os_structs.c) are define in another library.  I do think the JNI generator handles that well and it is probably why there are missing natives in cairo.c .

Comment 11 Alexander Kurtakov

2012-10-09 11:35:27 EDT

(In reply to comment #10)
> - The new natives in Cairo have to be dynamic
> - Region.add(int[]) still uses gtk_region_polygon()
> - cairo.c only has the implementation for one native.
> - using GdkRectangle as a replacement for cairo_rectangle_int_t should be ok
> as both structures are exactly the same.  But this is not going to work
> because the structs helpers for GdkRectangle (os_structs.c) are define in
> another library.  I do think the JNI generator handles that well and it is
> probably why there are missing natives in cairo.c .

So we should create cairo_rectangle_int_t in org.eclipse.swt.internal.cairo and start using it with if/else?

Comment 12 Alexander Kurtakov

2012-10-09 11:37:58 EDT

Or just duplicating the struct helpers in cairo_structs.c should work?

Comment 13 Silenio Quarti

2012-10-09 12:00:09 EDT

cairo_structs.c is generated. We would not be able to add the helpers manually. I think the only option is to add cairo_rectangle_int_t.

Comment 14 Anatoly Spektor

2012-10-11 16:42:24 EDT

Ok, the patch is redone, and new version is available here:

http://fedorapeople.org/cgit/aspektor/public_git/eclipse.platform.swt.git/commit/?h=gtk3_region_64


Silenio, could you please take a look at revised patch and tell me what you think.

Thanks in advance!

Comment 15 Anatoly Spektor

2012-10-17 10:22:07 EDT

(In reply to comment #10)
> - The new natives in Cairo have to be dynamic
> - Region.add(int[]) still uses gtk_region_polygon()
> - cairo.c only has the implementation for one native.
> - using GdkRectangle as a replacement for cairo_rectangle_int_t should be ok
> as both structures are exactly the same.  But this is not going to work
> because the structs helpers for GdkRectangle (os_structs.c) are define in
> another library.  I do think the JNI generator handles that well and it is
> probably why there are missing natives in cairo.c .

As this patch is an important milestone in the migration proccess, and as some of the other deprecated methods are dependant on this patch to be reviewed and applied, I would kindly ask SWT committers to review it as soon as possible, thus giving the opportunity to myself to move forward in migration proccess.

Thank you for your time and understanding.

Anatoly

Comment 16 Alexander Kurtakov

2012-10-17 10:30:08 EDT

(In reply to comment #15)
> (In reply to comment #10)
> > - The new natives in Cairo have to be dynamic
> > - Region.add(int[]) still uses gtk_region_polygon()
> > - cairo.c only has the implementation for one native.
> > - using GdkRectangle as a replacement for cairo_rectangle_int_t should be ok
> > as both structures are exactly the same.  But this is not going to work
> > because the structs helpers for GdkRectangle (os_structs.c) are define in
> > another library.  I do think the JNI generator handles that well and it is
> > probably why there are missing natives in cairo.c .
> 
> As this patch is an important milestone in the migration proccess, and as
> some of the other deprecated methods are dependant on this patch to be
> reviewed and applied, I would kindly ask SWT committers to review it as soon
> as possible, thus giving the opportunity to myself to move forward in
> migration proccess.
> 
> Thank you for your time and understanding.
> 
> Anatoly

It looks good to me but I would appreciate someone else confirming it too.

Comment 17 Silenio Quarti

2012-10-17 11:33:11 EDT

Pushed the patch to eclipse.org

http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=b178ded274cefa13fff8fc6fb8115ee14a844dc8

Fixed a few memory leaks and formatting:

http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=57c0e797a17f6a1fdd2b2a079dc4b2435e2d55ee

Our build is broken because cairo_rectangle_int_t does not exists on RHEL4 (or RHEL5). Investigating a solution.

Comment 18 Silenio Quarti

2012-10-17 11:57:46 EDT

Pushed fix for missing struct:

http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=248b732f3c6ebb8e46d26db5933179a833868159

Comment 19 Anatoly Spektor

2012-10-17 12:46:52 EDT

(In reply to comment #17)
> Pushed the patch to eclipse.org
> 
> http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/
> ?id=b178ded274cefa13fff8fc6fb8115ee14a844dc8
> 
> Fixed a few memory leaks and formatting:
> 
> http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/
> ?id=57c0e797a17f6a1fdd2b2a079dc4b2435e2d55ee
> 
> Our build is broken because cairo_rectangle_int_t does not exists on RHEL4
> (or RHEL5). Investigating a solution.

Silenio, after pulling the patch I got problems with os.c, it shows difference between JNI generated os.c and the one GdkRegion have.

difference:

my freshly generated os.c lines 6430-6433:

	if (arg1) if ((lparg1 = &_arg1) == NULL) goto fail;
/*
	gdk_region_get_clipbox(arg0, (GdkRectangle *)lparg1);
*/


The one you pushed:

	if (arg1) if ((lparg1 = getGdkRectangleFields(env, arg1, &_arg1)) == NULL) goto fail;
/*
	gdk_region_get_clipbox(arg0, lparg1);
*/

Comment 20 Silenio Quarti

2012-10-17 12:51:32 EDT

I forgot to regenerate the C code. Fixed now. Thanks.

Comment 21 Silenio Quarti

2012-10-17 13:37:33 EDT

We are getting errors while compiling the 32 bit version of GTK:

    [javac] Compiling 609 source files
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\graphics\Region.java:116: cairo_move_to(int,double,double) in org.eclipse.swt.internal.cairo.Cairo cannot be applied to (long,int,int)
    [javac] 		 		 Cairo.cairo_move_to(cairo, pointArray[0], pointArray[1]);
    [javac] 		 		      ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\graphics\Region.java:118: cairo_move_to(int,double,double) in org.eclipse.swt.internal.cairo.Cairo cannot be applied to (long,double,double)
    [javac] 		 		 		 Cairo.cairo_move_to(cairo, pointArray[j]+0.5, pointArray[j+1]+0.5);
    [javac] 		 		 		      ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\graphics\Region.java:120: cairo_close_path(int) in org.eclipse.swt.internal.cairo.Cairo cannot be applied to (long)
    [javac] 		 		 Cairo.cairo_close_path(cairo);
    [javac] 		 		      ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\graphics\Region.java:121: cairo_set_fill_rule(int,int) in org.eclipse.swt.internal.cairo.Cairo cannot be applied to (long,int)
    [javac] 		 		 Cairo.cairo_set_fill_rule(cairo, Cairo.CAIRO_FILL_RULE_EVEN_ODD);
    [javac] 		 		      ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\graphics\Region.java:122: cairo_fill(int) in org.eclipse.swt.internal.cairo.Cairo cannot be applied to (long)
    [javac] 		 		 Cairo.cairo_fill(cairo);
    [javac] 		 		      ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\graphics\Region.java:123: cairo_get_target(int) in org.eclipse.swt.internal.cairo.Cairo cannot be applied to (long)
    [javac] 		 		 long /*ing*/ surface = Cairo.cairo_get_target(cairo);
    [javac] 		 		                             ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\graphics\Region.java:124: gdk_cairo_region_create_from_surface(int) in org.eclipse.swt.internal.gtk.OS cannot be applied to (long)
    [javac] 		 		 int /*long*/ polyRgn = OS.gdk_cairo_region_create_from_surface(surface);
    [javac] 		 		                          ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\graphics\Region.java:126: cairo_destroy(int) in org.eclipse.swt.internal.cairo.Cairo cannot be applied to (long)
    [javac] 		 		 Cairo.cairo_destroy(cairo);
    [javac] 		 		      ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\graphics\Region.java:544: cairo_move_to(int,double,double) in org.eclipse.swt.internal.cairo.Cairo cannot be applied to (long,int,int)
    [javac] 		 		 Cairo.cairo_move_to(cairo, pointArray[0], pointArray[1]);
    [javac] 		 		      ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\graphics\Region.java:546: cairo_move_to(int,double,double) in org.eclipse.swt.internal.cairo.Cairo cannot be applied to (long,double,double)
    [javac] 		 		 		 Cairo.cairo_move_to(cairo, pointArray[j]+0.5, pointArray[j+1]+0.5);
    [javac] 		 		 		      ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\graphics\Region.java:548: cairo_close_path(int) in org.eclipse.swt.internal.cairo.Cairo cannot be applied to (long)
    [javac] 		 		 Cairo.cairo_close_path(cairo);
    [javac] 		 		      ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\graphics\Region.java:549: cairo_set_fill_rule(int,int) in org.eclipse.swt.internal.cairo.Cairo cannot be applied to (long,int)
    [javac] 		 		 Cairo.cairo_set_fill_rule(cairo, Cairo.CAIRO_FILL_RULE_EVEN_ODD);
    [javac] 		 		      ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\graphics\Region.java:550: cairo_fill(int) in org.eclipse.swt.internal.cairo.Cairo cannot be applied to (long)
    [javac] 		 		 Cairo.cairo_fill(cairo);
    [javac] 		 		      ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\graphics\Region.java:551: cairo_get_target(int) in org.eclipse.swt.internal.cairo.Cairo cannot be applied to (long)
    [javac] 		 		 long /*ing*/ surface = Cairo.cairo_get_target(cairo);
    [javac] 		 		                             ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\graphics\Region.java:552: gdk_cairo_region_create_from_surface(int) in org.eclipse.swt.internal.gtk.OS cannot be applied to (long)
    [javac] 		 		 int /*long*/ polyRgn = OS.gdk_cairo_region_create_from_surface(surface);
    [javac] 		 		                          ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\graphics\Region.java:554: cairo_destroy(int) in org.eclipse.swt.internal.cairo.Cairo cannot be applied to (long)
    [javac] 		 		 Cairo.cairo_destroy(cairo);
    [javac] 		 		      ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\graphics\GC.java:461: cairo_move_to(int,double,double) in org.eclipse.swt.internal.cairo.Cairo cannot be applied to (long,int,int)
    [javac] 		 		 		 		 Cairo.cairo_move_to(cairo, pointArray[0], pointArray[1]);
    [javac] 		 		 		 		      ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\graphics\GC.java:463: cairo_move_to(int,double,double) in org.eclipse.swt.internal.cairo.Cairo cannot be applied to (long,double,double)
    [javac] 		 		 		 		 		 Cairo.cairo_move_to(cairo, pointArray[j]+0.5, pointArray[j+1]+0.5);
    [javac] 		 		 		 		 		      ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\graphics\GC.java:465: cairo_close_path(int) in org.eclipse.swt.internal.cairo.Cairo cannot be applied to (long)
    [javac] 		 		 		 		 Cairo.cairo_close_path(cairo);
    [javac] 		 		 		 		      ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\graphics\GC.java:466: cairo_set_fill_rule(int,int) in org.eclipse.swt.internal.cairo.Cairo cannot be applied to (long,int)
    [javac] 		 		 		 		 Cairo.cairo_set_fill_rule(cairo, Cairo.CAIRO_FILL_RULE_EVEN_ODD);
    [javac] 		 		 		 		      ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\graphics\GC.java:467: cairo_fill(int) in org.eclipse.swt.internal.cairo.Cairo cannot be applied to (long)
    [javac] 		 		 		 		 Cairo.cairo_fill(cairo);
    [javac] 		 		 		 		      ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\graphics\GC.java:468: cairo_get_target(int) in org.eclipse.swt.internal.cairo.Cairo cannot be applied to (long)
    [javac] 		 		 		 		 long /*ing*/ surface = Cairo.cairo_get_target(cairo);
    [javac] 		 		 		 		                             ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\graphics\GC.java:469: gdk_cairo_region_create_from_surface(int) in org.eclipse.swt.internal.gtk.OS cannot be applied to (long)
    [javac] 		 		 		 		 int /*long*/ polyRgn = OS.gdk_cairo_region_create_from_surface(surface);
    [javac] 		 		 		 		                          ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\graphics\GC.java:471: cairo_destroy(int) in org.eclipse.swt.internal.cairo.Cairo cannot be applied to (long)
    [javac] 		 		 		 		 Cairo.cairo_destroy(cairo);
    [javac] 		 		 		 		      ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\widgets\ToolTip.java:260: gdk_cairo_create(int) in org.eclipse.swt.internal.gtk.OS cannot be applied to (long)
    [javac] 		 		  long /*ing*/ cairo = OS.gdk_cairo_create(window);
    [javac] 		 		                         ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\widgets\ToolTip.java:263: cairo_move_to(int,double,double) in org.eclipse.swt.internal.cairo.Cairo cannot be applied to (long,int,int)
    [javac] 		 		  Cairo.cairo_move_to(cairo, polyline[0], polyline[1]);
    [javac] 		 		       ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\widgets\ToolTip.java:265: cairo_move_to(int,double,double) in org.eclipse.swt.internal.cairo.Cairo cannot be applied to (long,double,double)
    [javac] 		 		 		  Cairo.cairo_move_to(cairo, polyline[j]+0.5, polyline[j+1]+0.5);
    [javac] 		 		 		       ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\widgets\ToolTip.java:267: cairo_close_path(int) in org.eclipse.swt.internal.cairo.Cairo cannot be applied to (long)
    [javac] 		 		  Cairo.cairo_close_path(cairo);
    [javac] 		 		       ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\widgets\ToolTip.java:268: cairo_set_fill_rule(int,int) in org.eclipse.swt.internal.cairo.Cairo cannot be applied to (long,int)
    [javac] 		 		  Cairo.cairo_set_fill_rule(cairo, Cairo.CAIRO_FILL_RULE_EVEN_ODD);
    [javac] 		 		       ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\widgets\ToolTip.java:269: cairo_fill(int) in org.eclipse.swt.internal.cairo.Cairo cannot be applied to (long)
    [javac] 		 		  Cairo.cairo_fill(cairo);
    [javac] 		 		       ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\widgets\ToolTip.java:270: cairo_get_target(int) in org.eclipse.swt.internal.cairo.Cairo cannot be applied to (long)
    [javac] 		 		  long /*ing*/ surface = Cairo.cairo_get_target(cairo);
    [javac] 		 		                              ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\widgets\ToolTip.java:271: gdk_cairo_region_create_from_surface(int) in org.eclipse.swt.internal.gtk.OS cannot be applied to (long)
    [javac] 		 		  int /*long*/ region = OS.gdk_cairo_region_create_from_surface(surface);
    [javac] 		 		                          ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\widgets\ToolTip.java:272: gtk_widget_shape_combine_region(int,int) in org.eclipse.swt.internal.gtk.OS cannot be applied to (long,int)
    [javac] 		 		  OS.gtk_widget_shape_combine_region (window, region);
    [javac] 		 		    ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\widgets\ToolTip.java:273: cairo_destroy(int) in org.eclipse.swt.internal.cairo.Cairo cannot be applied to (long)
    [javac] 		 		  Cairo.cairo_destroy(cairo);
    [javac] 		 		       ^
    [javac] C:\BUILD\swt-builddir\repos\master\tmp\check.compile.master\build\org\eclipse\swt\widgets\ToolTip.java:276: gdk_window_shape_combine_region(int,int,int,int) in org.eclipse.swt.internal.gtk.OS cannot be applied to (long,int,int,int)
    [javac] 		 		  OS.gdk_window_shape_combine_region (window, rgn, 0, 0);
    [javac] 		 		    ^

Comment 22 Silenio Quarti

2012-10-17 13:47:00 EDT

Fixed typo

http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=eafab0cf6ed048b53c3458e2ead53bf0be766694