https://bugs.freedesktop.org/show_bug.cgi?id=34495
--- Comment #27 from Pierre-Eric Pelloux-Prayer pelloux@gmail.com 2011-07-01 09:00:56 PDT --- (In reply to comment #26)
@Pierre: After looking at your patch for a while I have the following doubts/suggestions:
- on _mesa_exec_select_batch() why would you loop through buffer[..] for each
batch entry? at first sight it seems to me that if there is more than one entry, the minZ and maxZ values written by write_hit_record would be the same for every entry.
This is what I understood from GL_SELECT behavior : "Both the minimum and maximum window-coordinate z values of all vertices of the primitives that intersected the viewing volume" So by looping over buffer[] I'm trying to find the min/max z for the entry (at least that was the point, but maybe it's incorrectly done :-))
- What happens if the user changes the fbo or any state needed (depth and
scissor) after calling "glRenderMode(GL_SELECT)"?
What happens if user changes something : it will not work reliably... Thanks to remind me of this problem which I almost forgot. Now the necessary state change should be limited : - GL_DEPTH_TEST : I don't understand why it doesn't work with it enabled, but it definitely not be changed - GL_SCISSOR : this one is more annoying, as it's wrong to disable it too (maybe the user really wants to GL_SELECT on a sub area of the window). But as glScissor take window coordinates values, those should be changed before drawing (scissor_width would become : scissor_width * fbo_width / window_width) - FBO is needed
Maybe we should create our st_select_draw_vbo that makes sure to have the correct state->render using st_draw_vbo->restore state. It is possible that this could be a little heavy on state changes but it feels safer... Maybe we could implement some kind of "state-override" later to make sure we only restore states after exiting GL_SELECT mode and still be sure that the end user isn't interfering with our GL_SELECT implementation.
The 1st version of the patch had a custom drawing function ('select_draw_func'). Initially I was doing some state changes over there, but it caused problem in 'vbo_exec_FlushVertices' with this assert : assert(exec->flush_call_depth == 1); But sure, it would be safer to have some state check/change over there.