graphics::qt3d::view::getPoint() fails to read depth buffer via glReadPixels()

[andrew-hill]

I'm only seeing this on Mac OS X, so it's likely something specific to OS X or Apple's OpenGL implementation, which means it's a pretty low priority issue, but maybe something that's previously been solved elsewhere...

High level behaviour:

• cannot re-centre the rotation by double-clicking in either view-points or label-points
• cannot use any tools in label-points (select/filter/sample/fill IDs/etc all rely on hitting a specific point)

Debugging: The bug seems to stem from snark/graphics/qt3d/view.cpp, specifically the function view::getPoint( const QPoint& point2d ) (line ~300) and it's call to glReadPixels() to read the depth buffer.

I'm finding that glReadPixels() never modifies depth, but also never generates an error code via glGetError(). Have also tested getting values from GL_GREEN instead of GL_DEPTH_COMPONENT, and using a larger sampling window, neither of which results in any data.

Mac OS X 10.9 Qt4.8 and Qt5.3 Retina MacBook Pro (macbookpro11,3), tested both Intel Iris Pro & nVidia GT 750M

[vlaskine]

unfortunately, in the foreseeable future, we may never have workforce to deal specifically with mac issues

the most we can do: if you guys (mac users) fix them on your machine, you could push them

it is an imperative to do a code review with me before you push: it happened many times that the change would subtly break something else, as well as simply not be the right fix

[andrew-hill]

i think i have the answer — it looks like Mesa's OpenGL implementation (from MacPorts) is the source of the bug. The OpenGL framework that ships with OS X works fine, but CMake wasn't finding it.

The fix in graphics/CMakeLists.txt is to look for the OpenGL package the same way on APPLE as it does for WIN32:

FIND_PACKAGE( OpenGL )

rather than using the linux (or currently anything-non-WIN32) method:

FIND_LIBRARY( OPENGL_LIBRARY NAMES GL REQUIRED )

Also, should OpenGL be REQUIRED under windows as it is in Linux?

[vlaskine]

i may not have time to try it on linux: it works now and i would not touch it

would you add it in if-clause for mac only for now?

[andrew-hill]

done

[vlaskine]

great, thank you! please push it