Problems with gluUnProject

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

Problems with gluUnProject

lukej
I recently upgraded from JOGL 1.1 to JOGL 2.0 and I noticed some strange things were happening when I translated screen coordinates to world coordinates.  I have tracked it down to what appears to be a bug in the JOGL 2.0 implementation of GLU.gluUnProject.  

To make sure it wasn't something in my scene I took the source code from here and converted it to Java code.  When I replaced the GLU.gluUnProject call with that one, things work as expected.

Next I created some unit tests that test the resluts of the glhUnProjectf function vs. GLU.gluUnProject.  I found that in cases where the modelview matrix is the identity matrix, the answers match.  However, if the modelview matrix is not the identity matrix, the answers do not match (I did not rigorously test if this was always true, but it seemed to hold true).  It is worth noting that the GLU.gluUnProject call DOES return true, but the answer is incorrect.

Here are 2 such examples (the first uses the identity for a modelview matrix, the second does not):

Modelview Matrix: [1.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 1.0]
Projection Matrix: [2.3464675, 0.0, 0.0, 0.0, 0.0, 2.4142134, 0.0, 0.0, 0.0, 0.0, -1.0002, -1.0, 0.0, 0.0, -20.002, 0.0]
Viewport: [0, 0, 1000, 1000]
Screen Coordinates: [250.0, 250.0, 0.5]
JOGL GLU Result: [-4.261299, -4.141722, -19.998001]
Official OGL Code Result: [-4.261299, -4.141722, -19.998001]

Modelview Matrix: [1.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, -200.0, 1.0]
Projection Matrix: [2.3464675, 0.0, 0.0, 0.0, 0.0, 2.4142134, 0.0, 0.0, 0.0, 0.0, -1.0002, -1.0, 0.0, 0.0, -20.002, 0.0]
Viewport: [0, 0, 1000, 1000]
Screen Coordinates: [250.0, 250.0, 0.5]
JOGL GLU Result: [0.02141787, 0.020816857, 0.10051267] *** WRONG
Official OGL Code Result: [-4.2612987, -4.1417212, 180.002]
Reply | Threaded
Open this post in threaded view
|

Re: Problems with gluUnProject

lukej
This post was updated on .
I've done a little more research on the issue and looked at the JOGL 2.0 source code a little bit.  If I'm following this correctly, it looks like:

GLU.gluUnProject calls ProjectFloat.gluUnProject
ProjectFloat.gluUnProject calls FloatUtil.multMatrixf passing in the parameters (modelMatrix, projMatrix, matrixBuf)

According to the javadoc for multMatrixf, this should put a*b in d.  However, I believe it actually puts the matrix b*a in d.

I found this because I made a similar mistake looking at the OGL example code that I linked to in my original post.  That code has a function MultiplyMatrices4by4OpenGL_FLOAT that takes result, matrix1, matrix2.  However, what is actually put into result is matrix2 * matrix 1.  That code works fine because it passes in projection as matrix1 and modelview as matrix2.  (note that the desired result is modelview * projection).

I'm not sure what other code (if any) you have calling multMatrixf, but I think you should consider changing the order of the parameters in ProjectFloat.gluUnProject, or fixing FloatUtil.multMatrixf so that it puts a*b in d, as advertised.  If multMatrixf is left as is, the javadoc should be corrected to reflect that d is the result of b*a.

EDIT: Some of the a*b vs. b*a issue may have to do with my own confusion of OpenGL's matrix ordering.  Regardless, the multiplication for the gluUnProject seems to be applied in the incorrect order.

Hope this helps,
Luke
Reply | Threaded
Open this post in threaded view
|

Re: Problems with gluUnProject

Wade Walker
Administrator
Are you sure about this? I looked at the source code for FloatUtil.multMatrixf(), and it seems correct. It takes matrices a and b, which are in column-major order, and multiplies them together, one row at a time from a, against each of the four columns of b. The main loop forms one row at a time of the result matrix d.

Since post-multiplying by a column-major matrix is the same as pre-multiplying by a row-major matrix, perhaps you're just passing your matrices into GLU.gluUnProject() in row-major instead of column-major order?
Reply | Threaded
Open this post in threaded view
|

Re: Problems with gluUnProject

lukej
That is correct, I corrected my statement about a*b vs. b*a in my edit of my second post.  I was getting mixed up with the multiplication order for the OpenGL array order.  FloatUtil.multMatrixf() DOES appear to multiply in the correct order for OpenGL matrices.

However, I am fairly positive that the gluUnProject call is still calling FloatUtil.multMatrixf() with the wrong order.  The results of the recommended code from OpenGL (linked above, I ported to Java) match the results that I was getting from JOGL 1.1.  They do not match JOGL 2.0 (latest build).  I gave examples above of what the float arrays looked like.  Perhaps you could tell me if they are in the correct order or not.  They look accurate to me.  

I am not flipping the matrix around at all.  I am just calling OpenGL to get the modelview matrix, the projection matrix, and the viewport and then passing them into gluUnProject.

For further proof that there seems to be an issue, calling gluUnProject and then passing the results into gluProject does not get me back to where I started (unless one of the matrices is the identity matrix).

If you look at the example code  I linked above here is the line that does the multiplication:
MultiplyMatrices4by4OpenGL_FLOAT(A, projection, modelview);

Note that the GLU code in JOGL 2.0 does the opposite. (modelMatrix, projMatrix, matrixBuf)
Reply | Threaded
Open this post in threaded view
|

Re: Problems with gluUnProject

Wade Walker
Administrator
Hmm, I looked at the JOGL 1 source vs. the JOGL 2 source, and the old Project.__gluMultMatricesd() is different from the new FloatUtil.multMatrixf(). The old one is a nested loop, where the new one is unrolled, and I can't be certain just by visual inspection that they're equivalent. Let me do a test program tomorrow to compare their outputs to see for sure -- gotta go to bed right now unfortunately :)
Reply | Threaded
Open this post in threaded view
|

Re: Problems with gluUnProject

Wade Walker
Administrator
I looked at this some more, and found that Project.__gluMultMatricesd() multiplies the matrices "backwards" from FloatUtil.multMatrixf().

It looks like ProjectFloat.java changed from using gluMultMatricesf() to FloatUtil.multMatrixf() a few weeks ago. Sven may not have realized that gluMultMatricesf() was a column-major multiply instead of a row-major multiply, so he might have switched out these methods thinking they did the same thing.

Are you using the latest autobuild of JOGL, or a recent release candidate? If so, try using an older version (one at least three months old) and see if this bad behavior goes away. If so, that will confirm that this is a recently introduced bug.

I'll try writing a unit test for gluProject and gluUnproject to nail this behavior down and make sure these functions do exactly what they're supposed to. There doesn't appear to be any test coverage for these right now, so having a few tests would definitely help.
Reply | Threaded
Open this post in threaded view
|

Re: Problems with gluUnProject

lukej
Sounds good - I will check this out on Monday when I'm back in the office.  I'm using the 2.0-rc7 build that was on the main page of the site a few weeks ago.  I'll check out some of the 2.0 rc's and try to see which ones have the issue and which ones don't.
Reply | Threaded
Open this post in threaded view
|

Re: Problems with gluUnProject

Sven Gothel
Administrator
In reply to this post by Wade Walker
On 05/06/2012 01:44 AM, Wade Walker [via jogamp] wrote:
> I looked at this some more, and found that Project.__gluMultMatricesd()
> multiplies the matrices "backwards" from FloatUtil.multMatrixf().
>
> It looks like ProjectFloat.java changed from using gluMultMatricesf() to
> FloatUtil.multMatrixf() a few weeks ago. Sven may not have realized that
> gluMultMatricesf() was a column-major multiply instead of a row-major
> multiply, so he might have switched out these methods thinking they did the
> same thing.

Oops, may bad. So we should swap the multiplication arguments here right ?

>
> Are you using the latest autobuild of JOGL, or a recent release candidate? If
> so, try using an older version (one at least three months old) and see if this
> bad behavior goes away. If so, that will confirm that this is a recently
> introduced bug.
>
> I'll try writing a unit test for gluProject and gluUnproject to nail this
> behavior down and make sure these functions do exactly what they're supposed
> to. There doesn't appear to be any test coverage for these right now, so
> having a few tests would definitely help.
Very good, thank you Wade.

BTW: Jenkins will be back up at end of May .. until then,
     we have to use the source and our local tests.

~Sven


signature.asc (910 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Problems with gluUnProject

lukej
RC5 worked fine, RC6 and RC7 both have the issue I describe.
Reply | Threaded
Open this post in threaded view
|

Re: Problems with gluUnProject

Wade Walker
Administrator
I've entered this as an official bug at https://jogamp.org/bugzilla/show_bug.cgi?id=581. If you add yourself to the CC list for the bug, you can see when it's resolved (and help verify the solution).

Thanks for reporting this!
Reply | Threaded
Open this post in threaded view
|

Re: Problems with gluUnProject

lukej
No problem, thanks for working with me to make sure I wasn't going crazy!  
Reply | Threaded
Open this post in threaded view
|

Re: Problems with gluUnProject

Sven Gothel
Administrator
Fixed, see below bug 581 report ..

Please recompile jogl and test ... and report
whether the fix satisfies your expectation (it should).

Maybe you can provide a simple gluUnproject unit test. Thx.

+++

<https://jogamp.org/bugzilla/show_bug.cgi?id=581>:

<http://jogamp.org/git/?p=jogl.git;a=commit;h=cbc77718f01a8190e1a8aa0e9afdc2a3a3403358>

Fix regression of commit
<http://jogamp.org/git/?p=jogl.git;a=commit;h=de2b129a56335262a44a05541a3ab2e35668cc6e>:
ProjectFloat Matrix Multiplication of gluUnProject(..) impl.

ProjectFloat's previous gluMultMatricesf(..) used row-major order,
but the replacement multMatrixf(..) uses column-major order (like OpenGL, ..).

Note: The replaced 'gluMultMatrixVecf' by multMatrixVecf() already
           used column-major order.

Fix: Reverse the arguments of matrix multiplication
    m1 x m2 -> m2 x m1
Reply | Threaded
Open this post in threaded view
|

Re: Problems with gluUnProject

Sven Gothel
Administrator
<http://jogamp.org/git/?p=jogl.git;a=commit;h=7f85501bd448afc9ba52f6abbe5f049d673d824d>

Added further unit test w/ actual gluUnProject(..) usage.

Remark: FloatUtil's multMatrixf(..) does column-major multiplication,
i.e. expect the linear matrix layout to be in column order.

In case the implementation does not follow the above statement (you find a bug),
pls re-open this bug report.