[Bf-codereview] Use DerivedMesh instead of Mesh in COLLADA GeometryExporter (issue4387052)

danielmtavares at gmail.com danielmtavares at gmail.com
Thu May 5 08:37:19 CEST 2011


On 2011/05/03 07:14:52, brechtvl wrote:
> This patch is not using DerivedMesh correctly, don't know how it can
even work,
> at least uv and vertex color export would probably crash.


http://codereview.appspot.com/4387052/diff/1/blender/source/blender/collada/GeometryExporter.cpp
> File blender/source/blender/collada/GeometryExporter.cpp (right):


http://codereview.appspot.com/4387052/diff/1/blender/source/blender/collada/GeometryExporter.cpp#newcode73
> blender/source/blender/collada/GeometryExporter.cpp:73: DerivedMesh
*dm =
> mesh_create_derived_view(mScene, ob, CD_MASK_BAREMESH);
> Mask should include the layers you will use, i.e. CD_MASK_MTFACE,
CD_MASK_MCOL.
> Also, the derivedmesh is not being released at the end.


http://codereview.appspot.com/4387052/diff/1/blender/source/blender/collada/GeometryExporter.cpp#newcode89
> blender/source/blender/collada/GeometryExporter.cpp:89: CustomData
*face_data =
> static_cast<CustomData *>(dm->getFaceDataArray(dm, CD_MTFACE));
> This can't work, dm->getFaceDataArray does not return CustomData*, it
returns
> the actual layer data if it exists. dm->faceData has it.

Hi Bretch,

Thanks for reviewing the code.

Yes, the section that exports UV and vertex color is broken as of this
patch. I'm still learning the API and trying to figure how to handle
that.

What exactly is stored on the CD_MTFACE layer? Does it contains an array
of MTFaces?

I can't recall for certain, but I think the reason why I ended up
casting it to CustomData* was because I was getting errors. Also, after
reading the comments right above getFaceDataArray definition I
understood that it would return CustomData*'s. But clearly, I was
mistaken. This is what BKE_DerivedMesh.h says about getFaceDataArray:

...
	/* return a pointer to the entire array of vert/edge/face custom data
	 * from the derived mesh (this gives a pointer to the actual data, not
	 * a copy)
	 */
...


http://codereview.appspot.com/4387052/


More information about the Bf-codereview mailing list