blender
/
blender
136
407
Fork 589
Code Issues 6.3k Pull Requests 780 Projects 19 Wiki Activity

Refactor: in USD importer extract read_mesh params into a struct #104459

Closed
Sonny Campbell wants to merge 11 commits from SonnyCampbell_Unity/blender:unity/T91369-parameter-refactor into main
pull from: SonnyCampbell_Unity/blender:unity/T91369-parameter-refactor
merge into: blender:main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
Conversation 6 Commits 11 Files Changed 12 +6 -6
5 changed files with 6 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 409ee2669b - Show all commits

4
source/blender/io/usd/intern/usd_capi_import.cc
View File

@ -465,7 +465,7 @@ static USDPrimReader *get_usd_reader(CacheReader *reader,
return usd_reader;
}
USDMeshReadParams create_mesh_read_params(double motion_sample_time, int read_flags)
USDMeshReadParams create_mesh_read_params(const double motion_sample_time, const int read_flags)
SonnyCampbell_Unity marked this conversation as resolved
Sybren A. Stüvel commented 2023-02-13 11:52:53 +01:00
Review
Copy Link

const for both parameters.

`const` for both parameters.
{
USDMeshReadParams params = {};
params.motion_sample_time = motion_sample_time;
@ -476,7 +476,7 @@ USDMeshReadParams create_mesh_read_params(double motion_sample_time, int read_fl
struct Mesh *USD_read_mesh(struct CacheReader *reader,
struct Object *ob,
struct Mesh *existing_mesh,
USDMeshReadParams params,
const USDMeshReadParams params,
SonnyCampbell_Unity marked this conversation as resolved
Sybren A. Stüvel commented 2023-02-13 11:53:14 +01:00
Review
Copy Link

const

`const`
const char **err_str)
{
USDGeomReader *usd_reader = dynamic_cast<USDGeomReader *>(get_usd_reader(reader, ob, err_str));

2
source/blender/io/usd/intern/usd_reader_curve.cc
View File

@ -162,7 +162,7 @@ void USDCurvesReader::read_curve_sample(Curve *cu, const double motionSampleTime
}
Mesh *USDCurvesReader::read_mesh(struct Mesh *existing_mesh,
USDMeshReadParams params,
const USDMeshReadParams params,
const char ** /* err_str */)
{
if (!curve_prim_) {

2
source/blender/io/usd/intern/usd_reader_mesh.cc
View File

@ -818,7 +818,7 @@ void USDMeshReader::readFaceSetsSample(Main *bmain, Mesh *mesh, const double mot
}
Mesh *USDMeshReader::read_mesh(Mesh *existing_mesh,
USDMeshReadParams params,
const USDMeshReadParams params,
const char ** /* err_str */)
{
if (!mesh_prim_) {

2
source/blender/io/usd/intern/usd_reader_nurbs.cc
View File

@ -165,7 +165,7 @@ void USDNurbsReader::read_curve_sample(Curve *cu, const double motionSampleTime)
}
Mesh *USDNurbsReader::read_mesh(struct Mesh * /* existing_mesh */,
USDMeshReadParams params,
const USDMeshReadParams params,
const char ** /* err_str */)
{
pxr::UsdGeomCurves curve_prim_(prim_);

2
source/blender/io/usd/usd.h
View File

@ -70,7 +70,7 @@ struct USDImportParams {
* usd file for the mesh sequence cache.
*/
typedef struct USDMeshReadParams {
double motion_sample_time; /* Read USD TimeCode in frames. */
double motion_sample_time; /* USD TimeCode in frames. */
SonnyCampbell_Unity marked this conversation as resolved
Sybren A. Stüvel commented 2023-02-10 12:31:06 +01:00
Review
Copy Link

"Read" is ambiguous here. Is it past or future tense?

"Read" is ambiguous here. Is it past or future tense?
Sonny Campbell commented 2023-02-10 12:57:36 +01:00
Review
Copy Link

It's neither. It's read as opposed to write. It's usage is everywhere as a parameter in a read_mesh function, so it's very clear what it means from the context.

It's neither. It's read as opposed to write. It's usage is everywhere as a parameter in a `read_mesh` function, so it's very clear what it means from the context.
Sonny Campbell commented 2023-02-10 12:59:07 +01:00
Review
Copy Link

It's also consistent with ABCReadParams for ABC_read_mesh in the alembic import code

It's also consistent with `ABCReadParams` for `ABC_read_mesh` in the alembic import code
Sybren A. Stüvel commented 2023-02-13 11:56:52 +01:00
Review
Copy Link

In that case I would just change the comment to "USD TimeCode in frames", as it then limits itself to just describing the units of measurement, and not the operation that it's involved with. The fact that it's used for reading is clear from the name of the struct.

In that case I would just change the comment to "USD TimeCode in frames", as it then limits itself to just describing the units of measurement, and not the operation that it's involved with. The fact that it's used for reading is clear from the name of the struct.
Sonny Campbell commented 2023-02-13 12:46:14 +01:00
Review
Copy Link

Done

Done
int read_flags; /* MOD_MESHSEQ_xxx value that is set from MeshSeqCacheModifierData.read_flag. */
} USDMeshReadParams;