JavaFX Output Implementation - V2

[SonarSonic]

Hey Jannis,

Here is the second version of the JavaFX output implementation, I've created a fresh pull request to resolve the re-basing issues with the master branch and to show the changes more clearly.

Best wishes,
Ollie

[MrKuip]

The current behaviour of the FXSVGCanvas while rendering is that it changes the width and height of the Canvas.
I build my own canvas and it doesn't change the width and height of the Canvas but it scales the GraphicsContext2D in order to fit the SVGDocument. Is that something you would consider?

Here is my 'renderer':

  public void update()
  {
    double scaleX;
    double scaleY;
    double scale;
    GraphicsContext graphics;

    scaleX = getWidth() / document.size().getWidth();
    scaleY = getHeight() / document.size().getHeight();
    scale = Math.min(scaleX, scaleY);

    graphics = getGraphicsContext2D();
    graphics.save();
    try
    {
      graphics.setTransform(1, 0, 0, 1, 0, 0);
      graphics.scale(scale, scale);
      graphics.setGlobalAlpha(1D);
      graphics.setGlobalBlendMode(BlendMode.SRC_OVER);
      graphics.clearRect(0, 0, getWidth(), getHeight());
      FXSVGRenderer.render(document, getGraphicsContext2D());
    }
    finally
    {
      graphics.restore();
    }
  }

[SonarSonic]

@MrKuip Thanks for the suggestion, I think that's something I could look at adding when I'm next working on this, most likely by allowing user defined width + height on the FXSVGCanvas.

[weisJ]

I have made some changes in my version of the branch: https://github.com/weisJ/jsvg/tree/javafx-final

Looking very good now 🚀

[weisJ]

Does this require java 17? If yes we should update the language version for the javafx module to reflect this.

[SonarSonic]

They're slightly out of sync, so this actually only requires Java 11

[weisJ]

I could not get the tests to run locally. It got stuck trying to initialise the tests. Not sure whether this is a problem on my end.

However I checked using the test application and the results look very good.

[SonarSonic]

The try-with-resources prevents the test initialization as the JavaFX thread expects the thread which calls it to stay open. Launching JavaFX out of it's own lifecycle has some strange behaviour. Reverting the last commit should fix your local tests. We could just directly create the thread and not use an Executor Service at all in this case.

[weisJ]

Oh wow, what a footgun! 😅 Directly using a thread is fine. Maybe add a comment about why we don't properly join the thread.

[SonarSonic]

There is one issue I still need to work out when using a different sized ViewBox some of the masked SVGs don't seem to work, just seems to be a transform issue of some kind. I will look into that again soon, then most likely switch to using the SVGs own ViewBox by default instead of it's size for the FXSVGCanvas.

[weisJ]

It has been some time now and I would like to pick up this PR again. @SonarSonic if you currently don't have the time to work on it I might be able to complete the work of the remaining issues. The feature doesn't need to be fully fletched on the first iteration.

Appreciate all the work you have been putting into getting support for JavaFX!

[SonarSonic]

Hey @weisJ
I think the implementation was in a good state, I think the main remaining issue was the resizing issue I mentioned in my last message. I'd be very happy for you to continue the work from here, as I don't have much time to work on this at the moment.