Details

Reviewers

Commits

rB2d041fc46823: Object: Speed up duplication of large selections by doing fewer collection syncs

Summary

Previous code was doing N collection syncs when duplicating N objects. New code avoids all the intermediate syncs by using BKE_layer_collection_resync_forbid and BKE_layer_collection_resync_allow, and then does one BKE_main_collection_sync + BKE_main_collection_sync_remap for the whole operation. There is some complexity involved where the Base things of newly duplicated objects can't be found yet, without the sync, so some work on them (marking them selected, active, ...) has to be deferred until after the sync.

The implementation uses the same "forbit sync" machinery as added & used in D11889. A "much more proper fix" would be to make collection syncs not be expensive at all (T73411), but who knows when that would get implemented.

Timings: scene with 10k cubes, each with unique mesh (Windows, VS2022 Release build, AMD Ryzen 5950X):

Shift+D duplicate: 13.6s -> 9.2s
Alt+D duplicate: 4.76s -> 1.53s

Most of remaining time is within BKE_id_new_name_validate, for which there's a separate optimization T73412 & D14162.

Diff Detail

Repository: rB Blender

Event Timeline

Aras Pranckevicius (aras_p) requested review of this revision.Feb 18 2022, 7:30 PM

Aras Pranckevicius (aras_p) planned changes to this revision.

Aras Pranckevicius (aras_p) created this revision.

Aras Pranckevicius (aras_p) created this object with visibility "Aras Pranckevicius (aras_p)".

Aras Pranckevicius (aras_p) created this object with edit policy "Aras Pranckevicius (aras_p)".

Aras Pranckevicius (aras_p) changed the visibility from "Aras Pranckevicius (aras_p)" to "Public (No Login Required)".Feb 20 2022, 1:42 PM

Aras Pranckevicius (aras_p) changed the edit policy from "Aras Pranckevicius (aras_p)" to "All Users".

Aras Pranckevicius (aras_p) edited the summary of this revision. (Show Details)Feb 20 2022, 1:47 PM

Rebased on latest master
This allowed some simplification because obect_add.c got moved into C++

In the future, it's helpful to see the diff here with a bit of context (-U1000 when running git diff).

source/blender/editors/object/object_add.cc
3741 ↗	(On Diff #52155)	`for (const int i : new_objects.index_range())` or even `for (Object *object : new_objects)`
3754 ↗	(On Diff #52155)	The style guide mentions referring to symbols with a leading `#`.

Code style updates after comments from Hans

Aras Pranckevicius (aras_p) marked 2 inline comments as done.Jun 2 2022, 2:14 PM

Looks fairly straight and simple. mainly comments on naming below.

source/blender/editors/object/object_add.cc
3599 ↗	(On Diff #52166)	Use `r_` prefix for return parameters, avoid too much 'compression' of names to the point they become meaningless: `r_ob_new`
3700–3701 ↗	(On Diff #52166)	Document precisely what are the data stored here (the base of the original object, and the copy of the original object)
3703 ↗	(On Diff #52166)	`ob_new_active` ?
3706 ↗	(On Diff #52166)	`ob_new`
3739 ↗	(On Diff #52166)	`ob_new`
3740 ↗	(On Diff #52166)	`base_new`
3753 ↗	(On Diff #52166)	better store explicitly `item.first` into a `Base` pointer, and use it here (`Base *base_source = item.first` at the beginning of this loop).