Details
Diff Detail
- Repository
- rB Blender
Event Timeline
Please describe what is exactly happening.
I also vaguely remember ED_editors_init(C) was placed after wm_event_do_depsgraph() because editors initialization accessed depsgraph at some point.
We set ob->mode = OB_MODE_OBJECT in ED_editors_init() however depsgraph already evaluated these object. This way the evaluated copy of the object would have mode = OB_MODE_EDIT, while the original one would have it OB_MODE_OBJECT.
This would happen for any object that was saved in edit mode, but that was not of the same type of the active object (or when we have no active object).
I also vaguely remember ED_editors_init(C) was placed after wm_event_do_depsgraph() because editors initialization accessed depsgraph at some point.
I managed to get it to work without having to change the order around.
| source/blender/editors/util/ed_util.c | ||
|---|---|---|
| 167 | Nothing changed but tag is needed? Or am i missing something? | |
| source/blender/editors/util/ed_util.c | ||
|---|---|---|
| 157 | I think this else case needs the tag too? It should be fine to always tag the object along with ob->mode = OB_MODE_OBJECT; and not to worry about avoiding double tags. | |
| 173–180 | The logic here is different than the case where an active object exists. It checks BKE_object_has_mode_data, (ob->type == OB_GPENCIL) and sets the object mode before the linked data check. I see no good reason why the existence of an active object should influence other objects like that, which was already an issue in the existing code. Probably it's best to just have a single loop over all objects, and do some obact == NULL checks inside the loop. | |
Overall seems least intrusive.
Maybe worth verifying my memories about order of depsgraph update and editors update. Maybe editors don't need evaluated depsgraph nowadays (i don't have sources handy to verify).
More intrusive but CPU-friendly approach would be to run all non-depsgraph-related initialization before it is evaluated, and everything what depends on it after (but again, this requires verification of things in editors init).
That being said, have nothing against current solution. Just one DEG tag without obvious changes to ID seems weird. If data changes somewhere else, worth mentioning next to the tag call.
| source/blender/editors/util/ed_util.c | ||
|---|---|---|
| 157 | It probably doesn't need. The active object tends to always work. But agree that always tagging is simple in this case. | |
| 167 | We run into this when (ob->type != obact->type). What changed in this case is that we set ob->mode = OB_MODE_OBJECT (line 136). | |
| 173–180 | Agree, will look into that. | |
- From review: simplify code with double-tagging
- From review: Unify logic in a single object loop
Geez the patch became so small ... it makes no justice to the time I spent chasing this down =)
Patch is working fine here and ready for master imho.
@Sergey Sharybin (sergey) I agree that we are probably double-evaluating things, making this not the more efficient solution. But it is the least intrusive one. So even if we are to ever split this code may as well do it on top of a working code.