• Type: Bug
• Component: javafx
• Sub-Component: graphics
• Affected Version: 9

• Priority: P2
• Status: Resolved
• Resolution: Fixed

• Submitted: 2016-08-09
• Updated: 2017-01-04
• Resolved: 2016-12-15

JDK 9
9 b150Fixed

This change went into jdk-9+150. 04-01-2017 Changeset: 3784c96374cd Author: ddhill Date: 2016-12-15 17:49 -0500 URL: http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/3784c96374cd 8163496: Rework Glass GTK to correct gtk3 structure sizes Reviewed-by: kcr, ssadetsky 15-12-2016 All my testing is done and looks good. Changes above (diffs to the .9 patch) look good. +1 15-12-2016 addressing Kevin's comments diff --git a/modules/javafx.graphics/src/main/java/com/sun/glass/ui/gtk/GtkApplication.java b/modules/javafx.graphics/src/main/java/com/sun/glass/ui/gtk/GtkApplication.java --- a/modules/javafx.graphics/src/main/java/com/sun/glass/ui/gtk/GtkApplication.java +++ b/modules/javafx.graphics/src/main/java/com/sun/glass/ui/gtk/GtkApplication.java @@ -169,7 +169,7 @@ int libraryToLoad = _queryLibrary(gtkVersion, gtkVersionVerbose); AccessController.doPrivileged((PrivilegedAction<Void>) () -> { - if (libraryToLoad == QUERY_NO_DISPLAY) { + if (libraryToLoad == QUERY_NO_DISPLAY) { throw new UnsupportedOperationException("Unable to open DISPLAY"); } else if (libraryToLoad == QUERY_USE_CURRENT) { if (gtkVersionVerbose) { diff --git a/modules/javafx.graphics/src/main/native-glass/gtk/launcher.c b/modules/javafx.graphics/src/main/native-glass/gtk/launcher.c --- a/modules/javafx.graphics/src/main/native-glass/gtk/launcher.c +++ b/modules/javafx.graphics/src/main/native-glass/gtk/launcher.c @@ -47,19 +47,6 @@ static jboolean gtk_versionDebug = JNI_FALSE; -#define LOAD_SYMBOL(s, x) \ - if (s == NULL) { \ - s = dlsym(libraryHandle, #x); \ - if (s == NULL) { \ - fprintf(stderr,"failed loading %s\n", #x); \ - exit(-1); \ - } else { \ - fprintf(stderr,"loading %s\n", #x); \ - } \ - } else { \ - fprintf(stderr,"have %s\n", #x); \ - } - static const char * gtk2_chain[] = { "libglassgtk2.so", "libglassgtk3.so", 0 }; @@ -76,7 +63,7 @@ (void) reserved; javaVM = jvm; - // better way to get/save this? + return JNI_VERSION_1_6; } @@ -141,8 +128,13 @@ use_chain = three_to_two; gtk_version = 3; } else { - printf(" Huh ? what version is %d\n", wantVersion); - exit (-3); + // Note, this should never happen, java should be protecting us + if (gtk_versionDebug) { + printf("bad GTK version specified, assuming 2\n"); + } + wantVersion = 2; + use_chain = two_to_three; + gtk_version = 2; } int i, found = 0; 15-12-2016 Testing is almost done. The code changes look good except for one relatively minor issue, which needs to be fixed: * You have two places where you do an "exit" that need to be fixed. It is not OK for a library to exit because of bad data (rather an error should be returned or exception thrown). In LOAD_SYMBOL: 50 #define LOAD_SYMBOL(s, x) \ 51 if (s == NULL) { \ 52 s = dlsym(libraryHandle, #x); \ 53 if (s == NULL) { \ 54 fprintf(stderr,"failed loading %s\n", #x); \ 55 exit(-1); \ In sniffLibs: 144 printf(" Huh ? what version is %d\n", wantVersion); 145 exit (-3); * Also, as long as you are fixing the above, I found a minor nit that you might wish to fix. There is an extra space after '==' in the following: + if (libraryToLoad == QUERY_NO_DISPLAY) { 15-12-2016 This rev fixes the testin failures on OEL 7.0, and resolve a missing symbol on OEL 7.0 http://cr.openjdk.java.net/~ddhill/8163496.9/ The only known issue is the occasional transparent stage that is not transparent. 15-12-2016 http://cr.openjdk.java.net/~ddhill/8163496.8/ This corrects the incremental compile issue, g_settings_schema_unref (the_schema), the mis-versioning of the libraries for the dlopen check. Working on the failed web test still. 13-12-2016 Yet one minor issue with the linux build. Changes in C files are ignored by the GTK3 link task during incremental build. :graphics:ccLinuxGlassGlass UP-TO-DATE :graphics:linkLinuxGlassGlass UP-TO-DATE :graphics:ccLinuxGlassGlassgtk2 :graphics:linkLinuxGlassGlassgtk2 :graphics:ccLinuxGlassGlassgtk3 :graphics:linkLinuxGlassGlassgtk3 UP-TO-DATE <-------------- :graphics:nativeGlass :graphics:ccLinuxIio UP-TO-DATE :graphics:linkLinuxIio UP-TO-DATE 13-12-2016 I have three additional comments based on testing: 1. Transparent windows no longer seem to work (at least not reliably) with GTK 3. 2. Incremental compilation of a single native file (e.g., a .cpp file) will not cause the libglassgtk3 library to be relinked. 3. I get the following crash running 'gradle :web:test' on Oracle Linux 7. This happens both with GTK 2 and GTK 3. Gradle Test Executor 1 finished executing tests. # # A fatal error has been detected by the Java Runtime Environment: # # SIGSEGV (0xb) at pc=0x00007f5b92a8ad70, pid=4746, tid=4772 # # JRE version: Java(TM) SE Runtime Environment (9.0+144) (build 9-ea+144) # Java VM: Java HotSpot(TM) 64-Bit Server VM (9-ea+144, mixed mode, tiered, compressed oops, g1 gc, linux-amd64) # Problematic frame: # C [libXi.so.6+0x20d70] # # Core dump will be written. Default location: Core dumps may be processed with "/usr/libexec/abrt-hook-ccpp %s %c %p %u %g %t e" (or dumping to /localhome/krushfor/javafx/9-kcr/jfx/rt/modules/javafx.web/core.4746) # # An error report file with more information is saved as: # /localhome/krushfor/javafx/9-kcr/jfx/rt/modules/javafx.web/hs_err_pid4746.log Phoning home... Using server: 10.161.186.18, port 4711 # # If you would like to submit a bug report, please visit: # http://bugreport.java.com/bugreport/crash.jsp # trying for version 3 trying GTK library set libgtk-3.so.0, libgdk-3.so.0, libgdk_pixbuf-2.0.so.0 using GTK library version 3 set libgtk-3.so.0, libgdk-3.so.0, libgdk_pixbuf-2.0.so.0 :web:test FAILED 06-12-2016 Semyon, Good comments, thank you! I folded in your some of changes which I will webrev in a bit. > Should not it check GTK_CHECK_VERSION(3, 10, 0) instead? Simply..... no. I could add a separate path to not call the wrapped version, but rather the direct version if 3.10+, but that creates a separate test path that would add risk. > What is proposed to do with OEL7 (Glib 2.36)? > Absence of "g_settings_schema_has_key" function doesn't mean that the key-value doesn't exist. Will need to bring Jim in on this one as he fought this one for a while, and this is his code. The problem seems to be what is the least common supportable set of functions that don't segv the application in certain cases. 06-12-2016 Also, I suggest to fix library set names. Currently GTK3 is always loaded by JFX on Ubuntu 16.04 not because of absence of GTK2 but because of the next typo: modules/javafx.graphics/src/main/native-glass/gtk/launcher.c: 86 static char * gtk2_versioned[] = { 87 "2", "libgtk-x11-2.0.so.0", "libgdk-x11-2.0.so.0", "libgdk_pixbuf-2.0.so" line 87 should be: "2", "libgtk-x11-2.0.so.0", "libgdk-x11-2.0.so.0", "libgdk_pixbuf-2.0.so.0" 06-12-2016 modules/javafx.graphics/src/main/native-glass/gtk/wrapped.c: 116 // Note added in libgdk 3.10 which is > our OEL 7.0 version of 3.8 117 void wrapped_gdk_x11_display_set_window_scale (GdkDisplay *display, 118 gint scale) 119 { 120 #if GTK_CHECK_VERSION(3, 0, 0) 121 if(_gdk_x11_display_set_window_scale == NULL) { 122 _gdk_x11_display_set_window_scale = dlsym(RTLD_DEFAULT, "gdk_x11_display_set_window_scale"); 123 if (gtk_verbose && _gdk_x11_display_set_window_scale) { 124 fprintf(stderr, "loaded gdk_x11_display_set_window_scale\n"); fflush(stderr); 125 } 126 } 127 #endif Should not it check GTK_CHECK_VERSION(3, 10, 0) instead? --- 68 // Note added in Glib 2.36 which is >= our OEL 7.0 version of 2.36 69 // but does not seem to be in the headers properly 70 static GSettingsSchema * 71 (*_g_settings_schema_source_lookup) (GSettingsSchemaSource *source, 72 const gchar *schema_id, 73 gboolean recursive); GTK3 documentation says: Since: 2.32 The same for "g_settings_schema_source_get_default". --- 94 // Note added in Glib 2.40 which is > our OEL 7.0 version of 2.36 95 static gboolean (*_g_settings_schema_has_key) (GSettingsSchema *schema, const gchar *name); 96 97 gboolean wrapped_g_settings_schema_has_key (GSettingsSchema *schema, 98 const gchar *name) 99 { 100 if(_g_settings_schema_has_key == NULL) { 101 _g_settings_schema_has_key = dlsym(RTLD_DEFAULT, "g_settings_schema_has_key"); 102 if (gtk_verbose && _g_settings_schema_has_key) { 103 fprintf(stderr, "loaded g_settings_schema_has_key\n"); fflush(stderr); 104 } 105 } What is proposed to do with OEL7 (Glib 2.36)? Absence of "g_settings_schema_has_key" function doesn't mean that the key-value doesn't exist. 06-12-2016 modules/javafx.graphics/src/main/native-glass/gtk/glass_general.cpp: 906 guint glass_settings_get_guint_opt (const gchar *schema_name, 907 const gchar *key_name, 908 int defval) 909 { 910 GSettingsSchemaSource *default_schema_source = 911 wrapped_g_settings_schema_source_get_default(); 912 if (default_schema_source == NULL) { 913 if (gtk_verbose) { 914 fprintf(stderr, "No schema source dir found!\n"); 915 } 916 return defval; 917 } 918 GSettingsSchema *the_schema = 919 wrapped_g_settings_schema_source_lookup(default_schema_source, schema_name, TRUE); 920 if (the_schema == NULL) { 921 if (gtk_verbose) { 922 fprintf(stderr, "schema '%s' not found!\n", schema_name); 923 } 924 return defval; 925 } 926 if (!wrapped_g_settings_schema_has_key(the_schema, key_name)) { 927 if (gtk_verbose) { 928 fprintf(stderr, "key '%s' not found in schema '%s'!\n", key_name, schema_name); 929 } 930 return defval; 931 } 932 if (gtk_verbose) { 933 fprintf(stderr, "found schema '%s' and key '%s'\n", schema_name, key_name); 934 } 935 GSettings *gset = g_settings_new(schema_name); 936 return g_settings_get_uint(gset, key_name); 937 } "the_schema" need to be unreferenced using g_settings_schema_unref (the_schema) before the function returns. 06-12-2016 http://cr.openjdk.java.net/~ddhill/8163496.7 Now will build without gtk3-dev installed. 23-11-2016 http://cr.openjdk.java.net/~ddhill/8163496.6/ 22-11-2016 http://cr.openjdk.java.net/~ddhill/8163496.5/ 22-11-2016 http://cr.openjdk.java.net/~ddhill/8163496.4 22-11-2016 http://cr.openjdk.java.net/~ddhill/8163496.3/ 22-11-2016 http://cr.openjdk.java.net/~ddhill/8163496.2/ 21-11-2016 I tested the .3 version and it build fine for me, but I get GLX errors when trying to run it, both with GTK 2 and GTK 3. Failed in glXMakeCurrent 21-11-2016 http://cr.openjdk.java.net/~ddhill/8163496.1/ 21-11-2016 initial webrev: http://cr.openjdk.java.net/~ddhill/8163496/ 21-11-2016