eclipsesource / papyrus-gefx

A new Editor kind for Papyrus, based on GEFx and GMF Runtime
0 stars 2 forks source link

[Viewer] Investigate asynchronous loading of the editor #20

Open CamilleLetavernier opened 6 years ago

CamilleLetavernier commented 6 years ago

Unlike SWT, JavaFX supports creating a Node tree outside of the UI Thread. FX Thread is only required when manipulating nodes that already belong to a Scene.

It should be possible to load a diagram in a background thread (While presenting a progress bar in the UI thread), and only attach the diagram to the scene once it's been initialized (i.e. When the initial content part hierarchy has been built). This way, only the initial rendering would freeze the UI, which may be significant for medium/large diagrams.

Early experiments on this topic show that GEF requires the Viewer to be attached to a Scene before it is rendered. However, this is only required because Gestures attach event listeners to the Scene. It is possible to slightly delay Gestures initialization, i.e. replace:

With:

However this requires changes in GEF (Or to override/replace the default GEF Viewer and Gestures initialization, but it would probably be better & easier to just patch GEF directly).

A few bugs remain with this approach (Maybe related to the FX-in-SWT integration, or some bugs somewhere in the JavaFX/GEF/PapyrusFX stack - the exceptions were inconsistent), especially in the refreshVisuals() methods. I didn't investigate this much further, but I mostly got it working by skipping refreshVisuals() in the initialization phase, and only calling it after the Viewer is attached to a Scene.

CamilleLetavernier commented 5 years ago

A ticket has been created for GEF Gestures:

545407: Gestures do not support scene changes https://bugs.eclipse.org/bugs/show_bug.cgi?id=545407

and a PullRequest has been proposed

Unlike what I tried in my first approach, the Gestures are still initialized with the domain; but they listen to scene changes (And support initial null scenes); which is more convenient & reliable than postponing the initialization of Gestures.

CamilleLetavernier commented 5 years ago

The pull request has been accepted & merged to master; it will be available in 2019-06. Once this is done, I can update my branch to integrate this & start the viewer in the background

CamilleLetavernier commented 5 years ago

Performances didn't seem as good as expected (It still took quite some time to attach the diagram to the scene, partially defeating the purpose of background initialization), but this might be caused by https://bugs.openjdk.java.net/browse/JDK-8193445 (https://bugs.openjdk.java.net/browse/JDK-8151756)

Fortunately, the recent version of E(fx)clipse provides a workaround for that (https://github.com/eclipse-efx/efxclipse-rt/issues/340), so we might be able to integrate this for 2019-06

CamilleLetavernier commented 5 years ago

https://bugs.openjdk.java.net/browse/JDK-8193445

A proper performance fix has been pushed on OpenJFX master, and has even been backported on Oracle JDK8 (>= u251):

https://github.com/openjdk/jfx/pull/34