> > 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.rushfo...@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 <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. >> >> 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> >> . >> > >