Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Devtoolset-9 fixes #841

Merged
merged 2 commits into from
Oct 20, 2020
Merged

Devtoolset-9 fixes #841

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6449dd0
Devtoolset-9 fixes
JGamache-autodesk Oct 16, 2020
cdb5241
Use C++ instead of casting to void*
JGamache-autodesk Oct 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/mayaUsd/fileio/utils/meshWriteUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@ UsdMayaMeshWriteUtils::exportReferenceMesh(UsdGeomMesh& primSchema, MObject obj)
const int numVertices = referenceMesh.numVertices();
VtVec3fArray points(numVertices);

memcpy(points.data(), mayaRawPoints, numVertices * sizeof(float) * 3);
memcpy((void*)points.data(), mayaRawPoints, numVertices * sizeof(float) * 3);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It often makes me a little uneasy to see void* casts, so this feels a little bit dirty to me. Seems like the compiler would be able to produce better code with more type information rather than less.

points.data() will yield a GfVec3f*, and you could then use another data() call on that to get a float*. I think I'd also use std::copy instead of memcpy and let the compiler figure out the most efficient implementation.

So with that, I'd suggest something like this:

std::copy(mayaRawPoints, mayaRawPoints + numVertices * GfVec3f::dimension, points.data()->data());

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using cdata() on the std::vector turns it into a GfVec3f C pointer, then using ->data would turn it into a full C float pointer. I do not see how the C++ compiler could optimize std::copy given raw C pointers.
Can we meet in the middle? By reshaping the source raw float* into a GfVec3f pointer, then we can use an optimized C++ constructor and skip the zero-fill we had previously.


UsdGeomPrimvar primVar = primSchema.CreatePrimvar(
UsdUtilsGetPrefName(),
Expand Down Expand Up @@ -687,7 +687,7 @@ UsdMayaMeshWriteUtils::writePointsData(const MFnMesh& meshFn,
}

// use memcpy() to copy the data. HS April 09, 2020
memcpy((GfVec3f*)points.data(), pointsData, sizeof(float) * 3 * numVertices);
memcpy((void*)points.data(), pointsData, sizeof(float) * 3 * numVertices);

VtVec3fArray extent(2);
// Compute the extent using the raw points
Expand Down
4 changes: 2 additions & 2 deletions plugin/al/plugin/AL_USDMayaTestPlugin/test_translators_NurbsCurveTranslator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,15 @@ MObject createNurbStage(bool useSingleWidth=false)
VtVec3fArray points = VtVec3fArray(5);
for(uint32_t i = 0 ; i< pointsa.size(); ++i)
{
memcpy(&points[i], &pointsa[i], 3*sizeof(float));
memcpy((void*)&points[i], &pointsa[i], 3*sizeof(float));
Copy link
Contributor

@mattyjams mattyjams Oct 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same suggestion as above, but here I think I'd also remove the loops and include the array size in the range being copied?

I'm not really familiar with this code, so I'm not sure why we're initializing the values in one structure and then copying them into another. This is also just test code though, so maybe we don't care all that much.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think we can skip the intermediates and the copy as well.

}

std::vector<std::array<double,2> > rangesa = {{0.,2.}};
VtVec2dArray ranges = VtVec2dArray(2);

for(uint32_t i = 0 ; i< rangesa.size(); ++i)
{
memcpy(&ranges[i], &rangesa[i], 2*sizeof(double));
memcpy((void*)&ranges[i], &rangesa[i], 2*sizeof(double));
}

if(useSingleWidth)
Expand Down
2 changes: 1 addition & 1 deletion plugin/al/usdmayautils/AL/usdmaya/utils/DgNodeHelper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,7 @@ MStatus DgNodeHelper::setMatrix4x4Array(MObject node, MObject attribute, const d
MStatus status;
MMatrixArray arrayData;
arrayData.setLength(count);
memcpy(&arrayData[0], values, sizeof(MMatrix) * count);
memcpy((void*)&arrayData[0], values, sizeof(MMatrix) * count);

MFnMatrixArrayData fn;
MObject data = fn.create(arrayData, &status);
Expand Down
2 changes: 1 addition & 1 deletion plugin/al/usdmayautils/AL/usdmaya/utils/DiffPrimVar.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ uint32_t diffGeom(UsdGeomPointBased& geom, MFnMesh& mesh, UsdTimeCode timeCode,
{
const uint32_t numVertices = mesh.numVertices();
VtArray<GfVec3f> points(numVertices);
memcpy((GfVec3f*)points.data(), pointsData, sizeof(float) * 3 * numVertices);
memcpy((void*)points.data(), pointsData, sizeof(float) * 3 * numVertices);

VtArray<GfVec3f> mayaExtent(2);
UsdGeomPointBased::ComputeExtent(points, &mayaExtent);
Expand Down
14 changes: 7 additions & 7 deletions plugin/al/usdmayautils/AL/usdmaya/utils/MeshUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -844,7 +844,7 @@ void MeshImportContext::applyColourSetData()
{
const VtArray<GfVec4f> rawVal = vtValue.UncheckedGet<VtArray<GfVec4f> >();
colours.setLength(rawVal.size());
memcpy(&colours[0], (const float*) rawVal.cdata(), sizeof(float) * 4 * rawVal.size());
memcpy((void*)&colours[0], (const float*) rawVal.cdata(), sizeof(float) * 4 * rawVal.size());
representation = MFnMesh::kRGBA;
}

Expand Down Expand Up @@ -1990,7 +1990,7 @@ void MeshExportContext::copyVertexData(UsdTimeCode time)
const float* pointsData = fnMesh.getRawPoints(&status);
if(status)
{
memcpy((GfVec3f*)points.data(), pointsData, sizeof(float) * 3 * numVertices);
memcpy((void*)points.data(), pointsData, sizeof(float) * 3 * numVertices);

pointsAttr.Set(points, time);
}
Expand All @@ -2015,7 +2015,7 @@ void MeshExportContext::copyExtentData(UsdTimeCode time)
{
const uint32_t numVertices = fnMesh.numVertices();
VtArray<GfVec3f> points(numVertices);
memcpy((GfVec3f*)points.data(), pointsData, sizeof(float) * 3 * numVertices);
memcpy((void*)points.data(), pointsData, sizeof(float) * 3 * numVertices);

VtArray<GfVec3f> extent(2);
UsdGeomPointBased::ComputeExtent(points, &extent);
Expand Down Expand Up @@ -2048,7 +2048,7 @@ void MeshExportContext::copyBindPoseData(UsdTimeCode time)
const float* pointsData = fnMesh.getRawPoints(&status);
if(status)
{
memcpy((GfVec3f*)points.data(), pointsData, sizeof(float) * 3 * numVertices);
memcpy((void*)points.data(), pointsData, sizeof(float) * 3 * numVertices);

pRefPrimVarAttr.Set(points, time);
}
Expand Down Expand Up @@ -2121,7 +2121,7 @@ void MeshExportContext::copyNormalData(UsdTimeCode time, bool copyAsPrimvar)
mesh.SetNormalsInterpolation(UsdGeomTokens->vertex);
}

memcpy((GfVec3f*)normals.data(), normalsData, sizeof(float) * 3 * numNormals);
memcpy((void*)normals.data(), normalsData, sizeof(float) * 3 * numNormals);
normalsAttr.Set(normals, time);
}
else
Expand All @@ -2148,7 +2148,7 @@ void MeshExportContext::copyNormalData(UsdTimeCode time, bool copyAsPrimvar)
if(isPerVertex)
{
VtArray<GfVec3f> normals(numNormals);
memcpy((GfVec3f*)normals.data(), normalsData, sizeof(float) * 3 * numNormals);
memcpy((void*)normals.data(), normalsData, sizeof(float) * 3 * numNormals);
for(auto& c : missing)
{
const uint32_t orig = c.first;
Expand Down Expand Up @@ -2220,7 +2220,7 @@ void MeshExportContext::copyNormalData(UsdTimeCode time, bool copyAsPrimvar)
mesh.SetNormalsInterpolation(UsdGeomTokens->faceVarying);
}
VtArray<GfVec3f> normals(numNormals);
memcpy((GfVec3f*)normals.data(), normalsData, sizeof(float) * 3 * numNormals);
memcpy((void*)normals.data(), normalsData, sizeof(float) * 3 * numNormals);
normalsAttr.Set(normals, time);
}
else
Expand Down