I see. I looked through the newly changed files and it seems fine although
I don't have time to re-apply the updated patch.

-phil.

On 04/05/2016 11:16 AM, Semyon Sadetsky wrote:
XTaskbarPeer.java, awt_Taskbar.c and awt_Taskbar.h were changed to use the new common gtk_interface.

Glib calls which are necessary for the taskbar module are added to gtk_interface: g_object_unref(), g_list_append(), g_list_free(), g_list_free_full().

--Semyon


On 4/5/2016 8:54 PM, Phil Race wrote:
Could you point me to what was updated ?

-phil.

On 04/05/2016 10:04 AM, Semyon Sadetsky wrote:
The fix is updated because JEP 272 was pushed. JEP 272 has dependency on GTK.

http://cr.openjdk.java.net/~ssadetsky/8145547/webrev.04/

--Semyon

On 3/18/2016 4:02 PM, Semyon Sadetsky wrote:


On 3/17/2016 10:57 PM, Phil Race wrote:
I ran your changes (which included my fix) through jprt and all Solaris platforms failed as follows :-

jdk/src/java.desktop/unix/native/libawt_xawt/awt/sun_awt_X11_GtkFileDialogPeer.c", line 37: error: typedef redeclared: GtkWindow (E_TYPEDEF_REDECLARED) cc: acomp failed for jdk/src/java.desktop/unix/native/libawt_xawt/awt/sun_awt_X11_GtkFileDialogPeer.c

I have fixed the Solaris build: http://cr.openjdk.java.net/~ssadetsky/8145547/webrev.03/.

> gint intval = NULL;
I think this happens with you specific compiler only. Old gtk2_interface.c always contained the same line.

I ran JPRT with webrev.03. The URL is:
http://sthjprt.uk.oracle.com/archives/2016/03/2016-03-18-070745.ssadetsk.client/ It seem macosx_x64_10.9-product-c2-jdk_math build failed because of another change. (?) Could you help me to interpret the JPRT job status?

--Semyon

-phil.

On 03/17/2016 09:42 AM, Phil Race wrote:
This reminded me I had not yet tried the build myself.
When I did I not get the error that Sergey did but I got this as well :-

jdk9-client/jdk/src/java.desktop/unix/native/libawt_xawt/awt/gtk3_interface.c: In function ‘get_integer_property’: jdk9-client/jdk/src/java.desktop/unix/native/libawt_xawt/awt/gtk3_interface.c:2542:19: error: initialization makes integer from pointer without a cast [-Werror] jdk9-client/jdk/src/java.desktop/unix/native/libawt_xawt/awt/gtk3_interface.c: In function ‘get_boolean_property’: jdk9-client/jdk/src/java.desktop/unix/native/libawt_xawt/awt/gtk3_interface.c:2549:19: error: initialization makes integer from pointer without

Both lines it complains about were :

    gint intval = NULL;

I changed them to
       gint intval = 0;

I am not using the 'official' compiler, but you really should make sure jprt is happy before confirming the webrev as final - and also fix all of these even if
the official compiler does not complain.

And I did not notice any problems running SwingSet2 with either of GTK2 or GTK3 specified. It at least ran fine and clearly picked up different versions.


-phil.


On 03/17/2016 06:25 AM, Semyon Sadetsky wrote:
The awt_Desktop.c is missed in the webrev.
Please see the updated one: http://cr.openjdk.java.net/~ssadetsky/8145547/webrev.02/

--Semyon

On 3/17/2016 3:57 PM, Sergey Bylokhov wrote:
On 16.03.16 20:52, Semyon Sadetsky wrote:
Hi Phil,

Thank for review. You will find my reply below in the text.

The updated webrev is
http://cr.openjdk.java.net/~ssadetsky/8145547/webrev.01/

On my system this version fails to build(Ubuntu 15.10 + gcc 5.2.1):

ERROR: Build failed for target 'images' in configuration 'linux-x86_64-normal-server-release' (exit code 2)
=== Output from failing command(s) repeated here ===
* For target support_native_java.desktop_libawt_xawt_awt_Desktop.o: /mnt/hgfs/moe/workspaces/jdk/9/client-gate/jdk/src/java.desktop/unix/native/libawt_xawt/xawt/awt_Desktop.c: In function ‘Java_sun_awt_X11_XDesktopPeer_init’: /mnt/hgfs/moe/workspaces/jdk/9/client-gate/jdk/src/java.desktop/unix/native/libawt_xawt/xawt/awt_Desktop.c:46:9: error: implicit declaration of function ‘gtk2_load’ [-Werror=implicit-function-declaration]
     if (gtk2_load(env) && gtk2_show_uri_load(env)) {
         ^
/mnt/hgfs/moe/workspaces/jdk/9/client-gate/jdk/src/java.desktop/unix/native/libawt_xawt/xawt/awt_Desktop.c:46:27: error: implicit declaration of function ‘gtk2_show_uri_load’ [-Werror=implicit-function-declaration]
     if (gtk2_load(env) && gtk2_show_uri_load(env)) {
                           ^
cc1: all warnings being treated as errors
=== End of repeated output ===
=== Make failure sequence repeated here ===
Awt2dLibraries.gmk:369: recipe for target '/mnt/hgfs/moe/workspaces/jdk/9/client-gate/build/linux-x86_64-normal-server-release/support/native/java.desktop/libawt_xawt/awt_Desktop.o' failed
make/Main.gmk:175: recipe for target 'java.desktop-libs' failed
=== End of repeated output ===


It also contains:
- new properties jdk.gtk.version and jdk.gtk.verbose
- appearance tuning for Ubuntu 15 (GTK 3.14). It may require more but we
can do this later as a separate bug.
The main implementation was done for Ubuntu 14.05 LTS (GTK 3.10) and then tuned for OEL 7 (GTK 3.8). Each minor GTK version may have some
appearance changes.

On 3/15/2016 10:39 PM, Phil Race wrote:
There is a lot to read here. I think I need to patch and try it but
first ...

Two high level questions :
1) Have you verified that this behaves properly (or no worse than
currently) with -Djava.awt.headless=true since Swing components
are supposed to be able to draw off-screen in headless mode .. and yet a dependency on GTK and its dependency on xlibraries seems to mean
that you can't load GTK in this case.
BTW I think it may be painful to get them to layout in such a case
but that's another issue.
I tested it by painting to a BufferedImage. Seems it is enough.

2) Have you tried a hi-dpi system ?
Yes I have. It is identical to the existing GTK2 based appearance.

3) Have you submitted a JPRT job ? It is essential to know that this
builds cleanly on the "official" compilation environment.
I will do this before the push. I think it will be OK because I did not use any new C constructs and the new libraries are linked dynamically.
4)I expect you ran Swingset2 + GTK L&F but have you run any of the
regression test suite ?
Yes I ran javax/swing tests but many of them fails with GTK2 as well. With GTK3 the result was the same except for some unstable tests. Unity desktop has new window decorations like borderless windows which are
resized by dragging the outer window shadow, invisible overlay
scrollbars, etc. Many tests written for old window decorations fails.

Minor comments :

GTKEngine.java

494 Container parent = context.getComponent().getParent();
 495         if(GTKLookAndFeel.is3()) {
496 if (parent != null && parent.getParent() instanceof
JComboBox) {
 497                 if (parent.getParent().hasFocus()) {
 498                     synthState |= SynthConstants.FOCUSED;
 499                 }
 500             }
 501         }

GTKPainter.java

746         if (GTKLookAndFeel.is3()) {
747 if (slider.getOrientation() == JSlider.VERTICAL) {
 748                 y += 1;
 749                 h -= 2;
 750             } else {
 751                 x += 1;
 752                 w -= 2;
 753             }
 754         }

I don't know where these numbers come from or what coordinate system is being used here but it seems you are changing them for gtk 2.2 as
well as 3
Can you speak to this ?
It is an appearance tuning for GTK3. I didn't change it for GTK2, why do
you think so?
This was used before my fix as well, for example

                     if (containerParent instanceof JComboBox) {
                         x += (focusSize + 2);
                         y += (focusSize + 1);
                         w -= (2 * focusSize + 1);
                         h -= (2 * focusSize + 2);
                     } else {
                         x += focusSize;
                         y += focusSize;
                         w -= 2 * focusSize;
                         h -= 2 * focusSize;
                     }

The only place where I changed the existing GTK2 appearance is:

1121 CLASS_SPECIFIC_MAP.put("Slider.thumbWidth", "slider-length");

  in GTKStyle.java, because this property was omitted by mistake.

GTKStyle.java

735         if(!GTKLookAndFeel.is3()) {

840      } else if(GTKLookAndFeel.is3() &&
"ComboBox.forceOpaque".equals(key)) {


we prefer a space between "if" and "("
Accepted.

sun_awt_X11_GtkFileDialogPeer.c

 392     if (gtk->gtk_check_version(2, 8, 0) == NULL) {


Maybe I am not looking at the right fn but I thought I saw
this fn return a boolean so a check against NULL looks wrong.
The declaration is in GtkApi struct of gtk_interface.h. It returns
char*. NULL means that the version is compatible.

 393
gtk->gtk_file_chooser_set_do_overwrite_confirmation(GTK_FILE_CHOOSER(
 394                 dialog), TRUE);


You didn't add this but it is awfully specific about the GTK version and
I wonder if this test is doing what it should be doing on GTK 3?
Accepted.

It is interesting that some equivalent looking Java level dialog
checking in XToolkit.java
checked for 3.0 too ..

swing_GTKEngine.c :

30 /* Static buffer for conversion from java.lang.String to UTF-8 */
  31 static char convertionBuffer[CONV_BUFFER_SIZE];

So the variable name should be spelt conversionBuffer.
Accepted.

awt_UNIXToolkit.c

< 287 free(ret);

You deleted this free(). Is that correct ? It seems to imply
you now expect a boolean return as discussed above and
so in that case NULL looks odd here (line 260) too.
The JNI exported method returns boolean while the GTK method returns char*. free() is deleted intentionally according to the GTK docs it
belongs to the library and should not be freed by user code.

gtk3_interface.h :

36 #define G_PI 3.1415926535897932384626433832795028841971693993751

I don't think that is completely accurate :-) And I should have
reviewed this yesterday [1].
:) This is glib's definition I just copied.

--Semyon

-phil.

[1] http://www.piday.org/


On 03/05/2016 01:14 PM, Semyon Sadetsky wrote:
Hello,

Please review fix for JDK9:

bug: https://bugs.openjdk.java.net/browse/JDK-8145547
webrev: http://cr.openjdk.java.net/~ssadetsky/8145547/webrev.00/

The fix contains GTK3 based implementation for Swing GTK LnF, AWT
FileChooser for Linux and AWT Robot for Linux.
Also the new system property is added to request specific GTK version
swing.gtk.version.

--Semyon












Reply via email to