Page MenuHome

Edit Mesh Selection: Optimization: Draw only objects within the rect.
ClosedPublic

Authored by Germano Cavalcante (mano-wii) on Aug 8 2019, 12:44 AM.

Details

Summary

Nowadays this optimization will not be very efficient because almost never a user puts thousands of objects in edit mode.
But in the future the selection code may also be used in object mode (eg for snapping).
So to avoid using too much VRAM resources, it is good to avoid drawing all objects in the viewport.

The solution was to create an array with only objects that are detected within the selection area.
If the selection operator is modal, objects already detected are not removed from the array until view3d is moved or orbited.
To detect the object, its BoundBox is tested.
Since the Select Engine does not have a dedicated depth texture, whenever a new object is "found" the depth of the objects in the array already drawn is redrawn.

One thing to note is that an array (of struct SelectObjects) is created with all objects in context. This can be changed later.

Diff Detail

Repository
rB Blender
Branch
editmesh_selection_refactor (branched from master)
Build Status
Buildable 4361
Build 4361: arc lint + arc unit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Being able to draw a region once and re-use it is useful for circle-select for example.
However this patch looks like it will draw a small rectangle for circle select, then after cursor motion - need to draw another small rectangle.
Could there be a way to request the entire view is drawn for times when we know it will be reused?

This may be a confusion with the rect parameter in DRW_select_buffer_read.
Currently the drawing is made from the entire view.
The rect is used to calculate the required buffer size and to detect the objects that need to be drawn.

Clément Foucault (fclem) requested changes to this revision.Aug 9 2019, 2:38 PM
Clément Foucault (fclem) added inline comments.
source/blender/draw/engines/select/select_buffer.c
0

I don't really like having DRW_ functions here. Either rename them or put it elsewhere.

22

This is problematic since the texture space can be not centered. Also I'm not sure the texture space takes into account the loose edges/points.

28

I don't like this manual the manual culling here. Why not use DRW_culling_box_test ?

This revision now requires changes to proceed.Aug 9 2019, 2:38 PM
  • Add comments for main function DRW_draw_select_id
source/blender/draw/engines/select/select_buffer.c
22

I thought so too, but I noticed that the bb member always refers to the minimum and maximum mesh coordinates.
Location, rotation and scale are indicated by the members me->loc, me->rot and me->size.
I could skip testing for objects in edit mode, but that would practically remove the benefits of the patch.

28

It would be nice to use, but currently DRW_culling_box_test uses view->frustum_planes .
I could make a DRW_culling_box_test_ex that has as parameter the clipping planes in this case.

Germano Cavalcante (mano-wii) marked 2 inline comments as done.Aug 9 2019, 8:36 PM
Germano Cavalcante (mano-wii) updated this revision to Diff 16998.
  • Use DRW_culling_min_max_test instead of duplicating logic in similar functions.
  • Move interface file select_buffer.c out of the engine code.
  • Create a custom array of objects instead of copying the array of Bases.
  • Check element type in select_mode to draw depth.
  • Use descritive names
  • Silence warnings
  • Fix bitmap len
  • Improve Skip culling test hack
  • Optimize DRW_culling_min_max_test
  • Cleanup: Use descritive names
  • Invert DRW_culling_min_max_test bool
  • Cleanup: rename function
  • Store data in DrawData instead of integrating with a custom object array.
  • Simplify editselect_buf_cache_init
Clément Foucault (fclem) requested changes to this revision.Aug 13 2019, 1:23 PM

I'm fine with the functionality and how it is done, it is just about code organization.

source/blender/draw/DRW_select_buffer.h
35–36

What is the difference between objects and objects_draw? Explain what they contain in comments.

40

is it potentially going to grow? If not just use a bool.

40

Where is base_array_index_offsets ? Do you mean index_drawn_len is the number of element indices that are rendered to the bitmap?

42

do you really need an extra wrapper struct?

source/blender/draw/engines/select/select_engine.c
176–190

I still don't like base_ofs as a variable name. To me, it sounds like it is related to the Base struct which it isnt.

ob_offsets would sounds less confusing.

177–178

This line is not clear to me. I understand what follows, but not the condition. What is runtime.remaining? What is precalc_drawing.found_objects_len ? maybe just add some comments on the members of the struct.

218

you can disable culling by using DRW_shgroup_call_no_cull instead of DRW_shgroup_call. Or better, use DRW_shgroup_call_obmat. Thus removing the need for this hack.

source/blender/draw/intern/draw_manager.c
2602

I'm still not quite happy that this is not done by the SELECTID engine itself. The engine could create its own DRWView.

source/blender/draw/intern/draw_select_buffer.c
102

typo The rect has *been* clamped so you need to realign the buffer and fill in the blanks

616

Ok now I understand your intensions. So you are culling before iterating in draw_cache_populate. But why? Wouldn't it be more easy (and clear) to cull inside draw_cache_populate? once again to reduce code fragmentation.

664

Why can't you do that in SELECTID_init() ? We do something similar for Eevee and workbench to ensure TAA last frame is still valid.

This would reduce the selection code going into the draw manager.

This revision now requires changes to proceed.Aug 13 2019, 1:23 PM
  • Use bool instead of entering a bit width uint
  • Add comments, move found_objects_len and rename base_ofs.
  • Use DRW_shgroup_call_no_cull instead DRW_shgroup_call

    (More updates to come)
Germano Cavalcante (mano-wii) marked 8 inline comments as done.Aug 13 2019, 3:13 PM
Germano Cavalcante (mano-wii) added inline comments.
source/blender/draw/intern/draw_select_buffer.c
616

We need to know first if a new object has been added so we know if the depth of the objects already drawn needs to be redrawn.
Another solution would be looping draw_cache_populate twice in a row.

Germano Cavalcante (mano-wii) marked 2 inline comments as done.Aug 13 2019, 5:35 PM
Germano Cavalcante (mano-wii) updated this revision to Diff 17080.
  • Move draw engine utilities to the selection engine API
  • Only fill depth pass if a new object is found
  • Fix Depth pass issues
Germano Cavalcante (mano-wii) marked an inline comment as done.EditedAug 15 2019, 2:18 PM
Germano Cavalcante (mano-wii) updated this revision to Diff 17155.
  • Create DRWView on engine itself;
  • Fix BLI_assert failed: '!DRW_batch_requested(((GPUBatch **)&cache->batch)[i], 0)'

Looks much nicer now! Nice job!

This revision is now accepted and ready to land.Aug 15 2019, 3:02 PM