From b1933194aa819b768030d29a2e07cbab8e4be1a5 Mon Sep 17 00:00:00 2001 From: manuelk Date: Wed, 11 Feb 2015 14:25:44 -0800 Subject: [PATCH] Fix data race on static global variable in Far::PatchDescriptor - statically initialize valid patch descriptor tables - return a Vtr::Array instead of std::vector - fix dependencies in Far::PatchTables fixes #394 --- opensubdiv/far/patchDescriptor.cpp | 164 ++++++++++++++++---------- opensubdiv/far/patchDescriptor.h | 5 +- opensubdiv/far/patchTablesFactory.cpp | 27 +++-- 3 files changed, 120 insertions(+), 76 deletions(-) diff --git a/opensubdiv/far/patchDescriptor.cpp b/opensubdiv/far/patchDescriptor.cpp index cf45b9c3..ffa4865e 100644 --- a/opensubdiv/far/patchDescriptor.cpp +++ b/opensubdiv/far/patchDescriptor.cpp @@ -32,77 +32,119 @@ namespace OPENSUBDIV_VERSION { namespace Far { + // -// Lists of patch Descriptors for each subdivision scheme +// Lists of valid patch Descriptors for each subdivision scheme // -static PatchDescriptorVector const & -getAdaptiveCatmarkDescriptors() { - - static PatchDescriptorVector _descriptors; - - if (_descriptors.empty()) { - - _descriptors.reserve(72); - - // non-transition patches : 7 - for (int i=PatchDescriptor::REGULAR; - i<=PatchDescriptor::GREGORY_BASIS; ++i) { - - _descriptors.push_back( - PatchDescriptor(i, PatchDescriptor::NON_TRANSITION, 0)); - } - - // transition patches (1 + 4 * 3) * 5 = 65 - for (int i=PatchDescriptor::PATTERN0; i<=PatchDescriptor::PATTERN4; ++i) { - - _descriptors.push_back( - PatchDescriptor(PatchDescriptor::REGULAR, i, 0) ); - - // 4 rotations for single-crease, boundary and corner patches - for (int j=0; j<4; ++j) { - _descriptors.push_back( - PatchDescriptor(PatchDescriptor::SINGLE_CREASE, i, j)); - } - - for (int j=0; j<4; ++j) { - _descriptors.push_back( - PatchDescriptor(PatchDescriptor::BOUNDARY, i, j)); - } - - for (int j=0; j<4; ++j) { - _descriptors.push_back( - PatchDescriptor(PatchDescriptor::CORNER, i, j)); - } - } - } - return _descriptors; -} -static PatchDescriptorVector const & -getAdaptiveLoopDescriptors() { - - static PatchDescriptorVector _descriptors; - - if (_descriptors.empty()) { - _descriptors.reserve(1); - _descriptors.push_back( - PatchDescriptor(PatchDescriptor::LOOP, PatchDescriptor::NON_TRANSITION, 0) ); - } - return _descriptors; -} -PatchDescriptorVector const & +ConstPatchDescriptorArray PatchDescriptor::GetAdaptivePatchDescriptors(Sdc::SchemeType type) { - static PatchDescriptorVector _empty; + static PatchDescriptor _loopDescriptors[] = { + // XXXX work in progress ! + PatchDescriptor(LOOP, NON_TRANSITION, 0) + }; + + static PatchDescriptor _catmarkDescriptors[] = { + + // non-transition patches : 7 + PatchDescriptor(REGULAR, NON_TRANSITION, 0), + PatchDescriptor(SINGLE_CREASE, NON_TRANSITION, 0), + PatchDescriptor(BOUNDARY, NON_TRANSITION, 0), + PatchDescriptor(CORNER, NON_TRANSITION, 0), + PatchDescriptor(GREGORY, NON_TRANSITION, 0), + PatchDescriptor(GREGORY_BOUNDARY, NON_TRANSITION, 0), + PatchDescriptor(GREGORY_BASIS, NON_TRANSITION, 0), + + // transition pattern 0 + PatchDescriptor(REGULAR, PATTERN0, 0), + PatchDescriptor(SINGLE_CREASE, PATTERN0, 0), + PatchDescriptor(SINGLE_CREASE, PATTERN0, 1), + PatchDescriptor(SINGLE_CREASE, PATTERN0, 2), + PatchDescriptor(SINGLE_CREASE, PATTERN0, 3), + PatchDescriptor(BOUNDARY, PATTERN0, 0), + PatchDescriptor(BOUNDARY, PATTERN0, 1), + PatchDescriptor(BOUNDARY, PATTERN0, 2), + PatchDescriptor(BOUNDARY, PATTERN0, 3), + PatchDescriptor(CORNER, PATTERN0, 0), + PatchDescriptor(CORNER, PATTERN0, 1), + PatchDescriptor(CORNER, PATTERN0, 2), + PatchDescriptor(CORNER, PATTERN0, 3), + + // transition pattern 1 + PatchDescriptor(REGULAR, PATTERN1, 0), + PatchDescriptor(SINGLE_CREASE, PATTERN1, 0), + PatchDescriptor(SINGLE_CREASE, PATTERN1, 1), + PatchDescriptor(SINGLE_CREASE, PATTERN1, 2), + PatchDescriptor(SINGLE_CREASE, PATTERN1, 3), + PatchDescriptor(BOUNDARY, PATTERN1, 0), + PatchDescriptor(BOUNDARY, PATTERN1, 1), + PatchDescriptor(BOUNDARY, PATTERN1, 2), + PatchDescriptor(BOUNDARY, PATTERN1, 3), + PatchDescriptor(CORNER, PATTERN1, 0), + PatchDescriptor(CORNER, PATTERN1, 1), + PatchDescriptor(CORNER, PATTERN1, 2), + PatchDescriptor(CORNER, PATTERN1, 3), + + // transition pattern 2 + PatchDescriptor(REGULAR, PATTERN2, 0), + PatchDescriptor(SINGLE_CREASE, PATTERN2, 0), + PatchDescriptor(SINGLE_CREASE, PATTERN2, 1), + PatchDescriptor(SINGLE_CREASE, PATTERN2, 2), + PatchDescriptor(SINGLE_CREASE, PATTERN2, 3), + PatchDescriptor(BOUNDARY, PATTERN2, 0), + PatchDescriptor(BOUNDARY, PATTERN2, 1), + PatchDescriptor(BOUNDARY, PATTERN2, 2), + PatchDescriptor(BOUNDARY, PATTERN2, 3), + PatchDescriptor(CORNER, PATTERN2, 0), + PatchDescriptor(CORNER, PATTERN2, 1), + PatchDescriptor(CORNER, PATTERN2, 2), + PatchDescriptor(CORNER, PATTERN2, 3), + + // transition pattern 3 + PatchDescriptor(REGULAR, PATTERN3, 0), + PatchDescriptor(SINGLE_CREASE, PATTERN3, 0), + PatchDescriptor(SINGLE_CREASE, PATTERN3, 1), + PatchDescriptor(SINGLE_CREASE, PATTERN3, 2), + PatchDescriptor(SINGLE_CREASE, PATTERN3, 3), + PatchDescriptor(BOUNDARY, PATTERN3, 0), + PatchDescriptor(BOUNDARY, PATTERN3, 1), + PatchDescriptor(BOUNDARY, PATTERN3, 2), + PatchDescriptor(BOUNDARY, PATTERN3, 3), + PatchDescriptor(CORNER, PATTERN3, 0), + PatchDescriptor(CORNER, PATTERN3, 1), + PatchDescriptor(CORNER, PATTERN3, 2), + PatchDescriptor(CORNER, PATTERN3, 3), + + // transition pattern 4 + PatchDescriptor(REGULAR, PATTERN4, 0), + PatchDescriptor(SINGLE_CREASE, PATTERN4, 0), + PatchDescriptor(SINGLE_CREASE, PATTERN4, 1), + PatchDescriptor(SINGLE_CREASE, PATTERN4, 2), + PatchDescriptor(SINGLE_CREASE, PATTERN4, 3), + PatchDescriptor(BOUNDARY, PATTERN4, 0), + PatchDescriptor(BOUNDARY, PATTERN4, 1), + PatchDescriptor(BOUNDARY, PATTERN4, 2), + PatchDescriptor(BOUNDARY, PATTERN4, 3), + PatchDescriptor(CORNER, PATTERN4, 0), + PatchDescriptor(CORNER, PATTERN4, 1), + PatchDescriptor(CORNER, PATTERN4, 2), + PatchDescriptor(CORNER, PATTERN4, 3), + }; switch (type) { - case Sdc::SCHEME_BILINEAR : return _empty; - case Sdc::SCHEME_CATMARK : return getAdaptiveCatmarkDescriptors(); - case Sdc::SCHEME_LOOP : return getAdaptiveLoopDescriptors(); + case Sdc::SCHEME_BILINEAR : + return ConstPatchDescriptorArray(0, 0); + case Sdc::SCHEME_CATMARK : + return ConstPatchDescriptorArray(_catmarkDescriptors, + (int)(sizeof(_catmarkDescriptors)/sizeof(PatchDescriptor))); + case Sdc::SCHEME_LOOP : + return ConstPatchDescriptorArray(_loopDescriptors, + (int)(sizeof(_loopDescriptors)/sizeof(PatchDescriptor))); default: assert(0); } - return _empty; + return ConstPatchDescriptorArray(0, 0);; } diff --git a/opensubdiv/far/patchDescriptor.h b/opensubdiv/far/patchDescriptor.h index e61ba454..4d6be489 100644 --- a/opensubdiv/far/patchDescriptor.h +++ b/opensubdiv/far/patchDescriptor.h @@ -27,6 +27,7 @@ #include "../version.h" +#include "../far/types.h" #include "../sdc/types.h" #include @@ -154,7 +155,7 @@ public: /// \brief Returns a vector of all the legal patch descriptors for the /// given adaptive subdivision scheme - static std::vector const & GetAdaptivePatchDescriptors(Sdc::SchemeType type); + static Vtr::ConstArray GetAdaptivePatchDescriptors(Sdc::SchemeType type); /// \brief Allows ordering of patches by type inline bool operator < ( PatchDescriptor const other ) const; @@ -170,7 +171,7 @@ private: unsigned int _rotation:2; }; -typedef std::vector PatchDescriptorVector; +typedef Vtr::ConstArray ConstPatchDescriptorArray; // Returns the number of control vertices expected for a patch of this type inline short diff --git a/opensubdiv/far/patchTablesFactory.cpp b/opensubdiv/far/patchTablesFactory.cpp index a202411d..b128c476 100644 --- a/opensubdiv/far/patchTablesFactory.cpp +++ b/opensubdiv/far/patchTablesFactory.cpp @@ -850,13 +850,13 @@ PatchTablesFactory::createAdaptive( TopologyRefiner const & refiner, Options opt tables->reservePatchArrays(patchInventory.getNumPatchArrays()); // sort through the inventory and push back non-empty patch arrays - typedef PatchDescriptorVector DescVec; - - DescVec const & descs = PatchDescriptor::GetAdaptivePatchDescriptors(Sdc::SCHEME_CATMARK); + ConstPatchDescriptorArray const & descs = + PatchDescriptor::GetAdaptivePatchDescriptors(Sdc::SCHEME_CATMARK); int voffset=0, poffset=0, qoffset=0; - for (DescVec::const_iterator it=descs.begin(); it!=descs.end(); ++it) { - tables->pushPatchArray(*it, patchInventory.getValue(*it), &voffset, &poffset, &qoffset ); + for (int i=0; ipushPatchArray(desc, patchInventory.getValue(desc), &voffset, &poffset, &qoffset ); } tables->_numPtexFaces = refiner.GetNumPtexFaces(); @@ -1134,22 +1134,23 @@ PatchTablesFactory::populateAdaptivePatches( TopologyRefiner const & refiner, PatchFVarPointers fptrs; SharpnessIndexPointers sptrs; - typedef PatchDescriptorVector DescVec; + ConstPatchDescriptorArray const & descs = + PatchDescriptor::GetAdaptivePatchDescriptors(Sdc::SCHEME_CATMARK); - DescVec const & descs = PatchDescriptor::GetAdaptivePatchDescriptors(Sdc::SCHEME_CATMARK); + for (int i=0; ifindPatchArray(*it); + Index arrayIndex = tables->findPatchArray(desc); if (arrayIndex==Vtr::INDEX_INVALID) { continue; } - iptrs.getValue( *it ) = tables->getPatchArrayVertices(arrayIndex).begin(); - pptrs.getValue( *it ) = tables->getPatchParams(arrayIndex).begin(); + iptrs.getValue(desc) = tables->getPatchArrayVertices(arrayIndex).begin(); + pptrs.getValue(desc) = tables->getPatchParams(arrayIndex).begin(); if (patchInventory.hasSingleCreasedPatches()) { - sptrs.getValue( *it ) = tables->getSharpnessIndices(arrayIndex); + sptrs.getValue(desc) = tables->getSharpnessIndices(arrayIndex); } if (tables->_fvarPatchTables) { @@ -1161,7 +1162,7 @@ PatchTablesFactory::populateAdaptivePatches( TopologyRefiner const & refiner, fptr[channel] = tables->getFVarVerts(arrayIndex, channel).begin(); } - fptrs.getValue( *it ) = fptr; + fptrs.getValue(desc) = fptr; } }