<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 Apr 8 17:03:22 UTC 2016
The fix was updated to take into account defects discovered by parfait:
(*env)->ExceptionCheck(env); is removed from the
gtk3_get_drawable_data() in gtk2_interface.c and gtk3_interface.c
because GetPrimitiveArrayCritical()/ ReleasePrimitiveArrayCritical() do
not throw exceptions.
xx and yy variables in the gtk3_paint_arrow() of gtk3_interface.c might
not be initialized.
The updated webrev: http://cr.openjdk.java.net/~ssadetsky/8145547/webrev.05/
--Semyon
On 4/6/2016 10:32 AM, Alexander Scherbatiy wrote:
>
> The fix looks good to me.
>
> Thanks,
> Alexandr.
>
> On 4/5/2016 8:04 PM, 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
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20160408/5c4f576c/attachment.html>
More information about the swing-dev
mailing list