Improving TransformGroup / Transform3D

I know TransformGroup and Transform3D are a very good way to get performances and Transform trees are sometimes useful… But that code is just not cool :


            TransformGroup objTranslate = new TransformGroup();
            Transform3D translate = new Transform3D();
            translate.setTranslation(new Vector3f(-2f, -5f, 0f));
            objTranslate.setTransform(objTranslate);
            
            TransformGroup objRotate = new TransformGroup();
            Transform3D rotate = new Transform3D();
            rotate.rotXYZ((float)Math.toRadians(270f), 0f, 0f);
            objRotate.setTransform(rotate);
            
            TransformGroup objScale = new TransformGroup();
            Transform3D scale = new Transform3D();
            scale.setScale(0.06f);
            objScale.setTransform(scale);
            
            objScale.addChild(amodel);
            objRotate.addChild(objScale);
            objTranslate.addChild(objRotate);
            addChild(objTranslate);

It just does translate, then rotate, then scale, the “amodel” Shape3D.
I already did a proposal for cascade calls, which would reduce the code to :


            TransformGroup objTranslate = new TransformGroup();
            Transform3D translate = new Transform3D();
            translate.setTranslation(new Vector3f(-2f, -5f, 0f));
            objTranslate.setTransform(objTranslate);
            
            TransformGroup objRotate = new TransformGroup();
            Transform3D rotate = new Transform3D();
            rotate.rotXYZ((float)Math.toRadians(270f), 0f, 0f);
            objRotate.setTransform(rotate);
            
            TransformGroup objScale = new TransformGroup();
            Transform3D scale = new Transform3D();
            scale.setScale(0.06f);
            objScale.setTransform(scale);
            
            addChild(objTranslate).addChild(objRotate).addChild(objScale).addChild(amodel);

Hmm… still not that good. maybe an helper method in Transform3D something like getTG() that creates a new transform group and set the Transform3D… that would reduce code to :


            Transform3D translate = new Transform3D();
            translate.setTranslation(new Vector3f(-2f, -5f, 0f));
            
            Transform3D rotate = new Transform3D();
            rotate.rotXYZ((float)Math.toRadians(270f), 0f, 0f);
            
            Transform3D scale = new Transform3D();
            scale.setScale(0.06f);
            
            addChild(translate.getTG()).addChild(rotate.getTG()).addChild(scale.getTG()).addChild(amodel);

And now we could add an addChild(Transform3D t) method so it calls getTG() automatically, that would reduce code to :


            Transform3D translate = new Transform3D();
            translate.setTranslation(new Vector3f(-2f, -5f, 0f));
            
            Transform3D rotate = new Transform3D();
            rotate.rotXYZ((float)Math.toRadians(270f), 0f, 0f);
            
            Transform3D scale = new Transform3D();
            scale.setScale(0.06f);
            
            addChild(translate).addChild(rotate).addChild(scale).addChild(amodel);

And we could also provide cool constructors for Transform3D :


            Transform3D translate = new Transform3D(new Vector3f(-2f, -5f, 0f));
            Transform3D rotate = new Transform3D((float)Math.toRadians(270f), 0f, 0f);
            Transform3D scale = new Transform3D(scale.setScale(0.06f));
            addChild(translate).addChild(rotate).addChild(scale).addChild(amodel);

4 lines vs 12 lines with the original version !! And much clearer… Of course it breaks a bit of API strict definitions but It could be really convenient. If there’s no objection I may implement it.

Note about performances : implementation requires that Transform3D contains a “TransformGroup tg = null” field. This doesn’t consume more memory as long as tg equals null, right ? And it would lazy-create it only when needed

Note about bugs… If you call addChild(translate) two times it would fail. So the solution is to catch the IllegalScenegraphOperationException in the addChild(Transform3D t) method and to call the transform3d.getNewTG() method, which instanciate a new TransformGroup… and then we would need a Vector in the Transform3D class

Then when you would call something like rotXYZ(), setTranslation() or setScale(), the transform3D would update the TransformGroup (transformGroup.setTransform(this)) automagically so there are no more surprises…

Pheww… a whole programme, heh ? ;D

I’d rather let the transform groups visible :

Rather often you have to update the transforms of your models (for many parts of the games that aren’t static) to move things around.
Usually this means having transform nodes as instance variables of “game objects” to keep a convenient access on them.


TransformGroup martianPosition = new TransformGroup();
TransformGroup martianRotation = new TransformGroup();
BranchGroup martianModel = loader.load(...)

Martian() {
}

void update(float deltaTime){
   Transform3D translate = martianPosition.getTransform();
   translate.setTranslation(...);
   martianPosition.setTransform(translate);
}

Anyway you syntax is fine for static elements, so I’d propose a slightly different version of it :

it would be to have a helper class “StaticModel” with methods like :


scene.addChild(new StaticModel().translate(trans).rotate(rot).scale(s).addChild(model).getRoot());

Every method would return the same StaticModel, and allow for compact chained commands.

That way the API remains compatible, and we have a nice “core” utility for initializing simple models (that can be extended to include transforms as well)

What do you think ?

Lilian

OK for the StaticModel thing, but my idea :

  • let the transform groups visible : you can not use addChild(Transform3D t) and let the Vector to null
  • works too for dynamic thingies : just keep references like you did with TransformGroup… the thing is you don’t need to recreate Transform3D each time (object creation/destruction is waste of time).
    So both would be good.

[quote="<MagicSpark.org [ BlueSky ]>,post:3,topic:27554"]
OK for the StaticModel thing, but my idea :

  • let the transform groups visible : you can not use addChild(Transform3D t) and let the Vector to null
    [/quote]
    ??? sorry I don’t understand

[quote="<MagicSpark.org [ BlueSky ]>,post:3,topic:27554"]

  • works too for dynamic thingies : just keep references like you did with TransformGroup… the thing is you don’t need to recreate Transform3D each time (object creation/destruction is waste of time).
    [/quote]
  • you don’t have to recreate your T3D each time, just get it from the group when needed, than set in again (see my update() sample method)

Lilian :slight_smile:

??? sorry I don’t understand
[/quote]
I mean you don’t have to use these features you can still use TransformGroup the usual way

  • you don’t have to recreate your T3D each time, just get it from the group when needed, than set in again (see my update() sample method)
    [/quote]
    Ah yeah I see… Hmm… anyway, a :

Would be cooler. And if martianPosition was a Transform3D, a :

…would be even more convenient.

I’m very for code compaction in the case of using TransformGroug/Transform3D.

How about a method to rotate/scale without loosing translation/scaling, since it can be done in one transformationmatrix.[/li]
And I support the idea to link a Transform3D in the other direction with it’s TransformGroup to save code lines like “tg.setTranslation(t)”.

How about a method/contructor in Transform3D looking like that:


t3d.transform(float transX, float transY, float transZ, float rotX, float rotY, float rotZ, float scale)
and
t3d.transform(float transX, float transY, float transZ, float rotX, float rotY, float rotZ, float scale, boolean setToZero)

And of course an overloaded method with a boolean flag “setToZero” for each of the other translate/rotate/scale methods indicating if the translation/rotation/scaling is lost with this method (rotatin deletes translation and scaling).

What do you think?

Yeah bun in which order would these transformations be applied ?

[quote="<MagicSpark.org [ BlueSky ]>,post:7,topic:27554"]
Yeah bun in which order would these transformations be applied ?
[/quote]
Hmm… Good question. I think (scale->rotate->translate) is the most common order in use, so that only the group itself is scaled by some factor, then rotated around its own axis and finally all this is simply translated.

But maybe you could tell the order by an (optional) parameter in a Transform3D-contructor with a default of (scale->rotate->translate).

Watch this skeleton:


public class Transform3D
{
    public enum TransformationOrder
    {
        SRT,
        STR,
        RST,
        RTS,
        TSR,
        TRS;
    }
    
    private TransformationOrder transOrder = null;
    
    ...
    
    public Transform3D()
    {
        ...
        this.transOrder = TransformationOrder.SRT;
        ...
    }
    
    public Transform3D2(TransformationOrder transOrder)
    {
        this();
        
        this.transOrder = transOrder;
    }

Seems a bit complicated… I prefer the idea of a StaticModel, like arne suggested.

[quote="<MagicSpark.org [ BlueSky ]>,post:9,topic:27554"]
Seems a bit complicated… I prefer the idea of a StaticModel, like arne suggested.
[/quote]
I don’t think these two ideas rule each others out. I understand this StaticModel thing the way, that it is for transform initialization. But combined with my idea you could dynamically change the scale, translation and rotation on one object without affecting the others and control the transform order.

And I don’t think it is really complicated. It will be more code in the Transform3D class than there is now, of course. But Bor the user of this class it will be quite simple.

I don’t think these two ideas rule each others out. I understand this StaticModel thing the way, that it is for transform initialization. But combined with my idea you could dynamically change the scale, translation and rotation on one object without affecting the others and control the transform order.

And I don’t think it is really complicated. It will be more code in the Transform3D class than there is now, of course. But Bor the user of this class it will be quite simple.
[/quote]
Maybe it’s OK, but I think there are already many many methods in the Transform3D class it would be useful to review them…

[quote="<MagicSpark.org [ BlueSky ]>,post:11,topic:27554"]
Maybe it’s OK, but I think there are already many many methods in the Transform3D class it would be useful to review them…
[/quote]
Hmm… OK, seems like most od these methods are already there. So my only two suggestion that remain are, to make most methods not final to allow effective extensions. And to add some constructors that mirror the set(…) methods that exist in this class.

Why don’t you hide the “man in the middle” class that is Transform3D? The Xith3d scenegraph as a lot of “middle-man” classes that are allways present in the code (an heritage of java3d) but most of the time we don’t need them to be there and it’s not just TransformGroup/Transform3D but thats another subject.

My sugestion would be:


TransformGroup objTranslate = new TransformGroup();
objTranslate.setTranslation(new Vector3f(-2f,-5f,0f));
          
TransformGroup objRotate = new TransformGroup();
objRotate.setRotation(new AxisAngle4f(Math.toRadians(270f), 0f, 0f,1f));
           
TransformGroup objScale = new TransformGroup();
objScale.setScale(0.06f);

Shape3D shape = new Shape3D(); 
shape.setAppearance(...);
shape.setGeometry(...);

this.addChild(objTranslate).addChild(objRotate).addChild(objScale).addChild(shape);

An alternative:


TransformGroup objTranslate = new TransformGroup();
objTranslate.setTransform(Matrix4f.getTranslationMatrix(new Vector3f(-2f,-5f,0f)));
          
TransformGroup objRotate = new TransformGroup();
objRotate.setTransform(Matrix4f.getRotationMatrix(new AxisAngle4f(Math.toRadians(270f), 0f, 0f,1f)));
           
TransformGroup objScale = new TransformGroup();
objScale.setTransform(Matrix4f.getScaleMatrix(0.06f));

Shape3D shape = new Shape3D(); 
shape.setAppearance(...);
shape.setGeometry(...);

this.addChild(objTranslate).addChild(objRotate).addChild(objScale).addChild(shape);

However i don’t know why you would want to have 3 transform groups when you could do anything with just one:


TransformGroup objTransform  = new TransformGroup();
Matrix4f matrix = new Matrix4f();
matrix.setTranslationPart(new Vector3f(-2f,-5f,0f));
matrix.setRotationPart(new AxisAngle4f(Math.toRadians(270f), 0f, 0f,1f));
matrix.setScalePart(0.06f);
objTransform.setTransform(matrix);

Or width:


TransformGroup objTransform  = new TransformGroup();
Matrix4f matrix = new Matrix4f();
matrix.setTranslation(new Vector3f(-2f,-5f,0f));
matrix.multiplyRotation(new AxisAngle4f(Math.toRadians(270f), 0f, 0f,1f));
matrix.multiplyScale(0.06f);
objTransform.setTransform(matrix);

If you want to change something later you only have to do this:


Matrix4f matrix = objTransform.getTransform();
// increment rotation by 1 degree
AxisAngle4f rotation = matrix.getAxisAngleRotationPart();
rotation.increment(Math.toRadeans(1f));
matrix.setRotationPart(rotation);
objTransform.setTransform(matrix);

Or better yet:


// increment rotation by 1 degree
objTransform.incrementRotationPart(Math.toRadeans(1f))

I this this would have been nive in the beginning of Xith3D. Now it would be nice as an addition, because it would brake several existing Xith3D games out there. This “objTranslate.setTranslation(new Vector3f(-2f,-5f,0f));” thing (and all the other methods taken from Transform3D) would just create a Transform3D instance in the TransformGroup if it doesn’t exist and set the translation on it.

This is what I was talking about.

This would transfer the “man in the middle” thing to the Matrix4f. In my opinion it is really ok and better than the current way. But never forget, that you would brake existing games.

Watch the setRotation(Matrix3f), setTranslation(Vector3f), setScale(float) and set(Quat4f, Vector3f, float) methods of Transform3D. They are doing what you want. Of course they would have to be copied to the TransformGroup class to fulfill your plan.

This can be done by:


t3d.setScale(t3d.getScale() * 0.06f);

This seems to be more complicated, but it is just more flexiple. If you want to add a scale you couldn’t use your method. And it would maybe blow up the already large inferface.

I suggested earlier to add a “scale(float)” method, which would do exactly what your setScale(float) method does. But I accepted, that setScale(getScale()*s) is more flexible and not that much shorter.

@Qudus : the “nive” word doesn’t exist and “brake” is spelled “break” ;D ;D Sorry for late english lessons

So Qudus is right, just hiding the Transform3D class would break compatibility with existing games.
BUT kevglass pointed out that “Xith3D guys are always afraid to lose phantom users by breaking backward compatibility… It’s not even 1.0, so API Changes are to be expected”… And I agree with him…

However, it happens that phantom exists, phantom users at least… So it’s not a good idea…

I already did something about improving TransformGroup/Transform3D, it’s called… Transform ! It’s the org.xith3d.scenegraph.Transform class, which you can find browsing online the toolkit CVS…

BUT it’s not sufficient at all and it was in the beginning just intended to permit to transform objects in the constructor (hence the Transform-kind enum)…
What would be fine (and I’m all ready to do that) is (just picking ideas everywhere) :

  • To modify the existing org.xith3d.scenegraph.Transform class to suit our, i-want-useability users ;D, needs
  • To decide that translations are defined by Point3f
  • To decide that rotations are defined wether by Vector3f, wether by Quat4f, wether by AxisAngle4f
  • To decide that scale is defined by a Tuple3f
    Which would permit us to provides different constructors :
new Transform(Point3f translation);
new Transform(Quat4f/Vector3f/AxisAngle4f rotation);
new Transform(Tuple3f scale);

(maybe there’s a conflict between translation and scale, in which case we should provide constructors with - 3 floats for x-scale, y-scale, z-scale and - 1 global float for scale)

  • To add setTranslation/Rotation/Scale methods
  • To add addTranslation/RotationScale methods
  • To add addRotationX, addRotationY, addRotationZ methods

setRotationPart() method seems just impossible to do to me… in any case you would end up keeping 3 objects (one for translation, one for rotation, one for scale) and doing the - matrix initialization with translation - multiply with a rotation matrix - multiply with a scale matrix and even you don’t know if its the right order…
With the solution I suggest you could do :

transform.setTranslation(new Point3f(2f, 5f, 0f)).addRotationX(90f).addRotationY(45f).addScale(1.5f);

And, last idea but not least :

  • Provide method like transform(Point3f p) - returns Point3f which transforms a point by the current transformation…

That would make the Transform class useful not only for the scenegraph but for the whole game… I remember when I was younger I spent time to learn how rotations in 2D-space worked… I finally found out the sin(), cos() functions were actually useful… If Transform would have permitted me to do :


Point3f myHeroPosition;
// I handle the keyboard input and find out there's a "myHeroDisplacement" displacement to apply
// and a "myHeroAngleDelta" angle to change by
(Transform trans = new Transform()).setTranslation(myHeroDisplacement).addRotation(0f, 0f, myHeroAngleDelta);
trans.transform(myHeroPosition);

Note 1 : of course we should also provide Transform.transform(Point3f origin, Point3f result) vecmath-like methods
Note 2 : we could have instead of the setTranslation/Rotation… and addTranslation… methods have a identity() one and just translate(), rotate() and scale() ones. That would be just as well
Note 3 : cascaded calls imply that setTranslation()/translate() methods return the org.xith3d.scenegraph.Transform object.

[quote="<MagicSpark.org [ BlueSky ]>,post:15,topic:27554"]
@Qudus : the “nive” word doesn’t exist and “brake” is spelled “break” ;D ;D Sorry for late english lessons
[/quote]
“nive” is of course “nice”. “break”? ok. It was late.

by the way: “wether” is spelled “whether”. But I don’t think we both want to get on that level of communication, do we? :wink:

I think translation is done by a vector, but not by a point. So we should use a Vector3f instance for that, as it is now.

this Transform is a good idea!

first of all I think, it’s not so important what constructors we will supply, because a Quad4f can always be transformed into a Matrix3f…no for performance it’s more important what kind of structure we want to keep internally (for rotation, translation, scale). Options would be:

  • Matrix4f (like in Transform3D)

  • Vector3f (translation)

  • Matrix3f

  • Quad4f - Scaling would have to be done by another Vector3f (for non uniform scales - e.g. scaling only along the X-Axis)

a Vector3f for rotation is next to useless, because one can’t keep all rotation-information in it. As a constructor argument this would be nice.

The problem by using matrices is, that getting the scale is not an easy thing (because it doesn’t need to be saved outside the matrix), but on the other hand multiplying two Matrices (e.g. when applying a Transform on top of another) is very easy instead of doing that whith everything saved seperately.

And one thing, which I think is very important. With our datatypes we should as often as possible try to accept Tuple3f, because sometimes, the methods of Vector3f are also needed for a Point3f, so this would mean only stupid object creation.
So because of this problem it really might be wise to have something like Transform3D, but not as a class (which is actually only imitating Matrix4f) but as an Interface, so the most speedy version can be used for each different situation.

Yes, this is really cool. But for convenience, there should be some …Impl classes for the most common implementations either in the tk or in the core.

“nive” is of course “nice”. “break”? ok. It was late.

by the way: “wether” is spelled “whether”. But I don’t think we both want to get on that level of communication, do we? :wink:

I think translation is done by a vector, but not by a point. So we should use a Vector3f instance for that, as it is now.
[/quote]
Hah you’re right ;D ;D