Page MenuHome

Expose BLI_string_flip_side_name as bpy.utils.flip_name
ClosedPublic

Authored by Demeter Dzadik (Mets) on Aug 26 2021, 12:55 PM.

Details

Summary

Expose a new function in `bpy.utils.flip_name(name, strip_number=False)
that allows flipping bone names, eg "Bone.L" -> "Bone.R".

Useful for add-ons to avoid re-implementing Blender's name flipping.


Many thanks to @Sergey Sharybin (sergey) and @Sebastian Parborg (zeddb) for help :)

Diff Detail

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

Event Timeline

Demeter Dzadik (Mets) requested review of this revision.Aug 26 2021, 12:55 PM
Demeter Dzadik (Mets) created this revision.
  • forgot to remove a test line
  • mistake in the docstring
Demeter Dzadik (Mets) edited the summary of this revision. (Show Details)Aug 26 2021, 1:03 PM
  • forgot to mention the default value of strip_number in the docstring
Demeter Dzadik (Mets) edited the summary of this revision. (Show Details)Aug 26 2021, 3:59 PM
Campbell Barton (campbellbarton) requested changes to this revision.Sep 1 2021, 7:48 AM
Campbell Barton (campbellbarton) added inline comments.
source/blender/python/intern/bpy.c
157

Prefer name strip_digits, more closely follows Python's terminology (str.isdigits(), string.digits, string.hexdigits).

164

Use double tack for literals:

``.###``
173

from_name can be positional only argument, an empty string can be used here "".

180

No need to call strlen, instead pass s# to _PyArg_ParseTupleAndKeywordsFast.

182–185

This check isn't needed, _PyArg_ParseTupleAndKeywordsFast would return an error in this case, also the strlen above would have crashed if this is NULL.

194

No need to use strlen here, BLI_string_flip_side_name now returns the string length.

This revision now requires changes to proceed.Sep 1 2021, 7:48 AM
Campbell Barton (campbellbarton) added inline comments.
source/blender/python/intern/bpy.c
164

Correction: Use double back-tick for literals:

source/blender/python/intern/bpy.c
180

Can you elaborate on this one? Would something like "s#|$O&:flip_name" feed the string length into an argument, or so?

source/blender/python/intern/bpy.c
180

Yes, check the documentation for usage. https://docs.python.org/3/c-api/arg.html#c.Py_BuildValue

Demeter Dzadik (Mets) marked 4 inline comments as done.
  • rename strip_number to strip_digits
  • remove unneeded Null check

So, I tried making the changes but touching almost anything makes it crash or go haywire, which is pretty much what I expected :D I may need some further assistance.

source/blender/python/intern/bpy.c
173

Positional only arguments don't exist in Python, and even non-keyword-only arguments are usually discouraged, I don't see why we would want to prevent the caller from specifying the keyword, maybe I'm misunderstanding.

180

Following code crashes:

Py_ssize_t from_name_len;

static const char *_keywords[] = {"from_name", "strip_digits", NULL};
static _PyArg_Parser _parser = {"s#|$O&:flip_name", _keywords, 0};
if (!_PyArg_ParseTupleAndKeywordsFast(
        args, kw, &_parser, &from_name, &from_name_len, PyC_ParseBool, &strip_digits)) {
  return NULL;
}

The debugger is giving me something very odd:

==2372449==ERROR: AddressSanitizer: requested allocation size 0x7fff00000008 (0x7fff00001008 after adjustments for alignment, red zones etc.) exceeds maximum supported size of 0x10000000000 (thread T0)
    #0 0x7ffff7682517 in malloc (/lib/x86_64-linux-gnu/libasan.so.6+0xb0517)
    #1 0x1f2ea7f7 in _PyObject_Malloc Objects/obmalloc.c:1645
    #2 0x1f2ea7f7 in _PyObject_Malloc Objects/obmalloc.c:1638
194

With this code:

size_t size = from_name_len + 2;
char *name_flip_str = PyMem_MALLOC(size);

BLI_string_flip_side_name(name_flip_str, from_name, strip_number, size);

PyObject *return_value = PyUnicode_FromStringAndSize(name_flip_str, size);

I get strange results when I call the function from the PyAPI:

>>> bpy.utils.flip_name('a.Right')
'a.Left\x00\x00p'

>>> bpy.utils.flip_name('a.Left')
'a.Right\x00'

>>> bpy.utils.flip_name('a.L')
Traceback (most recent call last):
  File "<blender_console>", line 1, in <module>
UnicodeDecodeError: 'utf-8' codec can't decode byte 0xff in position 4: invalid start byte

>>> bpy.utils.flip_name('a.R')
Traceback (most recent call last):
  File "<blender_console>", line 1, in <module>
UnicodeDecodeError: 'utf-8' codec can't decode byte 0xff in position 4: invalid start byte
source/blender/python/intern/bpy.c
194

The input size wont work, the return value is the length, e.g:

const size_t name_flip_str_len = BLI_string_flip_side_name(name_flip_str, from_name, strip_digits, size);

PyObject *return_value = PyUnicode_FromStringAndSize(name_flip_str, name_flip_str_len);

PyMem_FREE(name_flip_str);

return return_value;
  • Get string length from BLI_string_flip_name.

Still couldn't figure out how to use the "s#" thing without crashing, even with Sergey's help, so that extra strlen() call had to stay.
I also used the formatting that you suggested in the chat earlier (P2581), although I know that wasn't directed at this patch, I liked the idea so I thought might as well get the ball rolling.

  • Use 's#' for accessing string length, other minor changes.
  • Get string length from BLI_string_flip_name.

Still couldn't figure out how to use the "s#" thing without crashing, even with Sergey's help, so that extra strlen() call had to stay.

This may have been caused by PY_SSIZE_T_CLEAN not being defined (I got a warning about this), updated the patch, can you check if this works?


That seems to work! Although I have no idea why :D Thanks a lot!
I don't have commit rights, so if you're happy with this, commit it for me please! ^^

Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)
This revision is now accepted and ready to land.Nov 9 2021, 3:20 PM