search

glob3mobile / g3m

The multiplatform advanced visualization framework
http://www.glob3mobile.com/
Other
116 stars 56 forks source link

Stereo rendering camera bug #181

Open Matyooo opened 8 years ago

Matyooo commented 8 years ago

Hi @DiegoGomezDeck and all!

We are experimenting the new stereo vision features from the #180 PR. There is an interesting bug/theoretical mistake in Camera.java that prevents the 3D rendering from working properly in Java/Android! The left and the right eye pictures are totally identical in our project.

The center of the problem is the field _timestamp. This field is incremented when the Camera object is modified. It is NOT in any manner a real timestamp. This field is checked in copyFrom() method and the copy is aborted if two 'timestamps' are equal. IMHO this is wrong. Even if two cameras have equal 'timestamp' fields that does not indicate that their other fields are equal. This prevents the proper initialisation of the leftEyeCam and rightEyeCam in G3mWidget.rawRenderStereoParallelAxis() if auxCam's timestamp equals their timestamps. That is the case in our app.

FIX: Removing the equals check from the beginning of Camera.copyFrom() solves the problem. The left and right images differ and the results are good stereo 3D.

Can you look into this? Thanks, Mat

DiegoGomezDeck commented 8 years ago

Hi @Matyooo

Thank you for finding!

Camera::_timestamp is something like a "modification counter". Taking a quick look at the code, what I think is happening is that Camera::setFOV() and Camera::setLookAtParams() are not updating the timestamp.

You there @amazingsmash ?

Matyooo commented 8 years ago

Hi @DiegoGomezDeck , @amazingsmash ,

Camera::setFOV() does update timestamp. Camera::setLookAtParams() does updates timestamp THREE times. But this is not the source of the problem. The main problem is caused by two things:

  1. Camera::copyFrom(parent) should use copy constructors for all fields that are not primitive types (eg: Frustum, FrustumData, Geodetic3D). Otherwise the new camera instance will have the references to the same fields of his parent. You can only get away with this error if an update forces the camera to recreate these fields.
  2. Camera::copyFrom() must not depend on timestamp as timestamp has nothing to do with two camera objects being identitcal or not, as a matter of fact I would suggest removing timestamp completely. I may be wrong on this but I do not see why a modification counter is needed for anything... Maybe an equals() and hashCode() woudl be more useful for the same purposes.

Guys?

DiegoGomezDeck commented 8 years ago

Hi @Matyooo,

Can you check, please, if the current version at purgatory branch works for you?

Matyooo commented 8 years ago

I am integrating it now!

I will get back to you as soon as possible.

Mat

2016-04-11 16:08 GMT+02:00 Diego Gomez Deck notifications@github.com:

Hi @Matyooo https://github.com/Matyooo,

Can you check, please, if the current version at purgatory branch works for you?

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/glob3mobile/g3m/issues/181#issuecomment-208364907