Experimental depth sorting selection picking
ClosedPublic

Authored by Campbell Barton (campbellbarton) on Fri, Mar 3, 12:53 PM.

Details

Summary

This patch replaces occlusion queries when picking (but keeps existing occlusion queries for border select).

Suggested by Ton, checks the depth buffer each draw and keeps 2 buffers, one for depth and another for id's so we can keep track of which ID draws into each pixel, then calculate a depth-sorted hit-list.

This isn't going to be efficient for large buffers but luckily for picking we don't need to read from a large buffer.

To test this patch enable "Occlusion Queries" in the preferences, border select behavior uses still uses "Occlusion Queries", picking (GPU_SELECT_DEPTH_SORT_NEAREST & GPU_SELECT_DEPTH_SORT_ALL) uses only the depth buffer.


Notes:

  • Selection performance on my system (NVIDIA) doesn't seem very different (a little slower than occlusion queries).
  • Bones with X-Ray don't work properly, see: T46043 (can look into a fix if this is otherwise acceptable to merge).
  • Selection passes GPU_SELECT_NEAREST_FIRST_PASS, GPU_SELECT_NEAREST_SECOND_PASS are now only used for manipulator picking.

Open Topics:

  • How much do we want to emulate GL_SELECT?, current occlusion queries for example only fill in the ID field of the selection buffer: the first 3 [number, znear, zfar, ] fields are dummy values. If we're not using GL_SELECT we could drop attempting to follow it's conventions.
  • Is this method going to be used long term if we can use shaders as @Mike Erwin (merwin) suggests.

Diff Detail

Repository
rB Blender
Branch
temp-select-pick
Build Status
Buildable 461
Build 461: arc lint + arc unit
Campbell Barton (campbellbarton) retitled this revision from to Experimental depth sorting selection.Fri, Mar 3, 12:53 PM
Campbell Barton (campbellbarton) retitled this revision from Experimental depth sorting selection to Experimental depth sorting selection picking.Fri, Mar 3, 1:04 PM

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.

source/blender/gpu/intern/gpu_select.c
91

glPush/PopAttrib are not part of core profile

@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).

More efficient depth sorting

  • No need to sort by id and depth (just ID is fine).
  • De-duplicate contiguous ID's before sorting.

@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.

Updated the patch, this makes changes which are more likely to conflict but was too hard to avoid.

  • Remove 2-pass occlusion query code.
  • Still keeps old GL_SELECT, we can remove but keep for testing for now.
  • Change behavior of edit-mode bone selection to match object mode. This was needed because the current code expected the hit-list to be draw-order. Now the first picking will always pick the front-most bone, then cycle bones after that. (which IMHO is an improvement, making it more consistent)

Expose:

  • VIEW3D_SELECT_DEPTH_SORT_ALL (uses depth buffer, sorted by the closest depth of each ID)
  • VIEW3D_SELECT_DEPTH_SORT_NEAREST (uses depth-buffer, occludes ID's behind other objects)
  • VIEW3D_SELECT_ALL (uses occlusion queries, for border select for eg since they're unordered)

Even though hit-lists are now depth sorted, only pose-bones respect the selection codes hit-list order. Everything else cycles by draw order. After this patch is applied, support can be added but I'd rather do it separately since this is already changing quite a few areas, so I rather get verified not to be breaking existing selection code and accepted first.

  • Added back multi-pass occlusion queries, needed for manipulator
  • Merge branch 'master' into temp-select-pick
  • Merge branch 'master' into temp-select-pick

For the records: assorted sample files I'm using for testing:


To see how to test them, refer to: https://www.youtube.com/watch?v=AA-Z_gB4OPk

This revision is now accepted and ready to land.Mon, Mar 6, 9:42 AM

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.

  • Testing functions: for mixed_bones_object_selectbuffer
  • Split select code
  • Merge branch 'master' into temp-select-pick
  • Cache depth buffer for reuse
  • Use cache
  • Cleanup, minor fixes
  • Various fixes
  • Rename select_method -> algorithm
  • Merge branch 'master' into temp-select-pick
  • Fix incorrect extern, also remove redundant rect
  • Fix mistake in glReadPixels
  • Avoid initial buffer read, fill with 1.0
  • minor tweaks
  • Use unsigned int for depth values
  • Modified method of cycling selection
  • Various minor changes, add back old multi-pass select
  • Add preference