Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8160325

[SWT] Provide public API to access FXCanvas for embedded scene

    Details

    • Subcomponent:
    • CPU:
      generic
    • OS:
      other

      Description

      A DESCRIPTION OF THE REQUEST :
      Up to now, there is no way to access the enclosing FXCanvas for a given (embedded) Scene. Please provide some public API via which the enclosing FXCanvas of an embedded Scene can be obtained. That could well be a static method within FXCanvas.

      JUSTIFICATION :
      In the context of the JavaFX-SWT-integration there are use cases where the enclosing of FXCanvas of the embedded scene needs to be obtained. Up to now this is only possible using reflection.


      CUSTOMER SUBMITTED WORKAROUND :
      Up to now, we can only access the enclosing FXCanvas via the scene window using the following code (based on reflection):

      protected FXCanvas getFXCanvas(Window window) {
        if (window != null) {
          // Obtain FXCanvas by accessing outer class
          // of FXCanvas$HostContainer
          FXCanvas canvas = ReflectionUtils.getPrivateFieldValue(ReflectionUtils. <Object> getPrivateFieldValue(window,"host"), "this$0");
          return canvas;
        }
        return null;
      }


      1. JDK-8160325_12_08_16.patch
        14 kB
        Kevin Rushforth
      2. JDK-8160325_28-07-16.patch
        35 kB
        Kevin Rushforth
      3. JDK-8160325_28-07-16.revised.patch
        14 kB
        Kevin Rushforth

        Activity

        Hide
        kcr Kevin Rushforth added a comment -
        Attached patch provided by Alexander Nyssen
        Show
        kcr Kevin Rushforth added a comment - Attached patch provided by Alexander Nyssen
        Hide
        kcr Kevin Rushforth added a comment -
        I note that without this API there will be no way in JDK 9 to get this information, since the com.sun.javafx.stage package is not exported publicly.
        Show
        kcr Kevin Rushforth added a comment - I note that without this API there will be no way in JDK 9 to get this information, since the com.sun.javafx.stage package is not exported publicly.
        Hide
        kcr Kevin Rushforth added a comment -
        Attached updated patch provided by Alexander.
        Show
        kcr Kevin Rushforth added a comment - Attached updated patch provided by Alexander.
        Hide
        kcr Kevin Rushforth added a comment -
        The proposed public API looks fine to me:

            public static FXCanvas getFXCanvas(Scene scene);

        I have a couple comments on the proposed API docs:

        1. There are a few typos and style issues:

        Style issue:

             * Retrieves the {@code FXCanvas} embedding the given {@code Scene},
             * i.e. the {@code FXCanvas}, to which the given {@code Scene} was

        we usually avoid abbreviations such as 'i.e' and there is a comma in the wrong place, so I recommend the following slight modification:

             * that is, the {@code FXCanvas} to which the given {@code Scene} was

        Style issue:

             * @param scene The {@code Scene} whose embedding {@code FXCanvas}
             * instance is to be retrieved

        'the' should be lower-case

        Typo:

             * @return the {@code FXCanvas} to which the given {@code Scene} is
             * attached, or null of the given {@code Scene} is not attached to an

        should be "if" the given Scene ...


        2. You need to add the '@since 9' javadoc tag to the new API


        I also have one comment on the implementation:

        3. Rather than using reflection and setAccessible in the implementation, please add a public getHostContainer method to EmbeddedWindow (since it is an internal method, there is no concern with doing that -- it isn't API).
        Show
        kcr Kevin Rushforth added a comment - The proposed public API looks fine to me:     public static FXCanvas getFXCanvas(Scene scene); I have a couple comments on the proposed API docs: 1. There are a few typos and style issues: Style issue:      * Retrieves the {@code FXCanvas} embedding the given {@code Scene},      * i.e. the {@code FXCanvas}, to which the given {@code Scene} was we usually avoid abbreviations such as 'i.e' and there is a comma in the wrong place, so I recommend the following slight modification:      * that is, the {@code FXCanvas} to which the given {@code Scene} was Style issue:      * @param scene The {@code Scene} whose embedding {@code FXCanvas}      * instance is to be retrieved 'the' should be lower-case Typo:      * @return the {@code FXCanvas} to which the given {@code Scene} is      * attached, or null of the given {@code Scene} is not attached to an should be "if" the given Scene ... 2. You need to add the '@since 9' javadoc tag to the new API I also have one comment on the implementation: 3. Rather than using reflection and setAccessible in the implementation, please add a public getHostContainer method to EmbeddedWindow (since it is an internal method, there is no concern with doing that -- it isn't API).
        Hide
        kcr Kevin Rushforth added a comment -
        I propose to add this new method to JavaFX for JDK 9. The risk is low since it is a new method that returns information already available to the implementation, but which is inaccessible to applications in JDK 9 due to its being internal API.

        It is being proposed by an external contributor and should be completed by Aug 16.
        Show
        kcr Kevin Rushforth added a comment - I propose to add this new method to JavaFX for JDK 9. The risk is low since it is a new method that returns information already available to the implementation, but which is inaccessible to applications in JDK 9 due to its being internal API. It is being proposed by an external contributor and should be completed by Aug 16.
        Hide
        kcr Kevin Rushforth added a comment - - edited
        One more style issue (spacing)

        EmbeddedWindow.java:

        + public HostInterface getHost(){

        there should be a space between the ')' and the '{'.

        Also, regarding #3 above, this can be implemented by adding a default (package) scope 'fxCanvas' field in the inner class initialized to 'FXCanvas.this' and you will be able to access it directly, since an instance of an inner class can always get access to the instance of the enclosing class.

            private class HostContainer implements HostInterface {
                final FXCanvas fxCanvas = FXCanvas.this;
                ...
        Show
        kcr Kevin Rushforth added a comment - - edited One more style issue (spacing) EmbeddedWindow.java: + public HostInterface getHost(){ there should be a space between the ')' and the '{'. Also, regarding #3 above, this can be implemented by adding a default (package) scope 'fxCanvas' field in the inner class initialized to 'FXCanvas.this' and you will be able to access it directly, since an instance of an inner class can always get access to the instance of the enclosing class.     private class HostContainer implements HostInterface {         final FXCanvas fxCanvas = FXCanvas.this;         ...
        Hide
        kcr Kevin Rushforth added a comment -
        Attached updated version of the patch from Alexander N
        Show
        kcr Kevin Rushforth added a comment - Attached updated version of the patch from Alexander N
        Hide
        kcr Kevin Rushforth added a comment -
        Here are the notes from the patch itself (since they cannot be part of the commit message, but belong here).

        ---------------------------
        - Provided static FXCanvas#getFXCanvas(Scene) method to obtain the FXCanvas instance embedding the given Scene instance.
        - Added EmbeddedWindow.getHost() so the HostInterface can be retrieved.
        - Added FXCanvasTest with a test method to test correct behavior of FXCanvas#getFXCanvas(Scene).
        - Introduced SwtTest JUnit MethodRule to have more concise tests and ensure it is also used by SWTCursorsTest.
        Show
        kcr Kevin Rushforth added a comment - Here are the notes from the patch itself (since they cannot be part of the commit message, but belong here). --------------------------- - Provided static FXCanvas#getFXCanvas(Scene) method to obtain the FXCanvas instance embedding the given Scene instance. - Added EmbeddedWindow.getHost() so the HostInterface can be retrieved. - Added FXCanvasTest with a test method to test correct behavior of FXCanvas#getFXCanvas(Scene). - Introduced SwtTest JUnit MethodRule to have more concise tests and ensure it is also used by SWTCursorsTest.
        Hide
        kcr Kevin Rushforth added a comment -
        Changeset: 36127d196ffe
        Author: kcr
        Date: 2016-08-18 11:36 -0700
        URL: http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/36127d196ffe

        8160325: [SWT] Provide public API to access FXCanvas for embedded scene
        Reviewed-by: azvegint
        Contributed-by: alexander@nyssen.org
        Show
        kcr Kevin Rushforth added a comment - Changeset: 36127d196ffe Author: kcr Date: 2016-08-18 11:36 -0700 URL: http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/36127d196ffe 8160325: [SWT] Provide public API to access FXCanvas for embedded scene Reviewed-by: azvegint Contributed-by: alexander@nyssen.org

          People

          • Assignee:
            kcr Kevin Rushforth
            Reporter:
            webbuggrp Webbug Group
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Due:
              Created:
              Updated:
              Resolved: