Page MenuHome

UI: Add dimensions to object properties tab
Needs ReviewPublic

Authored by Hans Goudey (HooglyBoogly) on Aug 23 2020, 6:31 AM.

Details

Summary

For the 1 year anniversary of this patch, D5577, which added these buttons, just
in a technically incorrect way, I decided to implement this as a template.

The original reasoning from @William Reynish (billreynish)'s patch:

Currently it's a weak point in Blender, that users cannot access the object dimensions
more easily. Currently we hide this inside the 3D View Sidebar. For such a basic
and useful concept as setting the dimensions, I would like this to be more apparent
in the UI, and also discourage opening the Sidebar just for this one item.

Additionally, this patch reuses the template for the N-panel dimensions buttons.
That code was a mess in my opinion, it called the same function with different
arguments for adding the buttons and applying the changes.

There is one remaining issue: multi-drag doesn't work properly for these buttons.
I still need to investigate that more, but maybe it's obvious for someone more
familiar with that system.

Just joking about the anniversary thing, I just noticed that now

Diff Detail

Repository
rB Blender
Branch
arcpatch-D8681 (branched from master)
Build Status
Buildable 10490
Build 10490: arc lint + arc unit

Event Timeline

Hans Goudey (HooglyBoogly) requested review of this revision.Aug 23 2020, 6:31 AM
Hans Goudey (HooglyBoogly) created this revision.
  • Fix accidentally duplicated comment
  • Remove period from UI description
  • Comment out test code temporarily
Campbell Barton (campbellbarton) requested changes to this revision.Aug 23 2020, 6:58 AM

Works well, minor comments inline.

source/blender/editors/interface/interface_templates.c
7449–7451

A brief explanation of why this isn't used would be good, also, for commented blocks of code, I'd normally is #if 0

7458

SNPRINTF() is simpler in this case.

7461

Mismatch between this and the check below seems odd, if there is a reason to define a column then not put anything in it, that should have an explanation.

7472

Any reason not to use UI_UNIT_Y ?

7480–7481

Prefer #if 0 commented code, with brief note on why it's not used.

This revision now requires changes to proceed.Aug 23 2020, 6:58 AM
Hans Goudey (HooglyBoogly) marked 5 inline comments as done.
  • Use UI_UNIT_Y
  • Use SNPRINTF
  • Clarify property split and property decorate behavior
  • Remove temporary testing code
  • Add more detail to comments
  • Reuse the initial struct allocation for the first button

@Campbell Barton (campbellbarton) I haven't been able to find what's causing the issue with multi-button
dragging here (only X is changed if you drag down from the top). Some research shows that
this was an issue with the initial implementation of the dimensions buttons and multi-but
dragging, but I haven't found what exactly I've done to cause the issue. Any guesses?

  • Merge branch 'master' into arcpatch-D8681
  • Fix after merge
  • Merge branch 'master' into arcpatch-D8681
  • A tiny bit of cleanup

I'm spent some more time trying to figure out why multi-button dragging isn't working in this case, with no luck though.

ui_multibut_lookup just fails in this case, but I haven't been able to find what differentiates it from cases that work.

Note that multi-button dragging never worked properly with the dimension buttons, T38587.

Actually that's what I thought at first, but it does work in master, at least for me!

Aha, I found it! This comment held the key to the solution: /* NOTE: if but->poin is allocated memory for every defbut, things fail... */
The old buttons necessary for multi-drag weren't kept around because they weren't "equal" to the old ones as their argument pointers had changed.

With this hacky diff the buttons work properly:

diff --git a/source/blender/editors/interface/interface_templates.c b/source/blender/editors/interface/interface_templates.c
index 11196874b2f..012f9672cfa 100644
--- a/source/blender/editors/interface/interface_templates.c
+++ b/source/blender/editors/interface/interface_templates.c
@@ -7327,7 +7327,10 @@ static void dimensions_apply_fn(bContext *C, void *ob_dims_store_v, void *ob_v)
 
 void uiTemplateObjectDimensions(uiLayout *layout, Object *ob)
 {
-  ObjectDimensionsStorage *ob_dims_store = MEM_mallocN(sizeof(ObjectDimensionsStorage), __func__);
+  if (ob->dimensions_apply_stupid_hack == NULL) {
+    ob->dimensions_apply_stupid_hack = MEM_mallocN(sizeof(ObjectDimensionsStorage), __func__);
+  }
+  ObjectDimensionsStorage *ob_dims_store = ob->dimensions_apply_stupid_hack;
   BKE_object_dimensions_get(ob, ob_dims_store->ob_dims);
   copy_v3_v3(ob_dims_store->ob_dims_orig, ob_dims_store->ob_dims);
   copy_v3_v3(ob_dims_store->ob_scale_orig, ob->scale);
@@ -7343,10 +7346,6 @@ void uiTemplateObjectDimensions(uiLayout *layout, Object *ob)
     char text[3];
     SNPRINTF(text, use_property_split ? "%c" : "%c:", 'X' + i);
 
-    /* Reuse the allocation for the first button. */
-    ObjectDimensionsStorage *ob_dims_store_new = (i == 0) ? ob_dims_store :
-                                                            MEM_dupallocN(ob_dims_store);
-
     uiLayout *decorate_col;
     if (use_property_split) {
       /* Add the label so that the number button will be added to the right of it, keeping
@@ -7362,7 +7361,7 @@ void uiTemplateObjectDimensions(uiLayout *layout, Object *ob)
                            0,
                            0,
                            UI_UNIT_Y,
-                           &(ob_dims_store_new->ob_dims[i]),
+                           &(ob_dims_store->ob_dims[i]),
                            0.0f,
                            FLT_MAX,
                            0,
@@ -7377,7 +7376,7 @@ void uiTemplateObjectDimensions(uiLayout *layout, Object *ob)
        * so these buttons align with other buttons that do have decorators to the right. */
       uiItemL(decorate_col, "", ICON_BLANK1);
     }
-    UI_but_funcN_set(but, dimensions_apply_fn, ob_dims_store_new, ob);
+    UI_but_func_set(but, dimensions_apply_fn, ob_dims_store, ob);
   }
 }
 
diff --git a/source/blender/makesdna/DNA_object_types.h b/source/blender/makesdna/DNA_object_types.h
index 586c704e0f1..104ae6b19d6 100644
--- a/source/blender/makesdna/DNA_object_types.h
+++ b/source/blender/makesdna/DNA_object_types.h
@@ -386,6 +386,8 @@ typedef struct Object {
 
   struct PreviewImage *preview;
 
+  void *dimensions_apply_stupid_hack;
+
   /** Runtime evaluation data (keep last). */
   Object_Runtime runtime;
 } Object;

This is basically a replacement for a field in View3D_Runtime:

/** Nkey panel stores stuff here. */
void *properties_storage;

@Campbell Barton (campbellbarton), if you think something like this is reasonable, where should this memory live? It could be stored per region? Or two other options:

  • Add a flag to disable multi-drag on these buttons
  • Somehow change ui_but_equals_old to account for this situation.