makehumancommunity / makehuman

This is the main repository for the MakeHuman application as such.
http://www.makehumancommunity.org
Other
1.18k stars 244 forks source link

Fixing issues with pickMesh, queryDepth, and Frame._asWindowStateSet. #235

Closed Avereniect closed 8 months ago

Avereniect commented 9 months ago

I encountered three issues while running the program on Ubuntu 22.04.3 and have resolved them on my end. Testing in other environments may still be necessary.

A syntax error in Frame._asWindowStateSet's implementation was reported when attempting to run the program. This has been resolved by changing the way that the stateFlags parameter was tested.

Dragging background images did not work. This was tracked down to the contents of the picking buffer not being changed by the call to glDrawElements in pickMesh. After ensuring that GL_VERTEX_ARRAY was enabled, the draw call changed the buffer as expected. This addresses issue 231.

After fixing that issue, queryDepth was being called where it wasn't before, and the call to glReadPixels was throwing an OpenGL error. It seems that the active framebuffer did not have a depth buffer at the time of the call as glGetIntegerv(GL_DEPTH_BUFFER_BITS) reported zero despite reporting 24 in other places in the program. It was determined that this was because the appropriate context was not active at the time of the function's evaluation. Calls to makeCurrent in functions related to handling mouse events appears to resolve this issue.

Also addressed issue 232 by adding cast to int.

joepal1976 commented 9 months ago

Thanks, seems sensible enough.

Out of curiosity, what graphics card have you tested this on?

Avereniect commented 9 months ago

I tested it on an Nvidia MX250 running Nvidia's proprietary drivers, version 535.54.03-0ubuntu1.

Edit: Actually I just realized something. The reason that GL_VERTEX_ARRAY was not already enabled when it should have been is the issue with the incorrect context being current. With that issue fixed, the call to glEnableClientState(GL_VERTEX_ARRAY) I added is actually extraneous.

joepal1976 commented 8 months ago

Thanks. I'm on this, and I think it should be merged. But I want to give it a test spin on an integrated intel gpu first.

joepal1976 commented 8 months ago

With the testing I've done, I haven't seen any negative side-effects.