blender/blender-addons
19
43
Fork 96
Code Issues 630 Pull Requests 27 Projects 1 Releases Wiki Activity

FBX IO: Corner vert/edge and edge verts access with attributes #104648

Merged
Bastien Montagne merged 1 commits from Mysteryem/blender-addons:attribute_access_corner_edge_v_e_indices_pr into main 2023-06-26 17:43:25 +02:00
Conversation 2 Commits 1 Files Changed 3 +23 -21
Thomas Barlow commented 2023-05-29 05:09:23 +02:00
Member
Copy Link

Blender 3.6 moved corner (loop) vertex index and edge index, and edge
vertices to generic attributes. The old API still works for now, but is
slower and may be removed in 4.0, so this patch updates FBX IO to use
the new ".corner_vert", ".corner_edge" and ".edge_verts" attributes.

This patch makes no changes to the import or export of FBX files.

Blender 3.6 moved corner (loop) vertex index and edge index, and edge vertices to generic attributes. The old API still works for now, but is slower and may be removed in 4.0, so this patch updates FBX IO to use the new ".corner_vert", ".corner_edge" and ".edge_verts" attributes. This patch makes no changes to the import or export of FBX files.
Thomas Barlow commented 2023-05-29 05:42:23 +02:00
Author
Member
Copy Link

This PR depends on the base patch for adding attribute utility functions: #104645

This PR depends on the base patch for adding attribute utility functions: https://projects.blender.org/blender/blender-addons/pulls/104645
Thomas Barlow requested review from Bastien Montagne 2023-05-29 05:45:28 +02:00
Thomas Barlow force-pushed attribute_access_corner_edge_v_e_indices_pr from a662ebc9b1 to 769d88ffd6 2023-06-26 05:01:43 +02:00 Compare
Thomas Barlow force-pushed attribute_access_corner_edge_v_e_indices_pr from 769d88ffd6 to 56c729f6f5 2023-06-26 05:10:04 +02:00 Compare
Thomas Barlow commented 2023-06-26 05:25:54 +02:00
Author
Member
Copy Link

Additional changes have been made to fix the arrays being passed to Blender with the wrong dtypes when importing. I force-pushed these changes separately for easier comparison.

With ".edge_verts" it was a mistake on my part and with ".corner_vert" it was unexpected behaviour of numpy.astype() which has been worked around by modifying the astype_view_signedness() utility function.

Additional changes have been made to fix the arrays being passed to Blender with the wrong dtypes when importing. I force-pushed these changes separately for easier comparison. With `".edge_verts"` it was a mistake on my part and with `".corner_vert"` it was unexpected behaviour of `numpy.astype()` which has been worked around by modifying the `astype_view_signedness()` utility function.
Bastien Montagne force-pushed attribute_access_corner_edge_v_e_indices_pr from 56c729f6f5 to ec702e8c19 2023-06-26 15:31:29 +02:00 Compare
Bastien Montagne reviewed 2023-06-26 16:01:07 +02:00
io_scene_fbx/fbx_utils.py
@ -419,0 +418,4 @@
Safely views arr as new_dtype if both arr and new_dtype have the same itemsize, byteorder and signedness, but could
have a different character code, e.g. 'i' and 'l'. np.ndarray.astype with copy=False does not normally create this
view, but Blender can be picky about the character code used, so this function will create the view.
Bastien Montagne commented 2023-06-26 16:00:56 +02:00
Owner
Copy Link

I do not understand that comment, besides the i vs. u special case, this function just returns the result of np.ndarray.astype. So how can it force described behavior regarding cases like i vs. l, if np.ndarray.astype does not handle it?

I do not understand that comment, besides the `i` vs. `u` special case, this function just returns the result of `np.ndarray.astype`. So how can it force described behavior regarding cases like `i` vs. `l`, if `np.ndarray.astype` does not handle it?
Thomas Barlow commented 2023-06-26 16:52:38 +02:00
Author
Member
Copy Link

Before this patch, the explicit creation of a view only occurred when arr and new_dtype were both integer types, but had opposite signedness (and had the same itemsize and byteorder).

This patch changes the explicit creation of a view to no longer require that both inputs have opposite signedness, just that both inputs are either signed or unsigned integers (and have the same itemsize and byteorder).

The dtype 'kind' is not the character code that describes the buffer data, it is a NumPy specific character that describes the general kind of data in the buffer. In this case, the two kinds are 'i' signed integer (of any size) and 'u' unsigned integer (of any size). https://numpy.org/doc/stable/reference/generated/numpy.dtype.kind.html

On a system like mine where the C long and int have the same itemsize, NumPy creates long arrays when I use the np.int32 type. Using the np.intc type instead will force the creation of an int array which is what foreach_get/set are expecting. But, because these two types only differ by their i and l character codes, np.ndarray.astype appears to consider the two types equal and therefore simply returns the array itself when using copy=False instead of creating a view with the character code of the new_dtype e.g. long_array.astype(np.intc, copy=False) is long_array is True whereas it would have been False if a np.intc view was created.

Confusingly, np.int32 == np.intc is False, but np.dtype(np.int32) == np.dtype(np.intc) is True, I don't know if this is intended behaviour of NumPy or a bug.

Eventually I aim to do some work on Blender's buffer support in foreach_get/set and other areas of the Python API so that Blender stops caring about specific character codes so long as the 'kind' of data, the itemsize and byteorder are correct, but for now, the character code of the array passed to foreach_get/set is important for whether Blender uses the array as a buffer (fast) or a sequence (slow).

Edit: In this case, the long arrays are coming from the parsed .fbx because data_types.ARRAY_INT32 ends up being l because it finds the character code for a 32-bit integer by iterating through 'ilq'. If both i and l are 32-bit, then l is used because it is iterated last.

Before this patch, the explicit creation of a view only occurred when `arr` and `new_dtype` were both integer types, but had opposite signedness (and had the same itemsize and byteorder). This patch changes the explicit creation of a view to no longer require that both inputs have opposite signedness, just that both inputs are either signed or unsigned integers (and have the same itemsize and byteorder). The dtype 'kind' is not the character code that describes the buffer data, it is a NumPy specific character that describes the general kind of data in the buffer. In this case, the two kinds are `'i'` signed integer (of any size) and `'u'` unsigned integer (of any size). https://numpy.org/doc/stable/reference/generated/numpy.dtype.kind.html On a system like mine where the C `long` and `int` have the same itemsize, NumPy creates `long` arrays when I use the `np.int32` type. Using the `np.intc` type instead will force the creation of an `int` array which is what `foreach_get/set` are expecting. But, because these two types only differ by their `i` and `l` character codes, `np.ndarray.astype` appears to consider the two types equal and therefore simply returns the array itself when using `copy=False` instead of creating a view with the character code of the `new_dtype` e.g. `long_array.astype(np.intc, copy=False) is long_array` is `True` whereas it would have been `False` if a `np.intc` view was created. Confusingly, `np.int32 == np.intc` is `False`, but `np.dtype(np.int32) == np.dtype(np.intc)` is `True`, I don't know if this is intended behaviour of NumPy or a bug. Eventually I aim to do some work on Blender's buffer support in `foreach_get/set` and other areas of the Python API so that Blender stops caring about specific character codes so long as the 'kind' of data, the itemsize and byteorder are correct, but for now, the character code of the array passed to `foreach_get/set` is important for whether Blender uses the array as a buffer (fast) or a sequence (slow). Edit: In this case, the `long` arrays are coming from the parsed .fbx because `data_types.ARRAY_INT32` ends up being `l` because it finds the character code for a 32-bit integer by iterating through `'ilq'`. If both `i` and `l` are 32-bit, then `l` is used because it is iterated last.
Bastien Montagne commented 2023-06-26 17:42:13 +02:00
Owner
Copy Link

Thanks, I understand better now!

Thanks, I understand better now!
Bastien Montagne approved these changes 2023-06-26 17:42:17 +02:00
Bastien Montagne merged commit d17ed07832 into main 2023-06-26 17:43:25 +02:00
Bastien Montagne deleted branch attribute_access_corner_edge_v_e_indices_pr 2023-06-26 17:43:25 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
Bastien Montagne
Labels
Clear labels   
Interest
Animation & Rigging

  
Interest
Blender Cloud

  
Interest
Collada

  
Interest
Core

  
Interest
Documentation

  
Interest
Eevee & Viewport

  
Interest
Geometry Nodes

  
Interest
Grease Pencil

  
Interest
Import and Export

  
Interest
Modeling

  
Interest
Modifiers

  
Interest
Nodes & Physics

  
Interest
Pipeline, Assets & IO

  
Interest
Platforms, Builds, Tests & Devices

  
Interest
Python API

  
Interest
Rendering & Cycles

  
Interest
Sculpt, Paint & Texture

  
Interest
Translations

  
Interest
User Interface

  
Interest
UV Editing

  
Interest
VFX & Video

  
Meta
Good First Issue

  
Meta
Papercut

  
Module
Add-ons (BF-Blender)

  
Module
Add-ons (Community)

  
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
Animation & Rigging
Interest
Blender Cloud
Interest
Collada
Interest
Core
Interest
Documentation
Interest
Eevee & Viewport
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
Import and Export
Interest
Modeling
Interest
Modifiers
Interest
Nodes & Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds, Tests & Devices
Interest
Python API
Interest
Rendering & Cycles
Interest
Sculpt, Paint & Texture
Interest
Translations
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Meta
Good First Issue
Meta
Papercut
Module
Add-ons (BF-Blender)
Module
Add-ons (Community)
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
2 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-addons#104648
No description provided.
Delete Branch "Mysteryem/blender-addons:attribute_access_corner_edge_v_e_indices_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?