[Bf-committers] [Bf-blender-cvs] SVN commit: /data/svn/bf-blender [56063] trunk/blender/source/blender: Fix: when using a search menu with an operator's enum prop, the operator was previously always executed with default options ( appart from the search-set enum, of course).

Ton Roosendaal ton at blender.org
Fri Apr 19 12:49:22 CEST 2013


Hi,

It breaks every search at the moment.
I know rna code is complex, but I don't understand a line of this... I also don't understand the description of the feature. What's the use case here?

One thing for sure: you cannot store stuff in buttons, storage should be done outside, by callers. 

The crash is because stored rna properties get freed elsewhere.

-Ton-

------------------------------------------------------------------------
Ton Roosendaal  Blender Foundation   ton at blender.org    www.blender.org
Blender Institute   Entrepotdok 57A  1018AD Amsterdam   The Netherlands

On 19 Apr, 2013, at 12:16, Lukas Tönne wrote:

> This commit seems to have broken the
> WM_enum_search_invoke function, used as wm.invoke_search_popup(self) in the
> node.add_search operator. Invoking the search popup (via ctrl+A in node
> editor -> "Search ...") and then exiting either by moving away the mouse or
> by executing one of the items leads to free'd memory access and crash:
> 
> http://www.pasteall.org/41525
> 
> Reported by Francesco Siddi via Sergey in IRC.
> 
> 
> On Mon, Apr 15, 2013 at 5:01 PM, Bastien Montagne <montagne29 at wanadoo.fr>wrote:
> 
>> Revision: 56063
>> 
>> http://projects.blender.org/scm/viewvc.php?view=rev&root=bf-blender&revision=56063
>> Author:   mont29
>> Date:     2013-04-15 15:01:12 +0000 (Mon, 15 Apr 2013)
>> Log Message:
>> -----------
>> Fix: when using a search menu with an operator's enum prop, the operator
>> was previously always executed with default options (appart from the
>> search-set enum, of course). Now we store the op's properties in search
>> button, so that you can specify non-default options (as it was already
>> possible with e.g. pop-up menu from an operator's enum prop).
>> 
>> To achieve this, some code (callbacks and search button creation) was
>> moved from wm_operators.c to interface/interface.c, and a new UI function
>> was added, uiDefSearchButO_ptr.
>> 
>> Note: This new code uses the fact that uiButHandleFunc callbacks get
>> executed before operator when one of its arg is the button itself!
>> 
>> Many thanks to Campbell who helped me a lot with this patch!
>> 
>> Cleanup: also removed two unused pointers from uiBut struct.
>> 
>> Modified Paths:
>> --------------
>>    trunk/blender/source/blender/editors/include/UI_interface.h
>>    trunk/blender/source/blender/editors/interface/interface.c
>>    trunk/blender/source/blender/editors/interface/interface_handlers.c
>>    trunk/blender/source/blender/editors/interface/interface_intern.h
>>    trunk/blender/source/blender/windowmanager/intern/wm_operators.c
>> 
>> Modified: trunk/blender/source/blender/editors/include/UI_interface.h
>> ===================================================================
>> --- trunk/blender/source/blender/editors/include/UI_interface.h 2013-04-15
>> 14:55:42 UTC (rev 56062)
>> +++ trunk/blender/source/blender/editors/include/UI_interface.h 2013-04-15
>> 15:01:12 UTC (rev 56063)
>> @@ -605,6 +605,9 @@
>> uiBut *uiDefHotKeyevtButS(uiBlock *block, int retval, const char *str,
>> int x, int y, short width, short height, short *keypoin, short *modkeypoin,
>> const char *tip);
>> 
>> uiBut *uiDefSearchBut(uiBlock *block, void *arg, int retval, int icon,
>> int maxlen, int x, int y, short width, short height, float a1, float a2,
>> const char *tip);
>> +uiBut *uiDefSearchButO_ptr(uiBlock *block, struct wmOperatorType *ot,
>> IDProperty *properties,
>> +                           void *arg, int retval, int icon, int maxlen,
>> int x, int y,
>> +                           short width, short height, float a1, float a2,
>> const char *tip);
>> 
>> uiBut *uiDefAutoButR(uiBlock *block, struct PointerRNA *ptr, struct
>> PropertyRNA *prop, int index, const char *name, int icon, int x1, int y1,
>> int x2, int y2);
>> int uiDefAutoButsRNA(uiLayout *layout, struct PointerRNA *ptr, bool
>> (*check_prop)(struct PointerRNA *, struct PropertyRNA *), const char
>> label_align);
>> 
>> Modified: trunk/blender/source/blender/editors/interface/interface.c
>> ===================================================================
>> --- trunk/blender/source/blender/editors/interface/interface.c  2013-04-15
>> 14:55:42 UTC (rev 56062)
>> +++ trunk/blender/source/blender/editors/interface/interface.c  2013-04-15
>> 15:01:12 UTC (rev 56063)
>> @@ -3832,6 +3832,82 @@
>>        }
>> }
>> 
>> +/* Callbacks for operator search button. */
>> +static void operator_enum_search_cb(const struct bContext *C, void *but,
>> const char *str, uiSearchItems *items)
>> +{
>> +       wmOperatorType *ot = ((uiBut *)but)->optype;
>> +       PropertyRNA *prop = ot->prop;
>> +
>> +       if (prop == NULL) {
>> +               printf("%s: %s has no enum property set\n",
>> +                      __func__, ot->idname);
>> +       }
>> +       else if (RNA_property_type(prop) != PROP_ENUM) {
>> +               printf("%s: %s \"%s\" is not an enum property\n",
>> +                      __func__, ot->idname,
>> RNA_property_identifier(prop));
>> +       }
>> +       else {
>> +               PointerRNA *ptr = uiButGetOperatorPtrRNA(but);  /* Will
>> create it if needed! */
>> +               EnumPropertyItem *item, *item_array;
>> +               int do_free;
>> +
>> +               RNA_property_enum_items((bContext *)C, ptr, prop,
>> &item_array, NULL, &do_free);
>> +
>> +               for (item = item_array; item->identifier; item++) {
>> +                       /* note: need to give the index rather than the
>> identifier because the enum can be freed */
>> +                       if (BLI_strcasestr(item->name, str)) {
>> +                               if (false == uiSearchItemAdd(items,
>> item->name, SET_INT_IN_POINTER(item->value), 0))
>> +                                       break;
>> +                       }
>> +               }
>> +
>> +               if (do_free)
>> +                       MEM_freeN(item_array);
>> +       }
>> +}
>> +
>> +static void operator_enum_call_cb(struct bContext *UNUSED(C), void *but,
>> void *arg2)
>> +{
>> +       wmOperatorType *ot = ((uiBut *)but)->optype;
>> +       PointerRNA *opptr = uiButGetOperatorPtrRNA(but);  /* Will create
>> it if needed! */
>> +
>> +       if (ot) {
>> +               if (ot->prop) {
>> +                       RNA_property_enum_set(opptr, ot->prop,
>> GET_INT_FROM_POINTER(arg2));
>> +                       /* We do not call op from here, will be called by
>> button code.
>> +                        * ui_apply_but_funcs_after() (in
>> interface_handlers.c) called this func before checking operators,
>> +                        * because one of its parameters is the button
>> itself!
>> +                        */
>> +               }
>> +               else {
>> +                       printf("%s: op->prop for '%s' is NULL\n",
>> __func__, ot->idname);
>> +               }
>> +       }
>> +}
>> +
>> +/* Same parameters as for uiDefSearchBut, with an additional operator
>> pointer, from where to get property to search,
>> + * operator type, and operator porperties. */
>> +uiBut *uiDefSearchButO_ptr(uiBlock *block, wmOperatorType *ot, IDProperty
>> *properties,
>> +                           void *arg, int retval, int icon, int maxlen,
>> int x, int y,
>> +                           short width, short height, float a1, float a2,
>> const char *tip)
>> +{
>> +       uiBut *but;
>> +
>> +       but = uiDefSearchBut(block, arg, retval, icon, maxlen, x, y,
>> width, height, a1, a2, tip);
>> +       uiButSetSearchFunc(but, operator_enum_search_cb, but,
>> operator_enum_call_cb, NULL);
>> +
>> +       but->optype = ot;
>> +       but->opcontext = WM_OP_EXEC_DEFAULT;
>> +
>> +       if (properties) {
>> +               PointerRNA *ptr = uiButGetOperatorPtrRNA(but);
>> +               /* Copy pointer. */
>> +               RNA_pointer_create(NULL, ot->srna, properties, ptr);
>> +       }
>> +
>> +       return but;
>> +}
>> +
>> /* push a new event onto event queue to activate the given button
>>  * (usually a text-field) upon entering a popup
>>  */
>> 
>> Modified:
>> trunk/blender/source/blender/editors/interface/interface_handlers.c
>> ===================================================================
>> --- trunk/blender/source/blender/editors/interface/interface_handlers.c
>> 2013-04-15 14:55:42 UTC (rev 56062)
>> +++ trunk/blender/source/blender/editors/interface/interface_handlers.c
>> 2013-04-15 15:01:12 UTC (rev 56063)
>> @@ -199,7 +199,6 @@
>>        uiButHandleFunc func;
>>        void *func_arg1;
>>        void *func_arg2;
>> -       void *func_arg3;
>> 
>>        uiButHandleNFunc funcN;
>>        void *func_argN;
>> @@ -386,7 +385,6 @@
>> 
>>                after->func_arg1 = but->func_arg1;
>>                after->func_arg2 = but->func_arg2;
>> -               after->func_arg3 = but->func_arg3;
>> 
>>                after->funcN = but->funcN;
>>                after->func_argN = MEM_dupallocN(but->func_argN);
>> 
>> Modified: trunk/blender/source/blender/editors/interface/interface_intern.h
>> ===================================================================
>> --- trunk/blender/source/blender/editors/interface/interface_intern.h
>> 2013-04-15 14:55:42 UTC (rev 56062)
>> +++ trunk/blender/source/blender/editors/interface/interface_intern.h
>> 2013-04-15 15:01:12 UTC (rev 56063)
>> @@ -201,7 +201,6 @@
>>        uiButHandleFunc func;
>>        void *func_arg1;
>>        void *func_arg2;
>> -       void *func_arg3;
>> 
>>        uiButHandleNFunc funcN;
>>        void *func_argN;
>> @@ -251,7 +250,6 @@
>> 
>>        /* Operator data */
>>        struct wmOperatorType *optype;
>> -       struct IDProperty *opproperties;
>>        struct PointerRNA *opptr;
>>        short opcontext;
>>        unsigned char menu_key; /* 'a'-'z', always lower case */
>> 
>> Modified: trunk/blender/source/blender/windowmanager/intern/wm_operators.c
>> ===================================================================
>> --- trunk/blender/source/blender/windowmanager/intern/wm_operators.c
>> 2013-04-15 14:55:42 UTC (rev 56062)
>> +++ trunk/blender/source/blender/windowmanager/intern/wm_operators.c
>> 2013-04-15 15:01:12 UTC (rev 56063)
>> @@ -939,58 +939,6 @@
>> 
>> 
>> /* generic enum search invoke popup */
>> -static void operator_enum_search_cb(const struct bContext *C, void
>> *arg_ot, const char *str, uiSearchItems *items)
>> -{
>> -       wmOperatorType *ot = (wmOperatorType *)arg_ot;
>> -       PropertyRNA *prop = ot->prop;
>> -
>> -       if (prop == NULL) {
>> -               printf("%s: %s has no enum property set\n",
>> -                      __func__, ot->idname);
>> -       }
>> -       else if (RNA_property_type(prop) != PROP_ENUM) {
>> -               printf("%s: %s \"%s\" is not an enum property\n",
>> -                      __func__, ot->idname,
>> RNA_property_identifier(prop));
>> -       }
>> -       else {
>> -               PointerRNA ptr;
>> -
>> -               EnumPropertyItem *item, *item_array;
>> -               int do_free;
>> -
>> -               RNA_pointer_create(NULL, ot->srna, NULL, &ptr);
>> -               RNA_property_enum_items((bContext *)C, &ptr, prop,
>> &item_array, NULL, &do_free);
>> -
>> -               for (item = item_array; item->identifier; item++) {
>> -                       /* note: need to give the index rather than the
>> identifier because the enum can be freed */
>> -                       if (BLI_strcasestr(item->name, str))
>> -                               if (false == uiSearchItemAdd(items,
>> item->name, SET_INT_IN_POINTER(item->value), 0))
>> -                                       break;
>> -               }
>> -
>> -               if (do_free)
>> -                       MEM_freeN(item_array);
>> -       }
>> -}
>> -
>> -static void operator_enum_call_cb(struct bContext *C, void *arg1, void
>> *arg2)
>> -{
>> -       wmOperatorType *ot = arg1;
>> -
>> -       if (ot) {
>> -               if (ot->prop) {
>> -                       PointerRNA props_ptr;
>> -                       WM_operator_properties_create_ptr(&props_ptr, ot);
>> -                       RNA_property_enum_set(&props_ptr, ot->prop,
>> GET_INT_FROM_POINTER(arg2));
>> -                       WM_operator_name_call(C, ot->idname,
>> WM_OP_EXEC_DEFAULT, &props_ptr);
>> -                       WM_operator_properties_free(&props_ptr);
>> -               }
>> -               else {
>> -                       printf("%s: op->prop for '%s' is NULL\n",
>> __func__, ot->idname);
>> -               }
>> -       }
>> -}
>> -
>> static uiBlock *wm_enum_search_menu(bContext *C, ARegion *ar, void
>> *arg_op)
>> {
>>        static char search[256] = "";
>> @@ -1006,8 +954,8 @@
>> #if 0 /* ok, this isn't so easy... */
>>        uiDefBut(block, LABEL, 0, RNA_struct_ui_name(op->type->srna), 10,
>> 10, 180, UI_UNIT_Y, NULL, 0.0, 0.0, 0, 0, "");
>> #endif
>> -       but = uiDefSearchBut(block, search, 0, ICON_VIEWZOOM,
>> sizeof(search), 10, 10, 9 * UI_UNIT_X, UI_UNIT_Y, 0, 0, "");
>> -       uiButSetSearchFunc(but, operator_enum_search_cb, op->type,
>> operator_enum_call_cb, NULL);
>> +       but = uiDefSearchButO_ptr(block, op->type, op->ptr->data, search,
>> 0, ICON_VIEWZOOM, sizeof(search),
>> +                                 10, 10, 9 * UI_UNIT_X, UI_UNIT_Y, 0, 0,
>> "");
>> 
>>        /* fake button, it holds space for search items */
>>        uiDefBut(block, LABEL, 0, "", 10, 10 - uiSearchBoxHeight(),
>> uiSearchBoxWidth(), uiSearchBoxHeight(), NULL, 0, 0, 0, 0, NULL);
>> 
>> _______________________________________________
>> Bf-blender-cvs mailing list
>> Bf-blender-cvs at blender.org
>> http://lists.blender.org/mailman/listinfo/bf-blender-cvs
>> 
> _______________________________________________
> Bf-committers mailing list
> Bf-committers at blender.org
> http://lists.blender.org/mailman/listinfo/bf-committers



More information about the Bf-committers mailing list