blender
/
blender
134
400
Fork 584
Code Issues 6.3k Pull Requests 758 Projects 19 Wiki Activity

RNA: Add missing raw types for DNA types #115761

Merged
Brecht Van Lommel merged 7 commits from Mysteryem/blender:fix_missing_raw_types_pr into main 2024-01-10 18:19:33 +01:00
Conversation 6 Commits 7 Files Changed 6 +173 -17
Thomas Barlow commented 2023-12-04 16:59:35 +01:00
Member
Copy Link

RNA raw types were missing for the int8_t, uchar (uint8_t),
ushort (uint16_t), int64_t and uint64_t DNA types types.

This patch adds the missing RNA raw types for all DNA types that can have
raw access.

Functional changes

Properties with one of the new unsigned raw types will raise a Python
OverflowError in #foreach_getset when attempting to read a negative
integer from bpy_prop_collection.foreach_set(). This is similar to the
existing behaviour of providing a Python int which is too large to
represent as a C long. The existing #foreach_getset code will print
the OverflowError and then raise a TypeError instead.

CPython's signed integer parsing functions accept numeric types that are
not the Python int instances by calling their __index__() method.
CPython's unsigned integer parsing functions, however, only accept
Python int instances. To make #foreach_getset accept the same
numeric types for unsigned raw types as it already accepts for signed
raw types, the unsigned integer parsing functions in py_capi_utils.h
have been updated to also call the __index__() method when given an
argument which is not a Python int instance.

Because the new unsigned integer parsing in #foreach_getset is using
the PyC_ family of functions, which perform their own overflow checks,
the existing signed integer parsing has also been updated to use the
PyC_ family of functions for signed integer parsing. Previously,
OverflowError would only have been raised when the parsed integer was
too large to fit in a C long. With this patch, OverflowError will be
raised whenever the parsed integer is too large to fit in the property's
raw type. Integer properties already have well-defined maximum and
minimum values which should fit within the property's raw type, and enum
properties have a fixed number of values they can take depending on
their items. The bigger change here, is that setting bool properties
which use PROP_RAW_BOOLEAN will now only accept 0/False and
1/True.

Now that PROP_RAW_CHAR parsing is using #PyC_Long_AsU8,
signed char buffers ("b" format) have been updated to no longer be
considered compatible with PROP_RAW_CHAR, matching the behaviour of
the other unsigned types only being considered compatible with unsigned
buffer formats.

The int64_t and uint64_t types can currently only be used by bool
properties (see IS_DNATYPE_BOOLEAN_COMPAT and the other macros in
RNA_define.hh), but bool properties only have raw access when they do
not use a bitmask and it doesn't make much sense to use an entire 64
bits just for a single bool property, so PROP_RAW_INT64 and
PROP_RAW_UINT64 are expected to be unused.

Performance changes

Providing raw types allows for faster access through
rna_access.cc#rna_raw_access, especially when a buffer compatible with
the property's raw type is passed through from
bpy_rna.cc#foreach_getset.

Before this patch, the bpy.types.Keyframe.handle_left_type property
did not have raw access, so #foreach_getset would fall back to
PROP_RAW_INT being the compatible type and then use the slower loop in
#rna_raw_access.

With this patch, the bpy.types.Keyframe.handle_left_type property has
raw access with the PROP_RAW_UINT8 type. Using a buffer compatible
with this raw type can use the faster #memcpy loop in
#rna_raw_access. Using a Python list will iterate the list into an
array whose type matches PROP_RAW_UINT8, which will also use the
faster #memcpy loop in #rna_raw_access.

Given an FCurve with 10500 keyframe points:

  • foreach_get with the handle_left_type property
    • Python list
      • Before: ~0.27ms
      • After: ~0.095ms (2.8x faster)
    • Compatible buffer (int compatible before, uint8_t compatible
      after)
      • Before: ~0.20ms
      • After: ~0.023ms (8.7x faster)
    • Incompatible buffer (NumPy ndarray compatible with short)
      • Before: ~0.61ms
      • After: ~0.42ms (1.5x faster)
  • foreach_set with the handle_left_type property
    • Python list
      • Before: ~0.33ms
      • After: ~0.095ms (3.5x faster)
    • Compatible buffer (int compatible before, uint8_t compatible
      after)
      • Before: ~0.24ms
      • After: ~0.023ms (10.4x faster)
    • Incompatible buffer (NumPy ndarray compatible with short)
      • Before: ~0.76ms
      • After: ~0.57ms (1.3x faster)

Most of the before and after cases appear to scale very closely to O(n),
so 10 times fewer elements is 10 times faster. The only exception is the
Compatible buffer case, which scales better in the after case. Given an
FCurve with 1000 keyframe points, foreach_set is only 8.3 times
faster than before.

The handling of unsigned types in #foreach_getset may need some discussion because the signed/unsigned of a property's raw type is not exposed to the Python API, which can make it more difficult for addon developers to create performant code that uses foreach_get/foreach_set (by passing in a compatible buffer).

In cases where differently named types are used interchangeably, e.g. ushort and uint16_t, I went with the uint16_t and similar types since they're suggested in the style guide, though I did not modify the existing types such as short -> int16_t for PROP_RAW_SHORT.

RNA raw types were missing for the `int8_t`, `uchar` (`uint8_t`), `ushort` (`uint16_t`), `int64_t` and `uint64_t` DNA types types. This patch adds the missing RNA raw types for all DNA types that can have raw access. **Functional changes** Properties with one of the new unsigned raw types will raise a Python `OverflowError` in `#foreach_getset` when attempting to read a negative integer from `bpy_prop_collection.foreach_set()`. This is similar to the existing behaviour of providing a Python `int` which is too large to represent as a C `long`. The existing `#foreach_getset` code will print the `OverflowError` and then raise a `TypeError` instead. CPython's signed integer parsing functions accept numeric types that are not the Python `int` instances by calling their `__index__()` method. CPython's unsigned integer parsing functions, however, only accept Python `int` instances. To make `#foreach_getset` accept the same numeric types for unsigned raw types as it already accepts for signed raw types, the unsigned integer parsing functions in `py_capi_utils.h` have been updated to also call the `__index__()` method when given an argument which is not a Python `int` instance. Because the new unsigned integer parsing in `#foreach_getset` is using the `PyC_` family of functions, which perform their own overflow checks, the existing signed integer parsing has also been updated to use the `PyC_` family of functions for signed integer parsing. Previously, `OverflowError` would only have been raised when the parsed integer was too large to fit in a C `long`. With this patch, `OverflowError` will be raised whenever the parsed integer is too large to fit in the property's raw type. Integer properties already have well-defined maximum and minimum values which should fit within the property's raw type, and enum properties have a fixed number of values they can take depending on their items. The bigger change here, is that setting bool properties which use `PROP_RAW_BOOLEAN` will now only accept `0`/`False` and `1`/`True`. Now that `PROP_RAW_CHAR` parsing is using `#PyC_Long_AsU8`, `signed char` buffers ("b" format) have been updated to no longer be considered compatible with `PROP_RAW_CHAR`, matching the behaviour of the other unsigned types only being considered compatible with unsigned buffer formats. The `int64_t` and `uint64_t` types can currently only be used by bool properties (see `IS_DNATYPE_BOOLEAN_COMPAT` and the other macros in `RNA_define.hh`), but bool properties only have raw access when they do not use a bitmask and it doesn't make much sense to use an entire 64 bits just for a single bool property, so `PROP_RAW_INT64` and `PROP_RAW_UINT64` are expected to be unused. **Performance changes** Providing raw types allows for faster access through `rna_access.cc#rna_raw_access`, especially when a buffer compatible with the property's raw type is passed through from `bpy_rna.cc#foreach_getset`. Before this patch, the `bpy.types.Keyframe.handle_left_type` property did not have raw access, so `#foreach_getset` would fall back to `PROP_RAW_INT` being the compatible type and then use the slower loop in `#rna_raw_access`. With this patch, the `bpy.types.Keyframe.handle_left_type` property has raw access with the `PROP_RAW_UINT8` type. Using a buffer compatible with this raw type can use the faster `#memcpy` loop in `#rna_raw_access`. Using a Python list will iterate the list into an array whose type matches `PROP_RAW_UINT8`, which will also use the faster `#memcpy` loop in `#rna_raw_access`. Given an `FCurve` with 10500 keyframe points: * `foreach_get` with the `handle_left_type` property * Python list * Before: ~0.27ms * After: ~0.095ms (2.8x faster) * Compatible buffer (`int` compatible before, `uint8_t` compatible after) * Before: ~0.20ms * After: ~0.023ms (8.7x faster) * Incompatible buffer (NumPy ndarray compatible with `short`) * Before: ~0.61ms * After: ~0.42ms (1.5x faster) * `foreach_set` with the `handle_left_type` property * Python list * Before: ~0.33ms * After: ~0.095ms (3.5x faster) * Compatible buffer (`int` compatible before, `uint8_t` compatible after) * Before: ~0.24ms * After: ~0.023ms (10.4x faster) * Incompatible buffer (NumPy ndarray compatible with `short`) * Before: ~0.76ms * After: ~0.57ms (1.3x faster) Most of the before and after cases appear to scale very closely to O(n), so 10 times fewer elements is 10 times faster. The only exception is the Compatible buffer case, which scales better in the after case. Given an `FCurve` with 1000 keyframe points, `foreach_set` is only 8.3 times faster than before. --- The handling of unsigned types in `#foreach_getset` may need some discussion because the signed/unsigned of a property's raw type is not exposed to the Python API, which can make it more difficult for addon developers to create performant code that uses `foreach_get`/`foreach_set` (by passing in a compatible buffer). In cases where differently named types are used interchangeably, e.g. `ushort` and `uint16_t`, I went with the `uint16_t` and similar types since they're suggested in the style guide, though I did not modify the existing types such as `short` -> `int16_t` for `PROP_RAW_SHORT`.
Thomas Barlow added 1 commit 2023-12-04 16:59:46 +01:00
06abf39164 Fix #115413: Missing RNA raw types for some DNA types 
RNA raw types were missing for the `int8_t`, `uchar` (`uint8_t`),
`ushort` (`uint16_t`), `int64_t` and `uint64_t` DNA types types.

For #115413 specifically, 27a5da4dc3 replaced the `char` fields used by
the `handle_left_type` and `handle_right_type` enum properties with
`uint8_t` fields. Without an RNA raw type being set for properties using
`uint8_t` fields, `rna_access.cc#RNA_property_raw_type()` would fall
back to assuming the raw type was `PROP_RAW_INT` which would then fail
due to #92621.

This patch adds the missing RNA raw types for all DNA types.

The `int64_t` and `uint64_t` types can currently only be used by bool
properties (see `IS_DNATYPE_BOOLEAN_COMPAT` and the other macros in
RNA_define.hh), which will means that `PROP_RAW_INT64` and
`PROP_RAW_UINT64` will be unused because bool properties use
`PROP_RAW_BOOLEAN`.

Properties with one of the new unsigned raw types have a difference in
behaviour to their signed counterparts in foreach_getset, in that
attempting to set a negative number raises a Python OverflowError (which
is caught and then re-raised as TypeError).
Thomas Barlow changed title from Fix #115413: Missing RNA raw types for some DNA types to WIP: Fix #115413: Missing RNA raw types for some DNA types 2023-12-20 02:08:30 +01:00
Thomas Barlow changed title from WIP: Fix #115413: Missing RNA raw types for some DNA types to WIP: RNA: Add missing raw types for some DNA types 2023-12-20 02:11:22 +01:00
Thomas Barlow added 2 commits 2023-12-20 05:41:15 +01:00
buildbot/vexp-code-patch-darwin-arm64 Build done. Details
buildbot/vexp-code-patch-linux-x86_64 Build done. Details
buildbot/vexp-code-patch-darwin-x86_64 Build done. Details
buildbot/vexp-code-patch-lint Build done. Details
buildbot/vexp-code-patch-windows-amd64 Build done. Details
buildbot/vexp-code-patch-coordinator Build done. Details
75a7c4ff49 Fix TypeError parsing NumPy scalars in foreach_set
Thomas Barlow changed title from WIP: RNA: Add missing raw types for some DNA types to WIP: RNA: Add missing raw types for DNA types 2023-12-20 18:34:38 +01:00
Thomas Barlow changed title from WIP: RNA: Add missing raw types for DNA types to RNA: Add missing raw types for DNA types 2023-12-20 18:37:39 +01:00
Thomas Barlow reviewed 2023-12-20 18:44:10 +01:00
source/blender/python/generic/py_capi_utils.h Outdated
@ -271,0 +277,4 @@
* means they cannot parse NumPy scalars and other numeric types.
*/
ulong PyC_Long_AsUnsignedLong(PyObject *value);
unsigned long long PyC_Long_AsUnsignedLongLong(PyObject *value);
Thomas Barlow commented 2023-12-20 18:44:10 +01:00
Author
Member
Copy Link

It does look a little odd having unsigned long long next to ulong. I could add typedef unsigned long long ulonglong; to BLI_sys_types.h and then change use of unsigned long long in this PR to ulonglong.

It does look a little odd having `unsigned long long` next to `ulong`. I could add `typedef unsigned long long ulonglong;` to `BLI_sys_types.h` and then change use of `unsigned long long` in this PR to `ulonglong`.
Brecht Van Lommel commented 2023-12-20 20:14:40 +01:00
Owner
Copy Link

From the Blender point of view we care about these being uint32_t and uint64_t. So I would suggest to use those types here and AsU32 and AsU64 in the function name.

From the Blender point of view we care about these being `uint32_t` and `uint64_t`. So I would suggest to use those types here and `AsU32` and `AsU64` in the function name.
Thomas Barlow commented 2023-12-20 21:19:01 +01:00
Author
Member
Copy Link

I didn't think that ulong would be guaranteed to fit in a uint32_t since I thought it was usually 64-bit on 64-bit Linux. Similarly for fitting unsigned long long into uint64_t which apparently is sometimes 128-bit or 96-bit padded to 128-bit. (Edit: I was thinking of long double, I'm not sure in what cases long long could be larger than 64-bit.) Unless Blender is compiled with settings that force these types to be those fixed sizes? I've seen assumptions about the size of int and smaller types in other parts of Blender's source, but not long and long long.

Since the intent is to replace #PyLong_AsUnsignedLong and #PyLong_AsUnsignedLongLong from CPython's API, I thought it would be better to use their return types and keep the names similar. There are also already implementations for #PyC_Long_AsU32 and similar in py_capi_utils.h.

I didn't think that `ulong` would be guaranteed to fit in a `uint32_t` since I thought it was usually 64-bit on 64-bit Linux. ~~Similarly for fitting `unsigned long long` into `uint64_t` which apparently is sometimes 128-bit or 96-bit padded to 128-bit.~~ (Edit: I was thinking of `long double`, I'm not sure in what cases `long long` could be larger than 64-bit.) Unless Blender is compiled with settings that force these types to be those fixed sizes? I've seen assumptions about the size of `int` and smaller types in other parts of Blender's source, but not `long` and `long long`. Since the intent is to replace `#PyLong_AsUnsignedLong` and `#PyLong_AsUnsignedLongLong` from CPython's API, I thought it would be better to use their return types and keep the names similar. There are also already implementations for `#PyC_Long_AsU32` and similar in `py_capi_utils.h`.
Thomas Barlow commented 2023-12-21 12:44:06 +01:00
Author
Member
Copy Link

I've had a further think about this and I don't think ulong potentially being 64-bit will be an issue because there is no PROP_RAW_ULONG and code should be using fixed-width types where possible.

I can instead implement the __index__() fallback directly into PyC_Long_AsU64, PyC_Long_AsU32, PyC_Long_AsU16 and PyC_Long_AsU8. I would remove both of the new functions in py_capi_utils.h and refactor PyC_Long_AsUnsignedLong into a static ulong pyc_Long_AsUnsignedLong(PyObject *value) function in py_capi_utils.cc, to avoid duplicate code in the 32-bit and smaller functions.

This would keep py_capi_utils.h using fixed-width types for integer parsing and would encourage the use of one of the fixed-width unsigned integer parsing functions instead of using PyLong_AsUnsignedLong or PyLong_AsUnsignedLongLong.

I've had a further think about this and I don't think `ulong` potentially being 64-bit will be an issue because there is no `PROP_RAW_ULONG` and code should be using fixed-width types where possible. I can instead implement the `__index__()` fallback directly into `PyC_Long_AsU64`, `PyC_Long_AsU32`, `PyC_Long_AsU16` and `PyC_Long_AsU8`. I would remove both of the new functions in `py_capi_utils.h` and refactor `PyC_Long_AsUnsignedLong` into a `static ulong pyc_Long_AsUnsignedLong(PyObject *value)` function in `py_capi_utils.cc`, to avoid duplicate code in the 32-bit and smaller functions. This would keep `py_capi_utils.h` using fixed-width types for integer parsing and would encourage the use of one of the fixed-width unsigned integer parsing functions instead of using `PyLong_AsUnsignedLong` or `PyLong_AsUnsignedLongLong`.
Brecht Van Lommel commented 2023-12-21 12:56:52 +01:00
Owner
Copy Link

This sounds good to me! In general we want to avoid using long in Blender wherever we can, in the places we do need it having it more contained like this helps.

This sounds good to me! In general we want to avoid using `long` in Blender wherever we can, in the places we do need it having it more contained like this helps.
Thomas Barlow commented 2023-12-20 19:04:59 +01:00
Author
Member
Copy Link

@ideasman42 @brecht Can I get a review from you both on this please? It looks like Campbell worked a lot on bpy_rna.cc#foreach_getset and py_capi_utils.h and Brecht on rna_access.cc#rna_raw_access.

@ideasman42 @brecht Can I get a review from you both on this please? It looks like Campbell worked a lot on `bpy_rna.cc#foreach_getset` and `py_capi_utils.h` and Brecht on `rna_access.cc#rna_raw_access`.
Brecht Van Lommel requested review from Brecht Van Lommel 2023-12-20 19:57:59 +01:00
Brecht Van Lommel requested review from Campbell Barton 2023-12-20 19:58:00 +01:00
Brecht Van Lommel requested changes 2023-12-20 20:14:59 +01:00
Brecht Van Lommel left a comment
Owner
Copy Link

Marking as request changes.

Marking as request changes.
Brecht Van Lommel commented 2023-12-20 20:15:30 +01:00
Owner
Copy Link

@blender-bot build

@blender-bot build
Thomas Barlow reviewed 2023-12-20 22:09:50 +01:00
source/blender/python/generic/py_capi_utils.cc Outdated
@ -1639,0 +1673,4 @@
PyObject *value_converted = PyNumber_Index(value);
if (value_converted == nullptr) {
/* A `TypeError` will have been raised. */
return unsigned long long(-1);
Thomas Barlow commented 2023-12-20 22:09:50 +01:00
Author
Member
Copy Link

I didn't know that functional-style casts can only be done with simple type specifiers, and for some reason it does compile on Windows.

My thoughts for fixing this would be either using static_cast<unsigned long long>(-1) or adding typedef unsigned long long ulonglong; to BLI_sys_types.h and then doing ulonglong(-1). Though, depending on feedback from the already requested changes, this code may be replaced with using fixed-with integer types, which would avoid the issue.

I didn't know that functional-style casts can only be done with simple type specifiers, and for some reason it does compile on Windows. My thoughts for fixing this would be either using `static_cast<unsigned long long>(-1)` or adding `typedef unsigned long long ulonglong;` to `BLI_sys_types.h` and then doing `ulonglong(-1)`. Though, depending on feedback from the already requested changes, this code may be replaced with using fixed-with integer types, which would avoid the issue.
Hans Goudey commented 2023-12-21 02:52:01 +01:00
Member
Copy Link

Not sure about how it fits with the other Python code, but if possible, fixed width types seem nicest. Other than that static_cast is totally fine here. Appreciate your desire to follow the style guide though :)

Not sure about how it fits with the other Python code, but if possible, fixed width types seem nicest. Other than that `static_cast` is totally fine here. Appreciate your desire to follow the style guide though :)
Thomas Barlow added 4 commits 2023-12-22 01:34:04 +01:00
93169bb632 Add `__index__` fallback to existing py_capi_utils functions rather than adding new functions 
This avoids adding new functions to the API that return the `unsigned long` and `unsigned long long` types.
0bed4aa84d Update foreach_set to use fixed-width integer parsing functions and PyC_Long_AsBool 
This is a potentially breaking change because these parsing functions
perform their own overflow checks, whereas before, `OverflowError` would
only have been raised when the input integer was too large to represent
as signed/unsigned `long`/`long long`, even when the raw property type
could only store much smaller values.

This change is probably OK however, because integer properties have a
well-defined minimum and maximum value, which should always fit in the
property's raw type, enum properties have a fixed number of values they
can take depending on the enum's items, and bool properties should
usually only be `0` and `1`.

Note, however, that bool properties with raw array access do not use the
#PROP_RAW_BOOLEAN type because DNA does not have a bool type, so the raw
property type will usually match one of the other single-byte types
instead. Most bool properties use a bitmask, which disables raw array
access, so this is expected to be an uncommon case.

For a user-facing release notes, the change could be written like:
`bpy_prop_collection.foreach_set()` is more restrictive with integer,
enum and bool properties, and will raise a `TypeError` when an input
value cannot be stored by the property's internal type.

If it's safe to do so, it may be worth replacing use of
#PROP_RAW_CHAR/`char`, #PROP_RAW_SHORT/`short` and #PROP_RAW_INT/`int`
with their fixed-width counterparts, so that all raw access is done with
fixed-width types.
dbe647a997 Remove "b" (`signed char`) buffers from being compatible with PROP_RAW_CHAR 
Now that reading an input sequence in foreach_set for a PROP_RAW_CHAR
property requires all values to fit within the minimum and maximum of
`uint8_t`, the buffer formats compatible with PROP_RAW_CHAR should be
updated to match.

Signed integer raw types are still considered compatible with unsigned
buffers, but maybe that should change too, since, for example, `128`
cannot be stored in a `int8_t`, but can in a "B" (`unsigned char`)
buffer, which would be considered compatible if the property had the
`PROP_UNSIGNED` flag.
buildbot/vexp-code-patch-linux-x86_64 Build done. Details
buildbot/vexp-code-patch-lint Build done. Details
buildbot/vexp-code-patch-darwin-x86_64 Build done. Details
buildbot/vexp-code-patch-darwin-arm64 Build done. Details
buildbot/vexp-code-patch-windows-amd64 Build done. Details
buildbot/vexp-code-patch-coordinator Build done. Details
3ddb3d0eb3 Fix additional probable compilation errors on non-Windows 
I expect the functional-style casts for `unsigned long long` and maybe
`long long` to fail to compile on non-Windows systems.

Existing code in bgl.cc#py_module_dict_add_int64 calls
#PyLong_FromLongLong and passes it an `int64_t` argument directly, so
I'm assuming it is safe to do this and also to pass a `uint64_t`
directly into #PyLong_FromUnsignedLongLong.
Brecht Van Lommel commented 2023-12-22 16:45:35 +01:00
Owner
Copy Link

@blender-bot build

@blender-bot build
Brecht Van Lommel approved these changes 2023-12-22 16:45:55 +01:00
Brecht Van Lommel left a comment
Owner
Copy Link

Thanks looks good to me.

Thanks looks good to me.
Thomas Barlow commented 2023-12-22 17:42:39 +01:00
Author
Member
Copy Link

I've updated the PR description to group together the changes based on whether they are functional or for performance, and I've updated and added the functional changes of the last few commits, reproduced below.

Updated:

CPython's signed integer parsing functions accept numeric types that are
not the Python int instances by calling their __index__() method.
CPython's unsigned integer parsing functions, however, only accept
Python int instances. To make #foreach_getset accept the same
numeric types for unsigned raw types as it already accepts for signed
raw types, the unsigned integer parsing functions in py_capi_utils.h
have been updated to also call the __index__() method when given an
argument which is not a Python int instance.

New:

Because the new unsigned integer parsing in #foreach_getset is using
the PyC_ family of functions, which perform their own overflow checks,
the existing signed integer parsing has also been updated to use the
PyC_ family of functions for signed integer parsing. Previously,
OverflowError would only have been raised when the parsed integer was
too large to fit in a C long. With this patch, OverflowError will be
raised whenever the parsed integer is too large to fit in the property's
raw type. Integer properties already have well-defined maximum and
minimum values which should fit within the property's raw type, and enum
properties have a fixed number of values they can take depending on
their items. The bigger change here, is that setting bool properties
which use PROP_RAW_BOOLEAN will now only accept 0/False and
1/True.

Now that PROP_RAW_CHAR parsing is using #PyC_Long_AsU8,
signed char buffers ("b" format) have been updated to no longer be
considered compatible with PROP_RAW_CHAR, matching the behaviour of
the other unsigned types only being considered compatible with unsigned
buffer formats.

I've updated the PR description to group together the changes based on whether they are functional or for performance, and I've updated and added the functional changes of the last few commits, reproduced below. Updated: > CPython's signed integer parsing functions accept numeric types that are > not the Python `int` instances by calling their `__index__()` method. > CPython's unsigned integer parsing functions, however, only accept > Python `int` instances. To make `#foreach_getset` accept the same > numeric types for unsigned raw types as it already accepts for signed > raw types, the unsigned integer parsing functions in `py_capi_utils.h` > have been updated to also call the `__index__()` method when given an > argument which is not a Python `int` instance. New: > Because the new unsigned integer parsing in `#foreach_getset` is using > the `PyC_` family of functions, which perform their own overflow checks, > the existing signed integer parsing has also been updated to use the > `PyC_` family of functions for signed integer parsing. Previously, > `OverflowError` would only have been raised when the parsed integer was > too large to fit in a C `long`. With this patch, `OverflowError` will be > raised whenever the parsed integer is too large to fit in the property's > raw type. Integer properties already have well-defined maximum and > minimum values which should fit within the property's raw type, and enum > properties have a fixed number of values they can take depending on > their items. The bigger change here, is that setting bool properties > which use `PROP_RAW_BOOLEAN` will now only accept `0`/`False` and > `1`/`True`. > > Now that `PROP_RAW_CHAR` parsing is using `#PyC_Long_AsU8`, > `signed char` buffers ("b" format) have been updated to no longer be > considered compatible with `PROP_RAW_CHAR`, matching the behaviour of > the other unsigned types only being considered compatible with unsigned > buffer formats.
Thomas Barlow reviewed 2023-12-24 02:34:16 +01:00
source/blender/python/intern/bpy_rna.cc
@ -5497,3 +5520,3 @@
break;
case PROP_RAW_BOOLEAN:
((bool *)array)[i] = int(PyLong_AsLong(item)) != 0;
((bool *)array)[i] = bool(PyC_Long_AsBool(item));
Thomas Barlow commented 2023-12-24 02:34:15 +01:00
Author
Member
Copy Link

I think it may be better to revert the change to bool parsing here because there is the py_capi_utils.h#PyC_AsArray() function, which is a good candidate for replacing the conversion from Python sequence to array here, and its bool parsing matches the existing int(PyLong_AsLong(item)) != 0 rather than using #PyC_Long_AsBool(). Reverting this change to bool parsing should make any future transition to using #PyC_AsArray() easier.

Edit: Or maybe not. While #PyC_AsArray() is faster for List/Tuple inputs, it appears to be slower for other sequences, such as NumPy ndarray, array.array and memoryview.

I think it may be better to revert the change to bool parsing here because there is the `py_capi_utils.h#PyC_AsArray()` function, which is a good candidate for replacing the conversion from Python sequence to array here, and its bool parsing matches the existing `int(PyLong_AsLong(item)) != 0` rather than using `#PyC_Long_AsBool()`. Reverting this change to bool parsing should make any future transition to using `#PyC_AsArray()` easier. Edit: Or maybe not. While `#PyC_AsArray()` is faster for `List`/`Tuple` inputs, it appears to be slower for other sequences, such as NumPy ndarray, array.array and memoryview.
Brecht Van Lommel commented 2024-01-02 12:38:42 +01:00
Owner
Copy Link

I'll wait with merging this in case @ideasman42 wants to have a look at the Python implementation.

I'll wait with merging this in case @ideasman42 wants to have a look at the Python implementation.
👍 1
Brecht Van Lommel commented 2024-01-10 18:08:52 +01:00
Owner
Copy Link

I think enough time has passed, merging.

I think enough time has passed, merging.
Brecht Van Lommel merged commit 992ec6487b into main 2024-01-10 18:19:33 +01:00
Brecht Van Lommel deleted branch fix_missing_raw_types_pr 2024-01-10 18:19:35 +01:00
Sign in to join this conversation.
Reviewers
No reviewers
Campbell Barton
Brecht Van Lommel
Labels
Clear labels   
Interest
Alembic

  
Interest
Animation & Rigging

  
Interest
Asset Browser

  
Interest
Asset Browser Project Overview

  
Interest
Audio

  
Interest
Automated Testing

  
Interest
Blender Asset Bundle

  
Interest
BlendFile

  
Interest
Collada

  
Interest
Compatibility
This issue affects/is about backward or forward compatibility

  
Interest
Compositing

  
Interest
Core

  
Interest
Cycles

  
Interest
Dependency Graph

  
Interest
Development Management

  
Interest
EEVEE

  
Interest
EEVEE & Viewport

  
Interest
Freestyle

  
Interest
Geometry Nodes

  
Interest
Grease Pencil

  
Interest
ID Management

  
Interest
Images & Movies

  
Interest
Import Export

  
Interest
Line Art

  
Interest
Masking

  
Interest
Metal

  
Interest
Modeling

  
Interest
Modifiers

  
Interest
Motion Tracking

  
Interest
Nodes & Physics

  
Interest
OpenGL

  
Interest
Overlay

  
Interest
Overrides

  
Interest
Performance

  
Interest
Physics

  
Interest
Pipeline, Assets & IO

  
Interest
Platforms, Builds & Tests

  
Interest
Python API

  
Interest
Render & Cycles

  
Interest
Render Pipeline

  
Interest
Sculpt, Paint & Texture

  
Interest
Text Editor

  
Interest
Translations

  
Interest
Triaging

  
Interest
Undo

  
Interest
USD

  
Interest
User Interface

  
Interest
UV Editing

  
Interest
VFX & Video

  
Interest
Video Sequencer

  
Interest
Virtual Reality

  
Interest
Vulkan

  
Interest
Wayland
Issues relating to Wayland windowing support.

  
Interest
Workbench

  
Interest: X11

Issues relating to Xorg/X11 support.

  
Legacy
Blender 2.8 Project

  
Legacy
Milestone 1: Basic, Local Asset Browser

  
Legacy
OpenGL Error

  
Meta
Good First Issue

  
Meta
Papercut

  
Meta
Retrospective

  
Meta
Security
Issues relating to security: https://wiki.blender.org/wiki/Process/Vulnerability_Reports

  
Module
Animation & Rigging

  
Module
Core

  
Module
Development Management

  
Module
EEVEE & Viewport

  
Module
Grease Pencil

  
Module
Modeling

  
Module
Nodes & Physics

  
Module
Pipeline, Assets & IO

  
Module
Platforms, Builds & Tests

  
Module
Python API

  
Module
Render & Cycles

  
Module
Sculpt, Paint & Texture

  
Module
Triaging

  
Module
User Interface

  
Module
VFX & Video

  
Platform
FreeBSD

  
Platform
Linux

  
Platform
macOS

  
Platform
Windows

  
Priority
High

  
Priority
Low

  
Priority
Normal

  
Priority
Unbreak Now!

  
Status
Archived

  
Status
Confirmed

  
Status
Duplicate

  
Status
Needs Info from Developers

  
Status
Needs Information from User

  
Status
Needs Triage

  
Status
Resolved

  
Type
Bug

  
Type
Design

  
Type
Known Issue

  
Type
Patch

  
Type
Report

  
Type
To Do

No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#115761
No description provided.
Delete Branch "Mysteryem/blender:fix_missing_raw_types_pr"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?