8236651: Simplify and update glass gtk backend

[tsayao]

Summary

• Simplify and update the Gtk glass backend, making Linux a first-class OpenJFX platform.

Goals

• Make Linux a first-class OpenJFX platform (see Motivation);
• Simplify the code and reduce it's size;
• Update to gtk3 (it was originally a port from gtk2);
• Remove unused code (such as applets and web start);
• Prepare the ground for a possible future Wayland support.

Testing

./gradlew -PEXTRA_TEST_ARGS='-Djavafx.gtk.experimental=true' -PFULL_TEST=true -PUSE_ROBOT=true :systemTests:test

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed

Testing

	Linux x64	Windows x64	macOS x64
Build / test	✔️ (1/1 passed)	✔️ (1/1 passed)	✔️ (1/1 passed)

Integration blocker

 ⚠️ The change requires a CSR request to be approved.

Issue

• JDK-8236651: Simplify and update glass gtk backend

Download

$ git fetch https://git.openjdk.java.net/jfx pull/77/head:pull/77
$ git checkout pull/77

[bridgekeeper]

👋 Welcome back tsayao! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request (refresh this page to view it).

[mlbridge]

Webrevs

• 00: Full (e19295f)
• 62: Full - Incremental (e19295f)
• 61: Full - Incremental (97c6741)
• 60: Full - Incremental (a8e1e18)
• 59: Full - Incremental (ffe3c36)
• 58: Full - Incremental (e373848)
• 57: Full - Incremental (2f3bdb2)
• 56: Full - Incremental (aeff508)
• 55: Full - Incremental (ad8823d)
• 54: Full - Incremental (2700f6f)
• 53: Full - Incremental (9c83ac1)
• 52: Full - Incremental (e947aa6)
• 51: Full - Incremental (7d350bc)
• 50: Full - Incremental (3e3aa58)
• 49: Full - Incremental (85defa5)
• 48: Full - Incremental (d18b5be)
• 47: Full - Incremental (2f83ce7)
• 46: Full - Incremental (8973d38)
• 45: Full - Incremental (91d9846)
• 44: Full - Incremental (748d81b)
• 43: Full - Incremental (acd5496)
• 42: Full - Incremental (f52f63b)
• 41: Full - Incremental (11688ae)
• 40: Full - Incremental (031d228)
• 39: Full - Incremental (e499a54)
• 38: Full - Incremental (547c3cb)
• 37: Full - Incremental (f367c9a)
• 36: Full - Incremental (142073d)
• 35: Full - Incremental (381e8d1)
• 34: Full - Incremental (b1222f8)
• 33: Full - Incremental (3855e16)
• 32: Full - Incremental (70878fb)
• 31: Full - Incremental (f8fe264)
• 30: Full - Incremental (086143a)
• 29: Full - Incremental (3108093)
• 28: Full - Incremental (11ca96c)
• 27: Full - Incremental (cddf3ad)
• 26: Full - Incremental (bc3711e)
• 25: Full (f772c2e)
• 24: Full - Incremental (08caafd)
• 23: Full - Incremental (2c51b04)
• 22: Full - Incremental (06968c9)
• 21: Full - Incremental (84bfd8b)
• 20: Full - Incremental (465c9cd)
• 19: Full - Incremental (7ed26f4)
• 18: Full (14a75ce)
• 17: Full - Incremental (e829b8e)
• 16: Full - Incremental (058a099)
• 15: Full - Incremental (0c982d6)
• 14: Full (694641f)
• 13: Full - Incremental (cb2d9fb)
• 12: Full (2bfac07)
• 11: Full (4b55562)
• 10: Full (f35fa9b)
• 09: Full (f99aebc)
• 08: Full (9850bb7)
• 08: Full (9850bb7)
• 07: Full (209b7ac)
• 06: Full (0dc70ab)
• 05: Full (e4d2a1d)
• 04: Full (cc7745b)
• 03: Full (49426b6)
• 02: Full (628d552)
• 01: Full (7b48e48)
• 00: Full (039daa4)

[kevinrushforth]

Please, mark it as WIP.

You can do this by editing the title and adding WIP: as a prefix.

[kevinrushforth]

This sort of enhancement needs to be discussed on the openjfx-dev mailing list first. While the WIP PR might be used to illustrate what you have in mind to propose, it is premature to actually review the implementation without first discussing whether and it makes sense to do it, what the high-level goals are, etc.

[tsayao]

I understand. Will do that when the code works 100%. I have submitted the PR to avoid duplicated efforts and let people test it (if anyone is willing).

[mlbridge]

Mailing list message from Thiago Milczarek Sayao on openjfx-dev:

Hello users of this list.

While fixing some bugs on glass-gtk backend I had some noticed some quite complex logic to mimic windows behavior on linux - for example, on linux window decorations are a job do the window manager - which may vary. So window sizes does not account decorations.

Also, the gtk2 -> gtk3 migration could use some improvement because gtk3 does a lot of "deprecations" - even inside the gtk3 release cycle.

There was a lot of code regarding applets and web start, which are discontinued.

So this PR simplifies te code (specially sizing a positioning of windows), updates to gtk3 recent changes (while being compatible with older releases) and removes web start / applet code.

It also eliminates some restrictions on DND. It will work with all text formats and all image formats that both gtk and javafx supports.

It's working very well. So if anyone is willing to test, or take a look.

I need to do more testing on:

* Mouse grabs to mimic windows behavior;
* Native _setBackground call

Any pointers on uses cases for this is appreciated.

Cheers.

________________________________
De: openjfx-dev <openjfx-dev-bounces at openjdk.java.net> em nome de Thiago Milczarek Sayao <tsayao at openjdk.java.net>
Enviado: s?bado, 11 de janeiro de 2020 22:00
Para: openjfx-dev at openjdk.java.net <openjfx-dev at openjdk.java.net>
Assunto: Re: [Rev 08] RFR: WIP: 8236651: Simplify and update glass gtk backend

This proposed change does the following:

- Ports DND target to use GTK reducing code size and adding extra text / image formats (such as .gif);
- Use gtk signals instead of gdk events (also to reduce code size);
- Simplifies geometry (sizing/positioning) with a more straightforward code (less special cases) ;
- Replaces (pointer and focus) grabbing with a gtk approach;
- Reworked frame extents (the wm extension to get decoration sizes) to reduce size and complexity;
- Simplified cursor changing;
- Reduced the use of gtk/gdk deprecated functions;

In general it reduces code size and complexity and hands more work to gtk.

Important notice: As I could not test the code for handling web start and web applets because browsers do not support it anymore and java has removed "javaws", I took the liberty to remove the code. Will restore if necessary.


The pull request has been updated with 1 additional commit.

-------------

Added commits:
- 9850bb7: Fix dialog with owner sizing

Changes:
- all: https://git.openjdk.java.net/jfx/pull/77/files
- new: https://git.openjdk.java.net/jfx/pull/77/files/209b7ac7..9850bb7

Webrevs:
- full: https://webrevs.openjdk.java.net/jfx/77/webrev.08
- incr: https://webrevs.openjdk.java.net/jfx/77/webrev.07-08

Stats: 7 lines in 1 file changed: 0 ins; 3 del; 4 mod
Patch: https://git.openjdk.java.net/jfx/pull/77.diff
Fetch: git fetch https://git.openjdk.java.net/jfx pull/77/head:pull/77

PR: https://git.openjdk.java.net/jfx/pull/77

[tsayao]

Code Changes

• glass_window.cpp / glass_window.h

  • Removed WindowContextPlug and WindowContextChild (that were used for applets / web start) and moved everything to
    one class named WindowContext (since inheritance was no required anymore)
  • Changed set_enabled() to use gtk_widget_set_sensitive instead of custom code;
  • Moved to gtk signals instead of gdk events (to use gtk_widget_set_sensitive and gtk_grab_add);
  • Frame Extents: Removed the code to request extents and gtk already does it by default;
  • Size calculation: Reworked size calculation code. In general, X windows are content size instead of whole window size (considering extents - frame decorations). OpenJfx uses "whole window size" as window sizes, so it requires a "hack" to recalculate sizes when set_bounds() is called with window sizes instead of content sizes. The rework was to simplify code paths and make it more straightforward.
  • Other Size calculation changes:
    • Use gtk_window_set_default_size() for initial size which is the appropriate function;
    • Gravity is now ignored as it is on Windows glass impl;
    • Avoid sending same sizes to Java;
    • Introduced calculate_adjustments() which is a fallback when frame extents is not present (it's optional to window managers to implement it);
  • Geometry: Min / Max sizes - reworked it to simplify / Concentrated geometry changes on
    apply_geometry().
  • Mouse grab: Reworked it to use to correct functions according to gtk+ version;
  • Draw: Reworked it to use the correct calls accord to gtk+ version changes;
  • Fixed JDK-8237491;
  • Moved background code to paint() as gtk3 uses styles to set the background and other functions were deprecated;
  • Reorganized function order on glass_window.cpp to match glass_window.h

• GlassCursor.cpp

  • Gtk+3 uses a name-like-css approach - so it was properly ported to gtk3 way;
  • Reworked Gtk+2 to use a function instead of manual calls to find the cursor;

• GtkWindow.java

  • Moved the extents special case to native glass;

• GlassApplication.cpp

  • Removed Gdk events where possible (it's now on glass_window as signals);
  • Removed applet/web start code;

• GlassView.cpp

  • Changes to reflect frame extents rework on glass_window

• GlassWindow.cpp

  • WindowContextTop -> WindowContext
  • Removed applet / web start code;
  • Removed frame extents (which is not called anymore due to GtkWindow.java change);

• glass_general.cpp

  • Removed functions that became unused;
  • Added is_grab_disabled() that is used on glass_window

• glass_window_ime.cpp

  • WindowContextTop -> WindowContext;

• glass_dnd.cpp / glass_dnd.h

  • Ported to Gtk signals;
  • Use all possible image formats (supported by GdkPixbuf / OpenJfx) - .gif is now possible (for ex.);
  • Allow COMPOUND_TEXT;
  • Do not request content while dragging;
  • Reduce overall code size.

[kevinrushforth]

This is going to need further discussion on the mailing list as indicated above, so it is still premature to review it (i.e., it should still be considered effectively a "WIP" until that discussion happens). Additionally, this is a significant and risky change, so I'd like additional eyes on it when we do get to the point of reviewing it.

[kevinrushforth]

/reviewers 3

[openjdk]

@kevinrushforth
The number of required reviews for this PR is now set to 3 (with at least 1 of role reviewers).

[tsayao]

I have been testing this for several days on ubuntu 18.04 and it's working good. It pass system tests, runs Ensemble 8 and Scenebuilder.

Will have to do some tests on 16.04.

[tsayao]

Ubuntu 20.04 Test Results

Updated April 2nd.

[pankaj-bansal]

After the latest commit on June 10, this is not building for me on my Ubuntu 18.04. I am attaching the build log for reference.
build.log

[tsayao]

After the latest commit on June 10, this is not building for me on my Ubuntu 18.04. I am attaching the build log for reference.
build.log

It's now fixed. I had used two compilation parameters to limit Gtk on 3.8 (so it would generate error if any symbol > 3.8 were used). But that does not seem to work on 18.04, so I removed it.

[kevinrushforth]

I had used two compilation parameters to limit Gtk on 3.8 (so it would generate error if any symbol > 3.8 were used). But that does not seem to work on 18.04, so I removed it.

Good. I was going to ask you about that, so I'm happy to see it gone.

[tsayao]

I have investigated the Tab Pane Drag Test and it works manually.

import javafx.application.Application;
import javafx.scene.Scene;
import javafx.scene.control.Tab;
import javafx.scene.control.TabPane;
import javafx.stage.Stage;

public class Test
        extends Application {

    @Override
    public void start(Stage stage) {
        TabPane tabPane = new TabPane();
        tabPane.setTabDragPolicy(TabPane.TabDragPolicy.REORDER);
        Scene scene = new Scene(tabPane, 800, 600);
        stage.setScene(scene);
        Tab tab1 = new Tab("Tab1");
        Tab tab2 = new Tab("Tab2");

        tabPane.getTabs().addAll(tab1, tab2);


        stage.setAlwaysOnTop(true);
        stage.show();
    }

    public static class Main {
        public static void main(String[] args) {
            Application.launch(Test.class, args);
        }
    }
}

It also works if I switch back to GDK Events instead of Gtk Signals. But it is a drag test, by experience they don't work well on Robot.

[pankaj-bansal]

Following are results in Ubuntu 18.04 after fix for tab pane tests.

[pankaj-bansal]

This is the result on OL 82 with latest commit

[tsayao]

@pankaj-bansal Sorry for commiting again. Now the Tab Pane test works properly.

[openjdk]

@tsayao This pull request looks like it contains a merge commit that brings in commits from outside of this repository. If you want these commits to be preserved when you integrate, you will need to make a 'merge-style' pull request. To do this, change the title of this pull request to Merge <project>:<branch>, where <project> is the name of another project in the OpenJDK organization. For example: Merge jdk:master.

[kevinrushforth]

If you want these commits to be preserved when you integrate, you will need to make a 'merge-style' pull request.

Please ignore this information from Skara. I don't know what caused it, but I've pinged the Skara folks about it.

[Torbuntu]

Would it be possible and related to add the gtk touch events to this PR as part of the Glass GTK3 update? Or would follow up update PR adding that be more appropriate?

Currently linux desktop does not have touch support because of the missing gesture/touch events from gtk3.

[tsayao]

@Torbuntu Not to this PR, I don't want to delay it too much. But can be done (I just do not own a touch device currently).

[Torbuntu]

Sounds good! I have a few devices I'd be more than excited to test on, and even help add the feature myself if I can figure it out if time is a big issue?

[tsayao]

I would starting hooking gtk event signals (https://developer.gnome.org/gtk3/stable/GtkWidget.html), specially the "touch-event" to JavaFx events (probably need to add through JNI). Should be simple. Contact me at thiago.sayao (gmail).

[Torbuntu]

I would starting hooking gtk event signals (https://developer.gnome.org/gtk3/stable/GtkWidget.html), specially the "touch-event" to JavaFx events (probably need to add through JNI). Should be simple. Contact me at thiago.sayao (gmail).

Would it be safe you think to branch from your PR to have the updated gtk backend? It would be much easier to work with.

[tsayao]

Rebased onto "master" and squashed commits.

[tsayao]

Tested on Ubuntu 20.04

756 tests completed, 6 failed, 110 skipped

Failed:

test.robot.javafx.scene.ColorPickerTest > testColorPickerSceneChange FAILED
    java.lang.AssertionError: Timeout: Failed to receive onAction callback.
        at org.junit.Assert.fail(Assert.java:91)
        at org.junit.Assert.assertTrue(Assert.java:43)
        at test.robot.javafx.scene.ColorPickerTest.waitForLatch(ColorPickerTest.java:203)
        at test.robot.javafx.scene.ColorPickerTest.clickColorPickerPalette(ColorPickerTest.java:88)
        at test.robot.javafx.scene.ColorPickerTest.testColorPickerSceneChange(ColorPickerTest.java:119)

test.robot.javafx.scene.RobotTest > testKeyPress FAILED
    org.junit.ComparisonFailure: letter 'a' should be pressed by Robot expected:<[a]> but was:<[]>
        at org.junit.Assert.assertEquals(Assert.java:123)
        at test.robot.javafx.scene.RobotTest.testKeyboard(RobotTest.java:193)
        at test.robot.javafx.scene.RobotTest.testKeyPress(RobotTest.java:144)

test.robot.javafx.scene.tableview.TableViewResizeColumnToFitContentTest > resizeColumnToFitContentTest FAILED
    java.lang.AssertionError: resizeColumnToFitContent failed
        at org.junit.Assert.fail(Assert.java:91)
        at org.junit.Assert.assertTrue(Assert.java:43)
        at test.robot.javafx.scene.tableview.TableViewResizeColumnToFitContentTest.resizeColumnToFitContentTest(TableViewResizeColumnToFitContentTest.java:96)

test.robot.javafx.stage.IconifyTest > canIconifyDecoratedStage FAILED
    junit.framework.AssertionFailedError: expected:rgba(255,0,0,255) but was:rgba(62,62,62,255)
        at test.robot.testharness.VisualTestBase.assertColorEquals(VisualTestBase.java:179)
        at test.robot.javafx.stage.IconifyTest.lambda$canIconifyStage$4(IconifyTest.java:97)

test.robot.javafx.stage.IconifyTest > canIconifyTransparentStage FAILED
    junit.framework.AssertionFailedError: expected:rgba(0,255,0,255) but was:rgba(88,88,88,255)
        at test.robot.testharness.VisualTestBase.assertColorEquals(VisualTestBase.java:179)
        at test.robot.javafx.stage.IconifyTest.lambda$canIconifyStage$6(IconifyTest.java:108)

test.robot.javafx.stage.IconifyTest > canIconifyNonResizableStage FAILED
    junit.framework.AssertionFailedError: expected:rgba(255,0,0,255) but was:rgba(44,44,44,255)
        at test.robot.testharness.VisualTestBase.assertColorEquals(VisualTestBase.java:179)
        at test.robot.javafx.stage.IconifyTest.lambda$canIconifyStage$4(IconifyTest.java:97)

For some reason the html report is not being generated.

Second run:
756 tests completed, 4 failed, 110 skipped

test.robot.javafx.scene.RobotTest > testKeyPress FAILED
    org.junit.ComparisonFailure: letter 'a' should be pressed by Robot expected:<[a]> but was:<[]>
        at org.junit.Assert.assertEquals(Assert.java:123)
        at test.robot.javafx.scene.RobotTest.testKeyboard(RobotTest.java:193)
        at test.robot.javafx.scene.RobotTest.testKeyPress(RobotTest.java:144)

test.robot.javafx.stage.IconifyTest > canIconifyDecoratedStage FAILED
    junit.framework.AssertionFailedError: expected:rgba(255,0,0,255) but was:rgba(44,44,44,255)
        at test.robot.testharness.VisualTestBase.assertColorEquals(VisualTestBase.java:179)
        at test.robot.javafx.stage.IconifyTest.lambda$canIconifyStage$4(IconifyTest.java:97)

test.robot.javafx.stage.IconifyTest > canIconifyTransparentStage FAILED
    junit.framework.AssertionFailedError: expected:rgba(0,255,0,255) but was:rgba(88,88,88,255)
        at test.robot.testharness.VisualTestBase.assertColorEquals(VisualTestBase.java:179)
        at test.robot.javafx.stage.IconifyTest.lambda$canIconifyStage$6(IconifyTest.java:108)

test.robot.javafx.stage.IconifyTest > canIconifyNonResizableStage FAILED
    junit.framework.AssertionFailedError: expected:rgba(255,0,0,255) but was:rgba(44,44,44,255)
        at test.robot.testharness.VisualTestBase.assertColorEquals(VisualTestBase.java:179)
        at test.robot.javafx.stage.IconifyTest.lambda$canIconifyStage$4(IconifyTest.java:97)

[ebresie]

Hi @ebresie, thanks for making a comment in an OpenJDK project!

All comments and discussions in the OpenJDK Community must be made available under the OpenJDK Terms of Use. If you already are an OpenJDK Author, Committer or Reviewer, please click here to open a new issue so that we can record that fact. Please Use "Add GitHub user ebresie for the summary.

If you are not an OpenJDK Author, Committer or Reviewer, simply check the box below to accept the OpenJDK Terms of Use for your comments.

• I agree to the OpenJDK Terms of Use for all comments I make in a project in the OpenJDK GitHub organization.

Your comment will be automatically restored once you have accepted the OpenJDK Terms of Use.

[tsayao]

I would prefer this approach:
#367

[kevinrushforth]

Given the alternate approach that you propose to pursue (which I agree is a better option), this PR should either be closed or moved to Draft.

[tsayao]

Closing it in favor of #367