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.

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

[usdAbc] Pass Sdf arguments to Alembic plugin. #1099

Merged
merged 6 commits into from
Mar 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 12 additions & 0 deletions pxr/usd/plugin/usdAbc/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ pxr_test_scripts(
testenv/testUsdAbcInstancing.py
testenv/testUsdAbcUvReadWrite.py
testenv/testUsdAbcUvReadWrite_OldEncoding.py
testenv/testUsdAbcSDFArguments.py
)

pxr_install_test_dir(
Expand Down Expand Up @@ -139,6 +140,11 @@ pxr_install_test_dir(
DEST testUsdAbcUvReadWrite_OldEncoding
)

pxr_install_test_dir(
SRC testenv/testUsdAbcSDFArguments
DEST testUsdAbcSDFArguments
)

pxr_register_test(testUsdAbcAlembicData
PYTHON
COMMAND "${CMAKE_INSTALL_PREFIX}/tests/testUsdAbcAlembicData"
Expand Down Expand Up @@ -242,3 +248,9 @@ pxr_register_test(testUsdAbcUvReadWrite_OldEncoding
USD_ABC_WRITE_UV_AS_ST_TEXCOORD2FARRAY=0
USD_ABC_READ_FLOAT2ARRAY_ST_PRIMVARS=1
)

pxr_register_test(testUsdAbcSDFArguments
PYTHON
COMMAND "${CMAKE_INSTALL_PREFIX}/tests/testUsdAbcSDFArguments"
EXPECTED_RETURN_CODE 0
)
10 changes: 5 additions & 5 deletions pxr/usd/plugin/usdAbc/alembicData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,9 @@ typedef std::set<double> UsdAbc_TimeSamples;

#define XXX_UNSUPPORTED(M) TF_RUNTIME_ERROR("Alembic file " #M "() not supported")

UsdAbc_AlembicData::UsdAbc_AlembicData()
UsdAbc_AlembicData::UsdAbc_AlembicData(SdfFileFormat::FileFormatArguments args)
: _arguments(std::move(args))
{
// Do nothing
}

UsdAbc_AlembicData::~UsdAbc_AlembicData()
Expand All @@ -119,9 +119,9 @@ UsdAbc_AlembicData::~UsdAbc_AlembicData()
}

UsdAbc_AlembicDataRefPtr
UsdAbc_AlembicData::New()
UsdAbc_AlembicData::New(SdfFileFormat::FileFormatArguments args)
{
return TfCreateRefPtr(new UsdAbc_AlembicData);
return TfCreateRefPtr(new UsdAbc_AlembicData(std::move(args)));
}

bool
Expand All @@ -148,7 +148,7 @@ UsdAbc_AlembicData::Open(const std::string& filePath)
//_reader->SetFlag(UsdAbc_AlembicContextFlagNames->verbose);

// Open the archive.
if (_reader->Open(filePath)) {
if (_reader->Open(filePath, _arguments)) {
return true;
}

Expand Down
6 changes: 4 additions & 2 deletions pxr/usd/plugin/usdAbc/alembicData.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

#include "pxr/pxr.h"
#include "pxr/usd/sdf/data.h"
#include "pxr/usd/sdf/fileFormat.h"
#include "pxr/base/tf/declarePtrs.h"
#include <boost/shared_ptr.hpp>

Expand All @@ -43,7 +44,7 @@ class UsdAbc_AlembicData : public SdfAbstractData {
/// Returns a new \c UsdAbc_AlembicData object. Outside a successful
/// \c Open() and \c Close() pairing, the data acts as if it contains
/// a pseudo-root prim spec at the absolute root path.
static UsdAbc_AlembicDataRefPtr New();
static UsdAbc_AlembicDataRefPtr New(SdfFileFormat::FileFormatArguments = {});

/// Opens the Alembic file at \p filePath read-only (closing any open
/// file). Alembic is not meant to be used as an in-memory store for
Expand Down Expand Up @@ -103,14 +104,15 @@ class UsdAbc_AlembicData : public SdfAbstractData {
EraseTimeSample(const SdfPath&, double);

protected:
UsdAbc_AlembicData();
UsdAbc_AlembicData(SdfFileFormat::FileFormatArguments);
virtual ~UsdAbc_AlembicData();

// SdfAbstractData overrides
virtual void _VisitSpecs(SdfAbstractDataSpecVisitor* visitor) const;

private:
boost::shared_ptr<class UsdAbc_AlembicDataReader> _reader;
const SdfFileFormat::FileFormatArguments _arguments;
};


Expand Down
2 changes: 1 addition & 1 deletion pxr/usd/plugin/usdAbc/alembicFileFormat.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ UsdAbcAlembicFileFormat::~UsdAbcAlembicFileFormat()
SdfAbstractDataRefPtr
UsdAbcAlembicFileFormat::InitData(const FileFormatArguments& args) const
{
return UsdAbc_AlembicData::New();
return UsdAbc_AlembicData::New(args);
}

bool
Expand Down
186 changes: 84 additions & 102 deletions pxr/usd/plugin/usdAbc/alembicReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,7 @@
#include <Alembic/Abc/ITypedArrayProperty.h>
#include <Alembic/Abc/ITypedScalarProperty.h>
#include <Alembic/AbcCoreAbstract/Foundation.h>

#ifdef PXR_MULTIVERSE_SUPPORT_ENABLED
#include <Alembic/AbcCoreGit/All.h>
#endif // PXR_MULTIVERSE_SUPPORT_ENABLED

#ifdef PXR_HDF5_SUPPORT_ENABLED
#include <Alembic/AbcCoreHDF5/All.h>
#endif // PXR_HDF5_SUPPORT_ENABLED

#include <Alembic/AbcCoreOgawa/All.h>
#include <Alembic/AbcCoreFactory/IFactory.h>
spiffmon marked this conversation as resolved.
Show resolved Hide resolved
#include <Alembic/AbcGeom/GeometryScope.h>
#include <Alembic/AbcGeom/ICamera.h>
#include <Alembic/AbcGeom/ICurves.h>
Expand Down Expand Up @@ -132,7 +123,7 @@ _GetNumOgawaStreams()
static_cast<int>(WorkGetConcurrencyLimit()));
}

#ifdef PXR_HDF5_SUPPORT_ENABLED
#if PXR_HDF5_SUPPORT_ENABLED && !H5_HAVE_THREADSAFE
// A global mutex until our HDF5 library is thread safe. It has to be
// recursive to handle the case where we write an Alembic file using an
// UsdAbc_AlembicData as the source.
Expand Down Expand Up @@ -701,7 +692,8 @@ class _ReaderContext {
/// @{

/// Open an archive.
bool Open(const std::string& filePath, std::string* errorLog);
bool Open(const std::string& filePath, std::string* errorLog,
const SdfFileFormat::FileFormatArguments& args);

/// Close the archive.
void Close();
Expand Down Expand Up @@ -788,14 +780,6 @@ class _ReaderContext {
typedef std::set<_ObjectPtr> _ObjectReaderSet;
typedef std::map<_ObjectPtr, _ObjectReaderSet> _SourceToInstancesMap;

// Open an archive of different formats.
bool _OpenHDF5(const std::string& filePath, IArchive*,
std::string* format, std::recursive_mutex** mutex) const;
bool _OpenOgawa(const std::string& filePath, IArchive*,
std::string* format, std::recursive_mutex** mutex) const;
bool _OpenGit(const std::string& filePath, IArchive*,
std::string* format, std::recursive_mutex** mutex) const;

// Walk the object hierarchy looking for instances and instance sources.
static void _FindInstances(const IObject& parent,
_SourceToInstancesMap* instances);
Expand Down Expand Up @@ -894,22 +878,58 @@ _ReaderContext::_ReaderContext() :
}

bool
_ReaderContext::Open(const std::string& filePath, std::string* errorLog)
_ReaderContext::Open(const std::string& filePath, std::string* errorLog,
const SdfFileFormat::FileFormatArguments& args)
{
Close();

IArchive archive;
std::vector<std::string> layeredABC;
{
auto abcLayers = args.find("abcLayers");
if (abcLayers != args.end()) {
for (auto&& l : TfStringSplit(abcLayers->second, ",")) {
layeredABC.emplace_back(std::move(l));
Copy link
Member

Choose a reason for hiding this comment

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

I know Alembic doesn't do any asset resolution, by-design... but would it not be valuable in the USD universe to attempt to anchor (to "filePath") and resolve each of these paths using the installed ArAssetResolver before passing the paths down to Alembic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've looked into this, but it's currently a bit problematic on the Alembic side to pass layers as relative (particularly in a multi-threaded env). Second, there may be cases where the layer is in a totally separate location, unrelated to the -base layer-.

Or are you suggesting to call the resolver within usdAbc.so ?

Copy link
Member

Choose a reason for hiding this comment

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

I wasn't suggesting that you pass relative paths to Alembic. Instead, use ArGetResolver().Resolve() (etc) to resolve the paths to full paths in USD-land before passing them on to Alembic.

But if that seems undesirable, then just please document in overview.dox that you must only supply full paths in the URI, which is a bit limiting for fancy resolver setups, but probably OK since this is for Alembic backwards compatibility.

Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah OK, then yes (i.e. this is currently already being done on the resolver side here), I'll add the note.

}
}
}
layeredABC.emplace_back(filePath);

#if PXR_HDF5_SUPPORT_ENABLED && !H5_HAVE_THREADSAFE
// HDF5 may not be thread-safe.
using lock_guard = std::lock_guard<std::recursive_mutex>;
std::unique_ptr<std::lock_guard<std::recursive_mutex>> hfd5Lock(new lock_guard(*_hdf5));
spiffmon marked this conversation as resolved.
Show resolved Hide resolved
#endif

using IFactory = ::Alembic::AbcCoreFactory::IFactory;
IFactory factory;
IFactory::CoreType abcType;
factory.setPolicy(Abc::ErrorHandler::Policy::kQuietNoopPolicy);
factory.setOgawaNumStreams(_GetNumOgawaStreams());
IArchive archive = factory.getArchive(layeredABC, abcType);

#if PXR_HDF5_SUPPORT_ENABLED && !H5_HAVE_THREADSAFE
if (abcType == IFactory::kHDF5 || abcType == IFactory::kLayer) {
// An HDF5, or layered which may have an HDF5 layer
_mutex = &*_hdf5;
} else {
// Don't need the HDF5 lock
hfd5Lock.reset();
}
#endif

std::string format;
if (!(_OpenOgawa(filePath, &archive, &format, &_mutex) ||
_OpenHDF5(filePath, &archive, &format, &_mutex) ||
_OpenGit(filePath, &archive, &format, &_mutex))) {
*errorLog = "Unsupported format";
switch (abcType) {
case IFactory::kHDF5: format = "HDF5"; break;
case IFactory::kOgawa: format = "Ogawa"; break;
case IFactory::kLayer: format = "Layer"; break;
default:
case IFactory::kUnknown: format = "Unknown"; break;
}
if (!archive.valid()) {
*errorLog = TfStringPrintf("Unsupported format: '%s'", format.c_str());
return false;
}

// Lock _mutex if it exists for remainder of this method.
_Lock lock(_mutex);

// Get info.
uint32_t apiVersion;
std::string writer, version, date, comment;
Expand All @@ -923,7 +943,7 @@ _ReaderContext::Open(const std::string& filePath, std::string* errorLog)
}

// Cut over.
_archive = archive;
_archive = std::move(archive);

// Fill pseudo-root in the cache.
const SdfPath rootPath = SdfPath::AbsoluteRootPath();
Expand Down Expand Up @@ -973,8 +993,30 @@ _ReaderContext::Open(const std::string& filePath, std::string* errorLog)
_SetupInstancing(instances, promotable, &usedRootNames);
}

// Re-root so the <defaultPrim> is actually the archive!
TfToken abcReRoot;
{
auto reRoot = args.find("abcReRoot");
if (reRoot != args.end()) {
if (!TfIsValidIdentifier(reRoot->second)) {
TF_WARN("[usdAbc] Ignoring re-root because identifer '%s' is"
" not valid (%s).", reRoot->second.c_str(),
filePath.c_str());
} else
abcReRoot = TfToken(reRoot->second);
}
}

// Fill rest of the cache.
_ReadPrimChildren(*this, root, rootPath, *_pseudoRoot);
if (!abcReRoot.IsEmpty()) {
SdfPath compPath = rootPath.AppendChild(abcReRoot);
auto& xform = _prims[compPath];
xform.typeName = UsdAbcPrimTypeNames->Xform;
xform.specifier = SdfSpecifierDef;
_ReadPrimChildren(*this, root, compPath, xform);
_pseudoRoot->children.emplace_back(std::move(abcReRoot));
spiffmon marked this conversation as resolved.
Show resolved Hide resolved
} else
_ReadPrimChildren(*this, root, rootPath, *_pseudoRoot);

// Append the masters to the pseudo-root. We use lexicographical order
// but the order doesn't really matter. We also note here the Alembic
Expand Down Expand Up @@ -1022,19 +1064,21 @@ _ReaderContext::Open(const std::string& filePath, std::string* errorLog)
_GetDoubleMetadata(metadata, _pseudoRoot->metadata,
SdfFieldKeys->FramesPerSecond);

// Read the default prim name.
_GetTokenMetadata(metadata, _pseudoRoot->metadata,
SdfFieldKeys->DefaultPrim);
UsdGeomTokens->upAxis);

// Read the default prim name.
_GetTokenMetadata(metadata, _pseudoRoot->metadata,
UsdGeomTokens->upAxis);
SdfFieldKeys->DefaultPrim);
}

// If no default prim then choose one by a heuristic (first root prim).
if (!_pseudoRoot->children.empty()) {
_pseudoRoot->metadata.insert(
std::make_pair(SdfFieldKeys->DefaultPrim,
VtValue(_pseudoRoot->children.front())));
// Use emplace to leave existing property above untouched and avoid the
// VtValue construction if possible (gcc-4.8 requires the fun syntax)
_pseudoRoot->metadata.emplace(std::piecewise_construct,
std::forward_as_tuple(SdfFieldKeys->DefaultPrim),
std::forward_as_tuple(_pseudoRoot->children.front()));
}

return true;
Expand Down Expand Up @@ -1327,69 +1371,6 @@ _ReaderContext::ListTimeSamplesForPath(const SdfPath& path) const
return empty;
}

bool
_ReaderContext::_OpenHDF5(
const std::string& filePath,
IArchive* result,
std::string* format,
std::recursive_mutex** mutex) const
{
#ifdef PXR_HDF5_SUPPORT_ENABLED
// HDF5 may not be thread-safe.
std::lock_guard<std::recursive_mutex> lock(*_hdf5);

*format = "HDF5";
*result = IArchive(Alembic::AbcCoreHDF5::ReadArchive(),
filePath, ErrorHandler::kQuietNoopPolicy);
if (*result) {
// Single thread access to HDF5.
*mutex = &*_hdf5;
return true;
}
return false;
#else
return false;
#endif // PXR_HDF5_SUPPORT_ENABLED
}

bool
_ReaderContext::_OpenOgawa(
const std::string& filePath,
IArchive* result,
std::string* format,
std::recursive_mutex** mutex) const
{
*format = "Ogawa";
#if ALEMBIC_LIBRARY_VERSION >= 10709
*result = IArchive(
Alembic::AbcCoreOgawa::ReadArchive(
_GetNumOgawaStreams(),
TfGetEnvSetting(USD_ABC_READ_ARCHIVE_USE_MMAP)),
filePath, ErrorHandler::kQuietNoopPolicy);
#else
*result = IArchive(Alembic::AbcCoreOgawa::ReadArchive(_GetNumOgawaStreams()),
filePath, ErrorHandler::kQuietNoopPolicy);
#endif
return *result;
}

bool
_ReaderContext::_OpenGit(
const std::string& filePath,
IArchive* result,
std::string* format,
std::recursive_mutex** mutex) const
{
#ifdef PXR_MULTIVERSE_SUPPORT_ENABLED
*format = "Git";
*result = IArchive(Alembic::AbcCoreGit::ReadArchive(),
filePath, ErrorHandler::kQuietNoopPolicy);
return *result;
#else
return false;
#endif // PXR_MULTIVERSE_SUPPORT_ENABLED
}

void
_ReaderContext::_FindInstances(
const IObject& parent,
Expand Down Expand Up @@ -4057,13 +4038,14 @@ UsdAbc_AlembicDataReader::~UsdAbc_AlembicDataReader()
}

bool
UsdAbc_AlembicDataReader::Open(const std::string& filePath)
UsdAbc_AlembicDataReader::Open(const std::string& filePath,
const SdfFileFormat::FileFormatArguments& args)
{
TRACE_FUNCTION();

_errorLog.clear();
try {
if (_impl->Open(filePath, &_errorLog)) {
if (_impl->Open(filePath, &_errorLog, args)) {
return true;
}
}
Expand Down
3 changes: 2 additions & 1 deletion pxr/usd/plugin/usdAbc/alembicReader.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

#include "pxr/pxr.h"
#include "pxr/usd/sdf/abstractData.h"
#include "pxr/usd/sdf/fileFormat.h"
#include "pxr/base/tf/token.h"
#include <boost/noncopyable.hpp>
#include <boost/scoped_ptr.hpp>
Expand All @@ -54,7 +55,7 @@ class UsdAbc_AlembicDataReader : boost::noncopyable {

/// Open a file. Returns \c true on success; errors are reported by
/// \c GetErrors().
bool Open(const std::string& filePath);
bool Open(const std::string& filePath, const SdfFileFormat::FileFormatArguments&);

/// Close the file.
void Close();
Expand Down
Loading