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
