<Swing Dev> Review-request for 8145547: JEP 283: [AWT/Swing] Conditional support for GTK 3 on Linux
Semyon Sadetsky
semyon.sadetsky at oracle.com
Fri Mar 18 13:02:02 UTC 2016
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
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>
More information about the swing-dev
mailing list