JDK-8157900 : Encapsulate JavaFX impl_* implementation methods in Node (peer and dirty)
  • Type: Enhancement
  • Component: javafx
  • Sub-Component: graphics
  • Affected Version: 9
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2016-05-25
  • Updated: 2017-05-11
  • Resolved: 2016-05-31
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 9
9Fixed
Related Reports
Blocks :  
JDK-8144585 - Encapsulate JavaFX impl_* implementation methods
Description
This is to make JDK-8144585 more manageable and easier for reviewer by breaking it into smaller pieces of work. The work is mainly focus on the encapsulation of peer and dirty structure and logic.
Comments
Changeset: 80cd1b32431e Author: ckyang Date: 2016-05-31 16:09 -0700 URL: http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/80cd1b32431e
31-05-2016
The .02 version looks good to me (with the above comment fix for "Shape" --> "Node") +1
31-05-2016
I missed another place in Node that I should replace Shape with Node. So here is the correct fix which I will commit if approved: /* * Store the singleton instance of the NodeHelper subclass corresponding * to the subclass of this instance of Node */
31-05-2016
Here is the revised webrev that includes all the feedback received from on .01: http://cr.openjdk.java.net/~ckyang/JDK-8157900/webrev.02/ Here is the delta between 01 and 02: http://cr.openjdk.java.net/~ckyang/JDK-8157900/webrev-01_02.diff/ OK, I will file a JIRA to clean up apps/experiments if I can't find an existing one.
31-05-2016
Everything else looks good to me. I'll do a quick check of the .02 webrev when it is available. Btw, the changes to apps/experiments/ConferenceScheduleApp point out the need to clean up apps/experiments. If there isn't one already, please file a follow-on issue to remove use of internal classes and methods from apps/experiments (they should already be gone from apps/toys and apps/samples). The only place we should use internals is in unit tests, robot tests, and performance tests.
31-05-2016
ShapeHelper should remain abstract since there is no instance of that class ever created (there is no initHelper() method for Shape to call). As a result you don't need to implement createPeer (it can be inherited as abstract from Node), and can change configShapeImpl to abstract.
31-05-2016
@Vadim: Thanks for the good catch. I will fix them all.
31-05-2016
The comment in the Node.java before "private NodeHelper nodeHelper = null;" is incorrect. Maybe move initHelper calls from the constructors to the initializer block for ParallelCamera and PerspectiveCamera? Also in CircleTest.java, ArcTest.java, TestNode.java, CubicCurveTest, EllipseTest, LineTest, PolygonTest, PolylineTest, QuadCurveTest, RectangleTest, SVGPathTest Text.java - unused import "+import javafx.scene.shape.CubicCurve;" Camera and PerspectiveCamera could use Node.isDirty and isDirtyEmpty directly without NodeHelper since they are in the javafx.scene package.
30-05-2016
Thanks for verifying it on Windows. Here is the revised webrev that addresses those problem and few other places that I missed: http://cr.openjdk.java.net/~ckyang/JDK-8157900/webrev.01/ Here is the delta between 00 and 01: http://cr.openjdk.java.net/~ckyang/JDK-8157900/webrev-00_01.diff/
29-05-2016
There are two failures that I see in testing (which was done on Windows) 1) The MediaView class throws NPE in the constructor 2) SwingNode doesn't run in Jigsaw mode. I'll review in detail next week.
28-05-2016
Please review the proposed fix: http://cr.openjdk.java.net/~ckyang/JDK-8157900/webrev.00/ I have filed 2 follow on JIRA (as bug) to improve our javadoc and runtime exception message on some of our JavaFX abstract classes where application shouldn't directly extend from: JDK-8158004 and JDK-8158006 I have also verified that this webrev passes full dev run in both JIGSAW and non-JIGSAW mode.
26-05-2016