Hi @Campbell Barton (campbellbarton) I'm glad you are working on this :)

Could you please explain me how to test this patch (--factory-startup? Automatic Selection mode? OpenGL Select?...)

I tried here and I couldn't see any difference when using Occlusion Query (all the bugs are still there - I can provide files if you want). While if I use OpenGL Select we are still relying on glRenderMode, which we want to get rid of.

If this patch makes Occlusion Query the one selection mode we will support, shouldn't it also get rid of the other one (and the user preference option)?

I'm also glad!

In the last viewport IRC meeting we brought up selection/picking as needing an overhaul, but have not gotten into the details yet.

We have a depth-only shader (no color) and could easily make a depth+ID shader. ID could mean whatever we want on CPU side — object ID, vertex #, face #, etc.

@Dalai Felinto (dfelinto), to test this patch use "Occlusion Queries",
From my testing this works as correctly and solves the selection order bug (where 2+ objects were detected, "Occlusion Queries" would select based on order drawn instead of depth). Otherwise if you're running into problems, do you have a link to a spesific bug that fails with this new code?

There is one bug already mentioned (T46043), but I think this is relating to x-ray, will have to investigate whats going on but didn't want to sink time into this if the basic method has more general problems.

As to your comment about being able to remove the select option if this is accepted - yes. It just makes the patch more noisy and likely to conflict, so left the change fairly small for now.

@Mike Erwin (merwin) - using color shader would be good longer term. It's not trivial to do this with 2.78x code, but if we could move to it later it would be handy.
Note that there are some problems with this - On NVidia at least, disabling multi-sampling is not reliable and will still over-sample (breaking selection) (its a long standing bug we had since it brakes GL_SELECT too). So we may need to use color+offscreen buffer for reliable non-multi sampling selection.

Using glPush/PopAttrib isn't especially important and can be fairly easily removed (was just following existing code for occlusion queries).

@Campbell Barton (campbellbarton) I left my sample files at the office, so I won't be able to share them before Monday :/ But you can see them in action in the video: https://www.youtube.com/watch?v=AA-Z_gB4OPk

Basically with your patch I still don't get the "cycling" option - where you select a bone (or object? I tried only with bones) and next time you click it goes to the one behind it, then the one behind this, and on and on.

I don't remember the other cases (pose bone and object), but I will re-test them on Monday.

Confirmed, this patch was only being used for nearest, for cycling it was using occlusion code. Note that I could only redo this bug with edit-bones. Pose-bones and objects worked as expected which is why I wasn't seeing the bug.

Checking 2.79 release with GL_SELECT, I get random order when cycling selection (not ordered based on depth), so we probably don't want to match GL_SELECT behavior exactly.

Will write up a patch which gives good depth sorting for both nearest and when cycling.

Note, this change made selection really slow(~2x) on bone pose mode in production files (see selection.zip).

Computer specs:

renderer:       'Gallium 0.4 on AMD TONGA (DRM 3.8.0 / 4.9.0-1-amd64, LLVM 3.9.1)'                   
vendor:         'X.Org'                                                                    
version:        '3.0 Mesa 13.0.4'

Probably best to leave this patch then. Reading pixels is known to be slow on some hardware (we ran into this with border select and open-source Linux drivers in bug reports already).

Some notes:

• Regarding this patch being 2x slower mixed_bones_object_selectbuffer can call view3d_opengl_select up to 3x times. There is really no need for this since the pixels created by the 28px radius could be re-used for the 18 and 0 radius, without any redrawing. We could also return hit data with distance from the center, to remove the need for running this multiple times.
• Using glReadPixels with pixel-buffer-objects might solve this problem, they're faster then regular glReadPixels in some cases apparently.

As discussed on IRC, this is a suggestion for how to handle the User Preferences in master, and later in 2.8:

master
------
User Preferences Option:

Enum:
  * OpenGL Select
  * Occlusion Query
  * Automatic


master-before-removing-gl_select
--------------------------------
User Preferences Option:

Enum:
  * OpenGL Select
  * Occlusion Query
  * Automatic

Bool:
  OpenGL Depth Buffer Picking" [X]


blender2.79
--------------
Ideally the same as blender2.8, but it will depend on user feedback.

blender2.8
-------------
User Preferences Option:

Bool:
  OpenGL Depth Buffer Picking" [X]

The tooltip for the "OpenGL Depth Buffer Picking" could be along those lines: "disable if you want more performance in ATI + Linux".

It would be interesting to also change Occlusion Query to be the Automatic method in master, with a doversion even. This way we can make sure people are actually testing the patch as it is, before we drop OpenGL Select entirely.