JDK-8155762 : Encapsulate JavaFX impl_* implementation methods in transform package
  • Type: Enhancement
  • Component: javafx
  • Sub-Component: graphics
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2016-04-29
  • Updated: 2016-05-13
  • Resolved: 2016-05-13
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.
Related Reports
Blocks :  
This is to make JDK-8144585 more manageable and easier for reviewer by breaking it into pieces of work. 
Changeset: f3a889ef836b Author: ckyang Date: 2016-05-13 11:02 -0700 URL: http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/f3a889ef836b

Yes, I will change Transform.add/remove methods to use package-scope.

+1 pending the change suggested by Jim

The Transform.add/remove methods should probably use package-scope, but other than that it looks fine...

I have filed an enhancement JIRA, JDK-8156850, to better design ImmutableTransform.

1) Transform.java: No good reason for this change. In fact the old code just instantiates directly so I will make this change. 2) TransformHelper.java: Format as suggested, plus deleted forceInit() method to use Utils.forceInit() instead. Here is webrev.01: http://cr.openjdk.java.net/~ckyang/JDK-8155762/webrev.01/ And here is the diff between webrev.00 and webrev.01: http://cr.openjdk.java.net/~ckyang/JDK-8155762/webrev00-01.diff/

Transform.java, line 2172 & 2187 - is there a reason to forward to the factory rather than just instantiating directly? TransformHelper.java, line 108 & 112 - [formatting] perhaps move the m[xyz],tx to the next line for consistency between the two (like 68 & 76)? Other than that, I'd like to see a better design for the Immutable transform. It's outside the scope of this bug fix, but it might be done better by simply delegating to an Affine instance, or by creating a new public class above Affine that exposes just the non-mutating utility methods of Affine and leaving Affine to be the modifiable version. (It wouldn't be called Immutable since it has a mutable sub-class, but it can be effectively Immutable and documented to be intentionally so without having to imply a guarantee in the name).

Please review the proposed fix. This webrev passed dev-build.sh run on Windows and Mac. http://cr.openjdk.java.net/~ckyang/JDK-8155762/webrev.00/