Fix T76541: OpenGl Depth Picking not selecting frontmost object
ClosedPublic
Actions

Authored by Sebastian Parborg (zeddb) on May 12 2020, 7:42 PM.

Details

Reviewers

Clément Foucault (fclem)
Jeroen Bakker (jbakker)
Campbell Barton (campbellbarton)

Maniphest Tasks

T76541: OpenGl Depth Picking not selecting the front-most object

Commits

rB82bda82ec449: Fix T76541: OpenGl Depth Picking not selecting frontmost object
rB36d9f7e37540: Fix T76541: OpenGl Depth Picking not selecting frontmost object

Summary

The issue was that we used GL_ALWAYS for depth checking here which would lead to the depth information from objects being messed up.
It would not represent which object was closest to the camera.

Now this is a very small and simple fix, I have looked at the code and I think it should be fine but I want it to be reviewed so that I'm not making a terrible mistake.
One of the biggest red flags is that I can't really figure out why we would ever want to have GL_ALWAYS for a depth test and this leads me to believe that I'm missing something here.

Diff Detail

Event Timeline

Sebastian Parborg (zeddb) requested review of this revision.May 12 2020, 7:42 PM

Sebastian Parborg (zeddb) created this revision.

Sebastian Parborg (zeddb) mentioned this in T76541: OpenGl Depth Picking not selecting the front-most object.

Ah now I see the code in perspective.

GPU_SELECT_PICK_ALL might be used for selection cycling or select picker under cursor. User can call that operator and sees all the object that are under the cursor. The user can then pick the object from the menu. It might be (i didn't test) that this change will not hide several options in that menu.

Right, the issue happen exactly when GPU_SELECT_PICK_ALL is used.
The first selection is not the object that is visble to the camera in certain cases.

With this change the cycling still works and the front most object is correctly selected it seems.

I'll see if the objects in the list menu changes tomorrow.

Sebastian Parborg (zeddb) retitled this revision from Fix T76541: OpenGl Depth Picking not selecting frontmost object in orthographic view to Fix T76541: OpenGl Depth Picking not selecting frontmost object.May 12 2020, 10:06 PM

@Jeroen Bakker (jbakker) the selection menu item order and number of items seems unchanged after my changes. So there doesn't seem to be any regressions there.

@Campbell Barton (campbellbarton) was the one to design this. Maybe he has some insight. To me this seems fine.

I don't know this code much, but I suspect that the use of GL_ALWAYS instead of GL_LEQUAL is for efficiency since at first it seems that we don't need to care about the order of objects in GPU_SELECT_PICK_ALL.

Ankit Meel (ankitm) added a task: T76541: OpenGl Depth Picking not selecting the front-most object.May 13 2020, 5:33 PM

Campbell Barton (campbellbarton) accepted this revision.May 14 2020, 1:31 PM

This revision is now accepted and ready to land.May 14 2020, 1:31 PM

Closed by commit rB36d9f7e37540: Fix T76541: OpenGl Depth Picking not selecting frontmost object (authored by Sebastian Parborg (zeddb)). · Explain WhyMay 14 2020, 1:47 PM

This revision was automatically updated to reflect the committed changes.

Sebastian Parborg (zeddb) added a commit: rB36d9f7e37540: Fix T76541: OpenGl Depth Picking not selecting frontmost object.

Sebastian Parborg (zeddb) added a commit: rB82bda82ec449: Fix T76541: OpenGl Depth Picking not selecting frontmost object.May 19 2020, 12:24 PM