Camera.lookAt + Vector3f.UNIT_Y

My camera is fixed at specified positions in the world and it tracks player Node as it moves, similar to Fahrenheit (Indigo child)

I use camera's lookAt() method, which takes worldUp vector as position to target camera at the node.



After I've noticed that Vector3f has static UNIT_Y (0,1,0), I've deleted all my 0,1,0 constants and started using it, but ever since my camera's started to pan strangely. Even if Vector3f's static field is final, it doesn't prevent someone from changing its internals, so I've logged its values after camera's targeting, and it showed that UNIT_Y changes values!

So I reverted back to my constant which makes camera tracking the way I want it…



…but is it supposed to be so, that UNIT_Y (up) vector is changing as camera moves? I though UP should be 'up', no matter the camera position.

As I dunno, can someone explain it to me please? Without shaming me too much :slight_smile:

i have no jme code here at work but i suppose the provided "up" vector is used to init the camera position and as a result it's modified if the "camera up" vector changes.

it's a good practice not to pass the Vector3f/2f constants to functions which could modify them. same for ColorRGBA constants.

rather pass a new Vector3f(Vector3f.UNIT_Y) or a new ColorRGBA(ColorRGBA.orange) to such functions.

I had a similar issue, I was passing Vector3f.Unit_z to a method that did some maths and passed it on down to a transform. I then updated siad transform elsewhere in my code and unwittingly changed Vector3f.UNIT_Z, which screwed lots of things up :slight_smile:



I now have lots of tempVect* type attributes and call .set(origVector) on them to protect the contents.



Endolf

No, the problem arises when I invoke lookAt camera method which also takes UP vector.

Funny though, when I pass my static final UP vector to camera frustum setup and same static to lookAt method, it works as expected. And its not that my 0,1,0 is 'more real' than Vector3f.UNIT_Y :smiley:



I guess JME internals change Vector3f.UNIT_Y as someting changes in the scene.



So, hardcopies you say, OK, fine by me…

deus_ex_machina said:

I guess JME internals change Vector3f.UNIT_Y as someting changes in the scene.

So, hardcopies you say, OK, fine by me...



If I understand correctly what're saying (I think I do) this sounds really bad  :|

I know a lot of "non Java" coding is done in jME core to make it fast but damn!  This sounds like the sort of thing that would trip up any Java coder and should probably be sorted.

i don't think Vector3f.UNIT_Y is changed anywhere by jme.

that's just superstitions  :wink:

I've just checked the source, and AbstractCamera copies the vector you pass in before it does any work on it, therefore, it must be being changed elsewhere, not in camera.lookat.



Unless you are using a different renderer or camera :slight_smile:



Endolf

In that case, ignore me  :stuck_out_tongue:

i fetched the jme code for references to Vector3f.UNIT_Y and couldn't find any place where it's changed.

Perhaps any internal core methods that change the passed in vector could have:



if (vec == Vector3f.UNIT_Y || vec == Vector3f.UNIT_X || etc) {
    LoggingSystem.getLogger().warn("You passed in a constant vector defined in Vector3f, its about to be modified by this method!");
}



or something.  I have no idea how many places would need updating but if it's not many it sounds like a good idea to me.  I'd be happy to code this up and submit a patch if anyone thinks it's a good idea.

imho there's no point doing such additions. you would have to add that to almost every method which accepts a Vector3f as argument.

java programmers should be aware that if such objects are passed to a method, the method might alter them. maybe adding such remarks to the javadoc of classes which define such "constants" would help.

Gentleman Hal said:

Perhaps any internal core methods that change the passed in vector could have:


if (vec == Vector3f.UNIT_Y || vec == Vector3f.UNIT_X || etc) {
    LoggingSystem.getLogger().warn("You passed in a constant vector defined in Vector3f, its about to be modified by this method!");
}



or something.  I have no idea how many places would need updating but if it's not many it sounds like a good idea to me.  I'd be happy to code this up and submit a patch if anyone thinks it's a good idea.


This is a bad idea... the number of checks that would start happening as a scene rendered would slow things down too much.

More appropriate would be to make the x,y,and z variables non-public, then change these static variables to override the set methods to not allow the operation.  I'm all for hiding the member variables of the math classes.  The accessor methods would get inlined by the compiler anyway.

I agree, compiler should be smart enough to inline such small calls…

As for UNIT_Y, it is obvious that it wouldn't change inside Camera class. If it was, setting my own copy of constant wouldn't help, as it would change too, but instead it works. As I said, its just like my up vector is 'stronger' than UNIT_Y… funny … :smiley:



I can provide decorator I use to make camera track a node. Just prepare a scene with fixed camera, movable box or something and a thin, wide box as a floor… attach KeyNode???InputAction to move node decorated with provided decorator and see behaviour… you can uncomment syserr line to track UNIT_Y…



Edit:

I've found it…  I was passing UNIT_Y to camera setFrame 'up' parameter. From a glance on jme AbstractCamera, It seems to modify passed vector as camera rotates or moves, to maintain camera's 'up' relative to world 'up.

My mistake… sorry… wasn't expecting this mutating behaviour from a method. I expected it to copy my vector for its own purposes, just like in the lookAt…

sorry again.





import com.jme.input.action.InputAction;
import com.jme.input.action.InputActionEvent;
import com.jme.renderer.Camera;
import com.jme.scene.Spatial;

/**
 * Decorates an InputAction to perform camera.lookAt after
 * decorated performAction.
 * Usable when using InputActions that move node around.
 *
 * @author deus
 * @version 1.0
 * @since Apr 12, 2007 12:16:32 PM
 */
public class CameraTrackingDecorator extends InputAction {
    private Camera camera;
    private Spatial playerCharacter;
    private InputAction action;

    public CameraTrackingDecorator(InputAction action, Spatial playerCharacter, Camera camera) {
        this.camera = camera;
        this.playerCharacter = playerCharacter;
        this.action = action;
    }

    public void performAction(InputActionEvent evt) {
        action.performAction(evt);
        camera.lookAt(playerCharacter.getLocalTranslation(), RiotGame.UP); // RiotGame.UP is static final  0,1,0 vector... pass Vector3f.UNIT_Y here and see...
        // System.err.println(Vector3f.UNIT_Y);
    }
}

renanse said:

This is a bad idea... the number of checks that would start happening as a scene rendered would slow things down too much.

More appropriate would be to make the x,y,and z variables non-public, then change these static variables to override the set methods to not allow the operation.  I'm all for hiding the member variables of the math classes.  The accessor methods would get inlined by the compiler anyway.


i totally agree to this solution. the only thing to take into account when doing such a change would be that a lot of code will break.
btw, this problem was also d

Yep, I remember that.  Moving to private fields might be a good first step towards different math types as at least the items in the repo will all work correctly once we fix the direct refs.  At that point, immutable vs. mutable will be possible even if we decided not to do it as a separate class type in the core math package (don't see a reason not to at this point.)

phewww so it's not only me who got screwed by these "Vector3f.UNIT_…" ! Seeing weird camera flippings, I was like wth !



I'll reverse back all to "new Vector3f(…)" right now  :wink:

Why not make the UNIT vectors member variables final, possibly as special cases that could contain empty set methods (although maybe printing a warning to the logger).  Then no-one would ever have to suffer through this again; although I must say once you go through it there generally is a much more profound understanding of Vectors and jME. 



Before anyone shoots me, I KNOW special cases suck; but sometimes they are needed.  And in this case it would not really be a single special case, but rather several special cases; which could be lumped together.



If this is just dumb, feel free to say so.

Actually I had encountered the same problem before, so I'm very happy to stumble on the reason for this behavior by accident :slight_smile:

We've talked about making them "constant"… unfortunately, that will require us to also eliminate direct access to the member fields (x, y, and z.)  While that is not necessarily a bad thing, it is a significant refactor job and api change.

If you just add to the JavaDocs that the up vector will have its values magically changed througout the course of the game it will already help a great deal :slight_smile: