On 12/15/2019 6:06 PM, Nir Lisker wrote:
Replying on the mailing list to the questions raised on GitHub.
The state of whether to use the computed center pivot or the value
of the pivot attribute is implicit with no way for an application
to know which it is, and no way to set it back to using the
computed center (i.e., the state is sticky once you set it).
Perhaps if you defined a null value as meaning "computed center"
then an app could at least reset it to the "computed center"
state, although there would still be no way for the app to know
that it was in that state.
In the JBS issue <https://bugs.openjdk.java.net/browse/JDK-8234712> I
alluded to this in point 5. I think that null should represent the
default (node center). However, if we use 3 doubles instead of a
Point3D we might need to use Double.NaN for this instead, which would
also be the default for this case. The docs will explain this.
Using Double.NaN as an out of band value would be odd, and probably not
what we want. In addition to the fact that it is somewhat artificial, it
would mean that X, Y, and Z would independently be treated as coming
from the set value or from the computed center. I guess this is one
argument for using a Point3D object, but as you note, there are other
drawbacks.
Do we need separate properties for scale pivot and center pivot?
I say yes, otherwise the enhancement will be very limited. I think of
this enhancement as adding pivot control to Rotate/Scale transitions,
and adding them to Node is a necessary (and desirable) step.
Yeah, I figured this was the case.
... you need to worry about what coordinate space the rotation
pivot is defined in. Perhaps if the rotation pivot were defined in
unscaled space, it might work.
Isn't it already? If I set the rotation pivot to the edge of the node,
then scale it down, then rotate, the rotation pivot would be outside
of the node's boundaries. In scaled space it would still be on the
edge. Or did I misunderstand you?
I think I was wrong in the comment I added to the PR.
What I meant is that you would need to define the coordinate space that
the pivot values are in, and it needs to be defined in a way that it is
consistent with current behavior. Today, the part of the matrix
transformation that does the scale and rotate, including the computed
pivot, is this:
T(pivot) * T(rotate) * T(scale) * T(-pivot) * object coords
As long as you do the following, it should work as expected:
T(pivotRot) * T(rotate) * T(-pivotRot) * T(pivotScale) * T(scale) *
T(-pivotScale) * object coords
Importantly, this will work exactly as it does today when pivotRot ==
pivotScale which is the case I was most concerned about.
In any case, I don't think that there's a single correct answer here.
It needs to be consistent with current behavior, and match what an
application would expect. I think it should not be a problem if defined
as above.
Should the pivot be specified as a |Point3D| or 3 separate
doubles? Separate doubles... there would be no out-of-band
|null| value to use
See my point above about Double.NaN.
OK, there is no "natural" out of band value that is likely to be
satisfying. We'd probably end up wanting a boolean that controls
computed versus explicit (either a single flag or one for each of
scalePivot and rotatePivot)
The doubles vs Point3D is an important choice. We might want to look
into the future even where Point3D (and 2D) could be made into an
Inline class with Valhalla, which will help with the performance
aspect. Binding to one of the coordinates is sill a problem there,
however.
I think this (doubles vs Point3D) is really the main question. I don't
know what the best answer is, but I'd like to hear from other developers.
-- Kevin
On Sat, Dec 14, 2019 at 6:25 PM Kevin Rushforth
<notificati...@github.com <mailto:notificati...@github.com>> wrote:
This will need discussion on the openjfx-dev mailing list. Here
are the questions that need to be resolved:
1.
The state of whether to use the computed center pivot or the
value of the pivot attribute is implicit with no way for an
application to know which it is, and no way to set it back to
using the computed center (i.e., the state is sticky once you
set it). Perhaps if you defined a null value as meaning
"computed center" then an app could at least reset it to the
"computed center" state, although there would still be no way
for the app to know that it was in that state.
2.
Do we need separate properties for scale pivot and center
pivot? A single pivot property would be easier to define, but
wouldn't allow you to set it from a |RotationTransition| and a
|ScaleTransition| if you wanted to apply both to the same
|Node|. With two separate properties, as you have defined it,
it is more flexible, but you need to worry about what
coordinate space the rotation pivot is defined in. The current
transform chain looks like this:
|T(layout+translate) * T(pivot) * T(rot) * T(scale) * T(-pivot) *
transform[0] * transform [1] ... |
Perhaps if the rotation pivot were defined in unscaled space, it
might work. The transform chain would then look like this:
|T(layout+translate) * T(pivotRot/scale) * T(rot) *
T(-pivotRot/scale) * T(pivotScale) * T(scale) * T(-pivotScale) *
transform[0] * transform [1] ... |
In any case, you need to think about the implications of having
one of scale or rotate being set explicitly and the other being
the computed center.
3. Should the pivot be specified as a |Point3D| or 3 separate
doubles? I can see pros / cons of either approach. Separate
doubles are more consistent with the the pivot values in the
|Rotate| and |Scale| |Transform| objects, and are easier to
using in binding. On the other hand, there would be no
out-of-band |null| value to use (see issue 1 above), so you
would need a boolean property for each of scale pivot and
rotation pivot to indicate whether the computed value or the
value of the pivot properties should be used. I don't think
that the fact that the rotation axis is defined as a |Point3D|
should have any bearing on whether the pivot should be so
defined. I'd probably lean towards separate doubles.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<https://github.com/openjdk/jfx/pull/53?email_source=notifications&email_token=AI5QOM6KRPRRP7VS5OUH6QLQYUCF7A5CNFSM4JR3TYY2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEG4GCGI#issuecomment-565731609>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AI5QOM4UKZQVDEN2A2HYYETQYUCF7ANCNFSM4JR3TYYQ>.
