JDK-8157350 : Encapsulate impl_ methods in Shapes related classes
  • Type: Enhancement
  • Component: javafx
  • Sub-Component: graphics
  • Affected Version: 9
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2016-05-19
  • Updated: 2016-05-24
  • Resolved: 2016-05-21
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 :  
Description
This is to make JDK-8144585 more manageable and easier for reviewer to review by breaking the work into smaller pieces.
Comments
Changeset: 75f8a4f1d1b9 Author: ckyang Date: 2016-05-20 16:56 -0700 URL: http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/75f8a4f1d1b9
21-05-2016

+1 on version .00 with the change to ArcHelper.java that Jim noticed (and, if you want, you can add my recommended additional comment in ShapeHelper).
20-05-2016

We looked through all the others and Arc was the only one.
20-05-2016

It sounds like you have a reason for the current helper paradigm as we move forward so that is good. The naming anomalies aren't functional, but annoying. So, no real open issues from my perspective. Note that Arc was the only helper I looked at so I can't say that there aren't other naming typos in there...
20-05-2016

I am OK with version .00 after the changes pointed out above. Let's make sure that the openjfx-dev thread with Jim's comments are addressed before proceeding.
20-05-2016

One thing I missed in my review that Jim pointed out is that there is a typo in ArcHelper.java: 49 public static void initHelper(Arc quadCurve) { 50 setHelper(quadCurve, getInstance()); 51 } that should be "arc" rather than "quadCurve".
20-05-2016

Looks good. Two minor comments (no need for a new webrev if you decide to fix them). ShapeAccessor.java: * You can also add the "TODO" comment here, since these will move to NodeAccessor: 115 ShapeHelper getHelper(Shape shape); 116 void setHelper(Shape shape, ShapeHelper shapeHelper); * You have an extra blank line or two at the end of most of the new Helper classes (e.g., ArcHelper.java, ArcToHelper.java, CircleHelper.java) [edited to remove the +1 while discussion is ongoing]
20-05-2016

Jim raised some points on the openjfx-dev list, so hold off pushing for now.
20-05-2016

Please review the proposed fix. In this fix I also did a minor fix to getMXWindowType() in Window and Stage including the helper classes to match the pattern we will use for overriding helper method. webrev: http://cr.openjdk.java.net/~ckyang/JDK-8157350/webrev.00/
20-05-2016