I didn't page in enough of the discussion before. I remember now why we
need separate pivots for rotate and scale, so thank you for reminding
me. Btw, I don't think that necessarily means we need a separate switch
for whether to use computed vs explicit for each (but that would be a
possibility).
I agree with the points raised about relative versus "absolute". While
it is an intriguing idea, it probably doesn't fit here.
While it is a clever idea to use an ObjectProperty<Double> rather than a
DoubleProperty so we have an out of band value to use for "compute this"
I don't think that we should do it that way for a few reasons. First,
it's inconsistent with all other number based properties, in a way that
could cause usability issues (e.g., you couldn't just pass
pivotXProperty() directly to a method that expects a DoubleProperty).
Second, I don't like the implications of having separate "auto-compute"
flags on each of X, Y, and Z. Third, is the documentation issue you
mentioned.
So amending my earlier answers to the three questions:
1. A single boolean property for whether to use a compute pivot (the
default) vs an explicit pivot; this would apply to both rotate and scale
pivots.
2. 3 separate doubles (using DoubleProperty).
3. Yes. Separate rotatePivot{X,Y,Z} and scalePivot{X,Y,Z} properties.
-- Kevin
On 8/21/2021 6:19 AM, Nir Lisker wrote:
In the previous time this was discussed I suggested a 3rd option, in
addition to ObjectProperty<Point3D> and 3 DoubleProperties, which is 3
ObjectProperty<Double>s.
Advantages:
1. Allows using null to remove any specified user values (null would
also be the default), similar to how we would do it with
ObjectProperty<Point3D>. No need for another property like a boolean
value.
2. Allows changing individual axes and binding to each axis
independently, which was a disadvantage of Point3D.
3. With how Valhalla is coming along, it's highly likely that there
would be no auto-boxing cost as Double will behave like a primitive.
Disadvantages:
1. Using null as the special value does not indicate well that the
default behavior is using the center of the node, but there is no
indication of it now, except in documentation which will be done anyway.
2. ObjectProperty<Double> does not have the specialized methods
inherited from DoubleExpression (like #add), so it is more difficult
to work with mathematically.
This addresses questions 1 and 2. The 2nd point in the disadvantages
makes this not too appealing for me, but the other options are also
problematic.
As for question 3, I think we have to. The main goal of the PR is
adding pivot properties to ScaleTransition and RotateTransition. The
various transitions work by changing values in Node (FadeTransform
changes opacity, RotateTransition changes rotate and rotationAxis
etc.). If we don't have separate pivots for rotation and scale in
Node, how can we implement these in the transitions? They surely can't
use the same one, both because it will be confusing and there will be
synching issues. Think about creating 1 RotateTransition and 1
ScaleTransition with different pivots. The order in which you start
them will matter because one will change the other (and if forceSync
is enabled then one will override the other mid-transition).
Having these pivots on Node for direct usage is convenient, but I see
it as a secondary goal. It is more of a necessity for the transitions.
Notice that the JBS issue [1] is aggregating 3 issues because they are
interdependent.
As for Michael's proposal, it has the benefit of getting rid of the
special computed value of the center of the node and making it clear
what the default is.
I'm not sure what Michael means by "absolute coordinates", I assume
it's relative to the parent, and that "relative to the layout bounds"
means relative to the node. This way, setting to 0.5 with "relative to
the layout bounds" will indeed mean the center of the node.
I find some inconsistency issues with the relativePivot idea:
1. The Scale, Rotate, and Shear transforms use the parent coordinates.
That means that if I set a pivoted scale transform on the node, and
then use a pivoted scale transition (keeping the default), the pivot
will not be the same. This would be very confusing I think.
2. TranslateTransition has a toX property (and Y and Z) that always
uses the parent coordinates. That means that if I use a translate
transition and a scale transition (keeping the default), I will be
working in 2 coordinate systems simultaneously. Also confusing for me.
3. Switching between local and global coordinate systems is a more
general issue, and solving it locally just for pivots of 2 transitions
seems incomplete and out of place to me in this patch.
To expand on point 3, I see 3 generally useful systems: node, parent,
and scene. We have methods that transform from one to the other.
Despite those, I also find it a lot of work to transform between
coordinate systems. This leads me to believe that the idea of toggling
between coordinate systems tries to solve a broader issue, one that
touches both transforms and transitions (including node properties).
As a result, I think that we should implement the current proposal of
adding pivots by using the current coordinate system that everything
else uses (parent), and then think about better ways of handling
coordinate system transformations more generally.
[1] https://bugs.openjdk.java.net/browse/JDK-8234712
<https://bugs.openjdk.java.net/browse/JDK-8234712>
On Thu, Aug 19, 2021 at 11:35 PM Kevin Rushforth
<kevin.rushfo...@oracle.com <mailto:kevin.rushfo...@oracle.com>> wrote:
The idea of adding explicit pivot properties was met with general
agreement as long as we preserve compatibility (meaning that an
auto-computed pivot at the center of the bounds must remain the
default).
The questions are around what form the API should take. They
basically boil down to:
1. How do we indicate whether the pivot is computed vs explicitly
specified?
2. Should the pivot position be 3 doubles or a Point3D
3. Do we need a separate pivot for rotation vs scaling
My quick thoughts (although I might be able to be persuaded
otherwise) are:
1) a boolean property
2) 3 separate doubles
3) no
Michael's proposal of a couple months ago [1] would match those
answers, and is an interesting approach that I hadn't considered
before. The question then is whether want the additional
flexibility that it provides and whether it is worth the
additional implementation, documentation, and testing that would
be needed. It can be a discussion point. I'd like to know what Nir
thinks of that idea.
I think we can discuss the details of the API in the PR, although
while it is Draft, many people will miss the discussion (since it
doesn't get sent to the mailing list), so it might be better to at
get high-level agreement on the answers to the outstanding API
questions on this list?
-- Kevin
[1] https://github.com/openjdk/jfx/pull/53#issuecomment-822667918
<https://urldefense.com/v3/__https://github.com/openjdk/jfx/pull/53*issuecomment-822667918__;Iw!!ACWV5N9M2RV99hQ!esInc10DOeQ3hwEawEKaiNBYfjN8QjOgrc3Q9I-wIfOuXf7SdcHy23A8jJAEmTS2AMW_$>
On 8/19/2021 12:03 PM, Nir Lisker wrote:
I'd like to see pivot properties move forward.
The PR seems to be a bit stale, as it has been sitting around for
almost two years.
Me too, but there was no decision as to the implementation type.
We can continue the discussion on the PR at this point I think.
On Fri, Aug 13, 2021 at 2:32 AM Michael Strauß
<michaelstr...@gmail.com <mailto:michaelstr...@gmail.com>> wrote:
I'd like to see pivot properties move forward.
The PR seems to be a bit stale, as it has been sitting around for
almost two years.
PR: https://github.com/openjdk/jfx/pull/53
<https://urldefense.com/v3/__https://github.com/openjdk/jfx/pull/53__;!!ACWV5N9M2RV99hQ!dpO9UK8458NQKXiiWkGAxJV2D2wUpaj0Lj6CGeQEei1FoV7bgKcV_3q4Szn7X8UYHGUa$>
Am Fr., 30. Juli 2021 um 14:57 Uhr schrieb Kevin Rushforth
<kevin.rushfo...@oracle.com <mailto:kevin.rushfo...@oracle.com>>:
>
> Now that JavaFX 17 is in RDP2, we can turn more attention
to bug fixes
> and enhancement requests for JavaFX 18. It's the summer, so
there may be
> delays as some people are out at various times (including
me), but I
> would like to get some of the outstanding enhancement
requests moving
> over the next few weeks.
>
> Specifically, I'd like to see the following proceed:
>
> * Transparent backgrounds in WebView
> JBS: https://bugs.openjdk.java.net/browse/JDK-8090547
<https://bugs.openjdk.java.net/browse/JDK-8090547>
> PR: https://github.com/openjdk/jfx/pull/563
<https://urldefense.com/v3/__https://github.com/openjdk/jfx/pull/563__;!!ACWV5N9M2RV99hQ!dpO9UK8458NQKXiiWkGAxJV2D2wUpaj0Lj6CGeQEei1FoV7bgKcV_3q4Szn7XyMUPibt$>
>
> * Add DirectionalLight
> JBS: https://bugs.openjdk.java.net/browse/JDK-8234921
<https://bugs.openjdk.java.net/browse/JDK-8234921>
> PR: https://github.com/openjdk/jfx/pull/548
<https://urldefense.com/v3/__https://github.com/openjdk/jfx/pull/548__;!!ACWV5N9M2RV99hQ!dpO9UK8458NQKXiiWkGAxJV2D2wUpaj0Lj6CGeQEei1FoV7bgKcV_3q4Szn7X4KG3EaB$>
>
> * CSS pseudoclasses :focus-visible and :focus-within
> https://bugs.openjdk.java.net/browse/JDK-8268225
<https://bugs.openjdk.java.net/browse/JDK-8268225>
> PR: https://github.com/openjdk/jfx/pull/475
<https://urldefense.com/v3/__https://github.com/openjdk/jfx/pull/475__;!!ACWV5N9M2RV99hQ!dpO9UK8458NQKXiiWkGAxJV2D2wUpaj0Lj6CGeQEei1FoV7bgKcV_3q4Szn7XwfLuVHR$>
>
> * Improve property system to facilitate correct usage
(minus the binary
> incompatible API change)
> JBS: https://bugs.openjdk.java.net/browse/JDK-8268642
<https://bugs.openjdk.java.net/browse/JDK-8268642>
> PR: https://github.com/openjdk/jfx/pull/590
<https://urldefense.com/v3/__https://github.com/openjdk/jfx/pull/590__;!!ACWV5N9M2RV99hQ!dpO9UK8458NQKXiiWkGAxJV2D2wUpaj0Lj6CGeQEei1FoV7bgKcV_3q4Szn7X_YUng6V$>
(Draft)
>
> And maybe the following:
>
> * Add CSS themes as a first-class concept (need a consensus
on how to
> proceed)
>
> * Undecorated interactive stage style (still in early
discussion, but
> the concept looks interesting and useful)
>
> There are probably others I'm forgetting.
>
> Each of the above should be discussed in their own thread
on openjfx-dev
> rather than a reply to this thread.
>
> -- Kevin
>
>