Page MenuHome
Differential D9057 Diff 29460 source/blender/io/alembic/intern/abc_reader_mesh.cc

Changeset View

Standalone View

View Options

source/blender/io/alembic/intern/abc_reader_mesh.cc

Show First 20 Lines • Show All 41 Lines • ▼ Show 20 Lines
#include "BKE_object.h" #include "BKE_object.h"
using Alembic::Abc::Int32ArraySamplePtr; using Alembic::Abc::Int32ArraySamplePtr;
using Alembic::Abc::P3fArraySamplePtr; using Alembic::Abc::P3fArraySamplePtr;
using Alembic::AbcGeom::IFaceSet; using Alembic::AbcGeom::IFaceSet;
using Alembic::AbcGeom::IFaceSetSchema; using Alembic::AbcGeom::IFaceSetSchema;
using Alembic::AbcGeom::IN3fGeomParam; using Alembic::AbcGeom::IN3fGeomParam;
using Alembic::AbcGeom::IC3fGeomParam;
using Alembic::AbcGeom::IC4fGeomParam;
using Alembic::AbcGeom::IObject; using Alembic::AbcGeom::IObject;
using Alembic::AbcGeom::IPolyMesh; using Alembic::AbcGeom::IPolyMesh;
using Alembic::AbcGeom::IPolyMeshSchema; using Alembic::AbcGeom::IPolyMeshSchema;
using Alembic::AbcGeom::ISampleSelector; using Alembic::AbcGeom::ISampleSelector;
using Alembic::AbcGeom::ISubD; using Alembic::AbcGeom::ISubD;
using Alembic::AbcGeom::ISubDSchema; using Alembic::AbcGeom::ISubDSchema;
using Alembic::AbcGeom::IV2fGeomParam; using Alembic::AbcGeom::IV2fGeomParam;
using Alembic::AbcGeom::kWrapExisting; using Alembic::AbcGeom::kWrapExisting;
▲ Show 20 Lines • Show All 434 Lines • ▼ Show 20 Lines
bool AbcMeshReader::valid() const bool AbcMeshReader::valid() const
{ {
return m_schema.valid(); return m_schema.valid();
} }
/* Specialisation of has_animations() as defined in abc_reader_object.h. */ /* Specialisation of has_animations() as defined in abc_reader_object.h. */
template<> bool has_animations(Alembic::AbcGeom::IPolyMeshSchema &schema, ImportSettings *settings) template<> bool has_animations(Alembic::AbcGeom::IPolyMeshSchema &schema, ImportSettings *settings)
{ {
if (settings->is_sequence || !schema.isConstant()) { if (settings->is_sequence || !schema.isConstant()) {
return true; return true;
sybrenUnsubmitted
Done
Inline Actions

Given the parameters, it's no longer given that this is related to colour at all. Probably it's better to rename color_param to geom_param.

sybren: Given the parameters, it's no longer given that this is related to colour at all. Probably it's…
} }
IV2fGeomParam uvsParam = schema.getUVsParam(); IV2fGeomParam uvsParam = schema.getUVsParam();
if (uvsParam.valid() && !uvsParam.isConstant()) {
return true;
}
sybrenUnsubmitted
Done
Inline Actions

Here you can flip the conditions and return early.

if (!typedGeomParam::matches(prop_header)) {
  return false;
}

typedGeomParam color_param(arbGeomParams, prop_header.getName());
return color_param.valid() && !color_param.isConstant();
sybren: Here you can flip the conditions and return early. ``` if (!typedGeomParam::matches…
IN3fGeomParam normalsParam = schema.getNormalsParam(); IN3fGeomParam normalsParam = schema.getNormalsParam();
return (uvsParam.valid() && !uvsParam.isConstant()) || if (normalsParam.valid() && !normalsParam.isConstant()) {
sybrenUnsubmitted
Done
Inline Actions

has_animated_... reads more naturally.

There is no guarantee that this function will only pick up on animated vertex colors, though. There could be other IC3fGeomParam or IC4fGeomParam that cause the function to return true. This should be reflected in the name of the function.

sybren: `has_animated_...` reads more naturally. There is no guarantee that this function will only…
(normalsParam.valid() && !normalsParam.isConstant()); return true;
}
sybrenUnsubmitted
Done
Inline Actions

I don't see how an ICompoundProperty can be nullptr. Maybe you mean to call .valid()?

sybren: I don't see how an `ICompoundProperty` can be `nullptr`. Maybe you mean to call `.valid()`?
ICompoundProperty arbGeomParams = schema.getArbGeomParams();
sybrenUnsubmitted
Done
Inline Actions

Does this always return a valid ICompoundProperty?

sybren: Does this always return a valid `ICompoundProperty`?
const size_t num_props = arbGeomParams.getNumProperties();
for (size_t i = 0; i < num_props; i++) {
const Alembic::Abc::PropertyHeader &prop_header = arbGeomParams.getPropertyHeader(i);
if (IC3fGeomParam::matches(prop_header)) {
IC3fGeomParam color_param(arbGeomParams, prop_header.getName());
if (color_param.valid() && !color_param.isConstant()) {
return true;
}
sybrenUnsubmitted
Done
Inline Actions

No else after return.

sybren: No `else` after `return`.
}
else if (IC4fGeomParam::matches(prop_header)) {
IC4fGeomParam color_param(arbGeomParams, prop_header.getName());
if (color_param.valid() && !color_param.isConstant()) {
return true;
}
}
sybrenUnsubmitted
Done
Inline Actions

The code in both if block is exactly the same, except for the variable type. This can be abstracted into a templated function.

sybren: The code in both `if` block is exactly the same, except for the variable type. This can be…
}
sybrenUnsubmitted
Done
Inline Actions

This should be its own function.

sybren: This should be its own function.
return false;
} }
void AbcMeshReader::readObjectData(Main *bmain, const Alembic::Abc::ISampleSelector &sample_sel) void AbcMeshReader::readObjectData(Main *bmain, const Alembic::Abc::ISampleSelector &sample_sel)
{ {
Mesh *mesh = BKE_mesh_add(bmain, m_data_name.c_str()); Mesh *mesh = BKE_mesh_add(bmain, m_data_name.c_str());
m_object = BKE_object_add_only_object(bmain, OB_MESH, m_object_name.c_str()); m_object = BKE_object_add_only_object(bmain, OB_MESH, m_object_name.c_str());
m_object->data = mesh; m_object->data = mesh;
▲ Show 20 Lines • Show All 436 Lines • Show Last 20 Lines