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.
Yes, I will change Transform.add/remove methods to use package-scope.
13-05-2016
+1 pending the change suggested by Jim
13-05-2016
The Transform.add/remove methods should probably use package-scope, but other than that it looks fine...
12-05-2016
I have filed an enhancement JIRA, JDK-8156850, to better design ImmutableTransform.
12-05-2016
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/
12-05-2016
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).
08-05-2016
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/