[openjdk/jfx] 8234712: Add pivot properties for scale and rotation in Node, ScaleTransition and RotateTransition (#53)

Nir Lisker nlisker at gmail.com
Tue Dec 17 00:46:55 UTC 2019 

>
> 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
>

 I think that this is what the PR does already. Line 5061 onward is:

localToParentTx = localToParentTx.deriveWithTranslation(rotPivotX,
rotPivotY, rotPivotZ)
                                 .deriveWithRotation(rotAngle, rotAxisX,
rotAxisY, rotAxisZ)
                                 .deriveWithTranslation(-rotPivotX,
-rotPivotY, -rotPivotZ);
...
localToParentTx = localToParentTx.deriveWithTranslation(scalePivotX,
scalePivotY, scalePivotZ)
                                 .deriveWithScale(getScaleX(), getScaleY(),
getScaleZ())
                                 .deriveWithTranslation(-scalePivotX,
-scalePivotY, -scalePivotZ);

On Tue, Dec 17, 2019 at 2:07 AM Kevin Rushforth <kevin.rushforth at oracle.com>
wrote:

>
>
> 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 <notifications at 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.
>>
>>    1. 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>
>> .
>>
>
>

More information about the openjfx-dev mailing list