Page MenuHome

Selection engine.
ClosedPublic

Authored by Germano Cavalcante (mano-wii) on Jun 19 2019, 12:42 AM.

Details

Summary

This patch proposes to move the entire API of selecting faces, vertices and edges to a DRW manager engine.
In DRW manager, we can use the existing feature of creating a DrawData per object.
With this feature we can store specific offset values per object and it will be very useful for detecting updates (so we can solve the T65834 efficiently).
Another change is that you no longer need to create and maintain a context handle. (This facilitates adaptation to modal operators).

Diff Detail

Repository
rB Blender

Event Timeline

  • Rename /selec. to /select_engine.
  • Move DRW_select_context_create to DRW_draw_select_id and not the other way round.
  • Fix crash in in weigt paint selection.
Germano Cavalcante (mano-wii) retitled this revision from New selection engine. to Selection engine..Jun 20 2019, 9:51 PM
Campbell Barton (campbellbarton) requested changes to this revision.Jun 21 2019, 3:45 AM

DRW_select_object_first_get / DRW_select_object_next_get to loop over objects seems a bit strange, would be nice if we could avoid it.

Tested this and it's noticeably slower with many objects (Select all and press tab:

)

Selection goes from nearly instant to taking around a second on an optimized build.

source/blender/draw/DRW_engine.h
140–145

Causes error building, doesn't match function signature.

source/blender/draw/engines/select/select_engine.c
499

See other comment regarding for loop.

source/blender/draw/intern/draw_manager.c
2596–2599

This will run even if ob is NULL.

Even though it doesn't crasn on an empty scene, would prefer more straightforward loop.

for (Object *ob = DRW_select_object_first_get(); ob; ob = DRW_select_object_next_get(ob)) { ... }

This revision now requires changes to proceed.Jun 21 2019, 3:45 AM
Germano Cavalcante (mano-wii) marked 3 inline comments as done.Jun 21 2019, 7:16 AM
Germano Cavalcante (mano-wii) updated this revision to Diff 16004.
  • Cleanup
  • Fix build error
  • Use for loop instead o while
  • Remove DrawData.

I can't redo the slowness in my setup.
The runtime of view3d_select_exec corresponds to only 1.08% of the total time of a frame here, making the visual comparison between the builds difficult.
However I removed the use of the DrawData in the patch, thus also removing the use of linklists and returning to the initial solution of arrays.
This may help a bit, but if the problem persists, the only solution I see is to do the whole drawing in select_draw_scene. (No Shadow Groups or pass).

(The idea of using DrawData to identify updates can be considered in another patch).

It's running faster after this update, generally LGTM, although there are some warnings.

[ 72%: -107] Building C object source/blender/draw/CMakeFiles/bf_draw.dir/engines/select/select_engine.c.o
/src/blender/source/blender/draw/engines/select/select_engine.c:131:13: warning: function declaration isn’t a prototype [-Wstrict-prototypes]
  131 | static void draw_select_framebuffer_select_id_setup()
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/src/blender/source/blender/draw/engines/select/select_engine.c: In function ‘draw_select_id_object’:
/src/blender/source/blender/draw/engines/select/select_engine.c:161:49: warning: unused parameter ‘rv3d’ [-Wunused-parameter]
  161 |                                   RegionView3D *rv3d,
      |                                   ~~~~~~~~~~~~~~^~~~
/src/blender/source/blender/draw/engines/select/select_engine.c: In function ‘DRW_select_context_create’:
/src/blender/source/blender/draw/engines/select/select_engine.c:577:39: warning: unused parameter ‘bases’ [-Wunused-parameter]
  577 |                                Base **bases,
      |                                ~~~~~~~^~~~~
This revision is now accepted and ready to land.Jun 21 2019, 10:20 AM
source/blender/draw/DRW_engine.h
178

Should be select_engine.c

source/blender/draw/engines/select/select_engine.h
17

2019

Campbell Barton (campbellbarton) requested changes to this revision.Jun 21 2019, 10:26 AM

Noticed that border Clipping (Alt-B) isn't supported, shouldn't be hard to add though.

This revision now requires changes to proceed.Jun 21 2019, 10:26 AM

I'm not sure if now is the right time to commit this kind of refactor. The last selection changes caused various platform specific issues which were hard to debug.

We should only do this if we're really confident about it, or if it's needed to fix an important issue.

  • Silence warnings and correct typos

We can wait for the launch of blender 2.81.
The changes are more on the CPU side (except for floatBitsToUint(intBitsToFloat(id))) so I believe it is less problematic.

  • Use DRW_shgroup_create_sub instead of DRW_shgroup_create.
  • Use DRW_shgroup_create_sub instead of DRW_shgroup_create.
  • Silence warnings
  • suport for border Clipping (Alt-B)
Germano Cavalcante (mano-wii) marked 2 inline comments as done.Tue, Jul 30, 12:32 AM
  • Removes comment that no longer applies to new code
This revision is now accepted and ready to land.Tue, Jul 30, 8:23 AM