Merge pull request #1537 from johnhaddon/firstClassPointInstancer

PointInstancer class

Author: johnhaddon

Changes

@@ -1,16 +1,35 @@
 10.7.x.x (relative to 10.7.0.0a10)
 ========
 
+Features
+--------
+
+- PointInstancer : Added class to provide first-class encoding of point instancers.
+
+Improvements
+------------
+
+- PrimitiveVariable : Added Python bindings for `IndexedView` class.
+- USDScene : Added support for writing `IECoreScene::PointInstancer` as `UsdGeomPointInstancer`.
+
 Fixes
 -----
 
 - RunTimeTyped : Fixed unnecessary overhead of `runTimeCast()` and `isInstanceOf()` for subclasses implemented in Python. The GIL is now never needed when the cast is to a C++ type.
+- PrimitiveVariable : Made IndexedView `begin()` and `end()` const.
+- PointsAlgo :
+  - Fixed loss of blind data in `deletePoints()`.
+  - Fixed loss of primitive type in `deletePoints()` (if processing a derived class of PointsPrimitive).
 
 Breaking Changes
 ----------------
 
 - ClassParameter, ClassVectorParameter : Replaced statically allocated TypeIds with ids allocated dynamically by `IECore.registerRunTimeTyped()`. If the id is needed from C++, it can be queried at runtime with `RunTimeTyped::typeIdFromTypeName()`.
 - RunTimeTyped : Removed `typId` argument from `registerRunTimeTyped()` Python function. TypeIds are now always allocated dynamically for Python classes.
+- USD :
+  - Removed support for `IECOREUSD_POINTINSTANCER_RELATIVE_PROTOTYPES` environment variable. Prototype paths are now always specified as relative where possible.
+  - Removed `./` prefix from relative prototype paths.
+  - UsdGeomPointInstancer `inactiveIds` are now merged into the `invisibleIds` primitive variable on loading, instead of being loaded separately.
 
 Build
 -----

contrib/IECoreUSD/include/IECoreUSD/PrimitiveAlgo.h

@@ -62,6 +62,10 @@ IECOREUSD_API void writePrimitiveVariable( const std::string &name, const IECore
 IECOREUSD_API void writePrimitiveVariable( const std::string &name, const IECoreScene::PrimitiveVariable &primitiveVariable, const pxr::UsdGeomGprim &gprim, pxr::UsdTimeCode time );
 /// As above, but redirects "P", "N" etc to the relevant attributes of `pointBased`.
 IECOREUSD_API void writePrimitiveVariable( const std::string &name, const IECoreScene::PrimitiveVariable &primitiveVariable, pxr::UsdGeomPointBased &pointBased, pxr::UsdTimeCode time );
+
+/// Expands an IndexedView into a VtArray.
+template<typename T>
+IECOREUSD_API pxr::VtValue toUSDExpanded( const IECoreScene::PrimitiveVariable::IndexedView<T> &view );
 /// Equivalent to `DataAlgo::toUSD( primitiveVariable.expandedData() )`, but avoiding
 /// the creation of the temporary expanded data.
 IECOREUSD_API pxr::VtValue toUSDExpanded( const IECoreScene::PrimitiveVariable &primitiveVariable, bool arrayRequired = false );
@@ -89,4 +93,6 @@ IECOREUSD_API IECoreScene::PrimitiveVariable::Interpolation fromUSD( pxr::TfToke
 
 } // namespace IECoreUSD
 
+#include "IECoreUSD/PrimitiveAlgo.inl"
+
 #endif // IECOREUSD_PRIMITIVEALGO_H

contrib/IECoreUSD/include/IECoreUSD/PrimitiveAlgo.inl

@@ -0,0 +1,64 @@
+//////////////////////////////////////////////////////////////////////////
+//
+//  Copyright (c) 2026, Cinesite VFX Ltd. All rights reserved.
+//
+//  Redistribution and use in source and binary forms, with or without
+//  modification, are permitted provided that the following conditions are
+//  met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//
+//     * Redistributions in binary form must reproduce the above copyright
+//       notice, this list of conditions and the following disclaimer in the
+//       documentation and/or other materials provided with the distribution.
+//
+//     * Neither the name of Image Engine Design nor the names of any
+//       other contributors to this software may be used to endorse or
+//       promote products derived from this software without specific prior
+//       written permission.
+//
+//  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS
+//  IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+//  THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+//  PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+//  CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+//  EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+//  PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+//  PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+//  LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+//  NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+//  SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+//
+//////////////////////////////////////////////////////////////////////////
+
+#ifndef IECOREUSD_PRIMITIVEALGO_INL
+#define IECOREUSD_PRIMITIVEALGO_INL
+
+#include "IECoreUSD/DataAlgo.h"
+
+IECORE_PUSH_DEFAULT_VISIBILITY
+#include "pxr/base/gf/quatf.h"
+#include "pxr/base/gf/quatd.h"
+IECORE_POP_DEFAULT_VISIBILITY
+
+namespace IECoreUSD::PrimitiveAlgo
+{
+
+template<typename T>
+pxr::VtValue toUSDExpanded( const IECoreScene::PrimitiveVariable::IndexedView<T> &view )
+{
+	using USDType = typename CortexTypeTraits<T>::USDType;
+	pxr::VtArray<USDType> array;
+	array.reserve( view.size() );
+	for( const auto &e : view )
+	{
+		array.push_back( DataAlgo::toUSD( static_cast<const T &>( e ) ) );
+	}
+
+	return pxr::VtValue( array );
+}
+
+} // namespace IECoreUSD::PrimitiveAlgo
+
+#endif // IECOREUSD_PRIMITIVEALGO_INL

contrib/IECoreUSD/src/IECoreUSD/PointInstancerAlgo.cpp

@@ -36,12 +36,15 @@
 #include "IECoreUSD/ObjectAlgo.h"
 #include "IECoreUSD/PrimitiveAlgo.h"
 
-#include "IECoreScene/PointsPrimitive.h"
+#include "IECoreScene/PointInstancer.h"
 
 IECORE_PUSH_DEFAULT_VISIBILITY
 #include "pxr/usd/usdGeom/pointInstancer.h"
 IECORE_POP_DEFAULT_VISIBILITY
 
+#include "boost/algorithm/string/predicate.hpp"
+#include "boost/container/flat_set.hpp"
+
 using namespace IECore;
 using namespace IECoreScene;
 using namespace IECoreUSD;
@@ -53,28 +56,15 @@ using namespace IECoreUSD;
 namespace
 {
 
-bool checkEnvFlag( const char *envVar, bool def )
-{
-	const char *value = getenv( envVar );
-	if( value )
-	{
-		return std::string( value ) != "0";
-	}
-	else
-	{
-		return def;
-	}
-}
-
 IECore::ObjectPtr readPointInstancer( pxr::UsdGeomPointInstancer &pointInstancer, pxr::UsdTimeCode time, const Canceller *canceller )
 {
 	pxr::VtVec3fArray pointsData;
 	pointInstancer.GetPositionsAttr().Get( &pointsData, time );
 	Canceller::check( canceller );
 	IECore::V3fVectorDataPtr positionData = DataAlgo::fromUSD( pointsData );
 
-	positionData->setInterpretation( GeometricData::Point );
-	IECoreScene::PointsPrimitivePtr newPoints = new IECoreScene::PointsPrimitive( positionData );
+	IECoreScene::PointInstancerPtr newPoints = new IECoreScene::PointInstancer( positionData->readable().size() );
+	newPoints->setPosition( positionData );
 
 	// Per point attributes
 
@@ -99,29 +89,49 @@ IECore::ObjectPtr readPointInstancer( pxr::UsdGeomPointInstancer &pointInstancer
 	Canceller::check( canceller );
 	PrimitiveAlgo::readPrimitiveVariable( pointInstancer.GetAngularVelocitiesAttr(), time, newPoints.get(), "angularVelocity" );
 
+	// Inactive and invisible IDs. These both do the same thing - prevent specific instances
+	// from rendering. Inactive IDs are metadata-based and therefore can't be animated.
+	// Invisible IDs are attribute-based and therefore can be animated. Since we're converting
+	// to Cortex PrimitiveVariables, the distinction is irrelevant - all PrimitiveVariables can
+	// be animated. Our PointInstancer therefore just has invisible IDs, which we merge both
+	// USD properties into.
+
+	IECore::Int64VectorDataPtr invisibleIds;
 	if( pointInstancer.GetInvisibleIdsAttr().HasAuthoredValue() )
 	{
-		DataPtr cortexInvisIds = DataAlgo::fromUSD( pointInstancer.GetInvisibleIdsAttr(), time, true );
-		if( cortexInvisIds )
-		{
-			newPoints->variables["invisibleIds"] = IECoreScene::PrimitiveVariable(
-				PrimitiveVariable::Constant, cortexInvisIds
-			);
-		}
+		invisibleIds = IECore::runTimeCast<IECore::Int64VectorData>(
+			DataAlgo::fromUSD( pointInstancer.GetInvisibleIdsAttr(), time, true )
+		);
 	}
 
 	pxr::SdfInt64ListOp inactiveIdsListOp;
 	if( pointInstancer.GetPrim().GetMetadata( pxr::UsdGeomTokens->inactiveIds, &inactiveIdsListOp ) )
 	{
-		newPoints->variables["inactiveIds"] = IECoreScene::PrimitiveVariable(
-			PrimitiveVariable::Constant,
-			new IECore::Int64VectorData( inactiveIdsListOp.GetExplicitItems() )
-		);
+		const std::vector<int64_t> &inactiveIds = inactiveIdsListOp.GetExplicitItems();
+		if( inactiveIds.size() )
+		{
+			if( invisibleIds )
+			{
+				invisibleIds->writable().insert(
+					invisibleIds->writable().end(),
+					inactiveIds.begin(), inactiveIds.end()
+				);
+				std::sort( invisibleIds->writable().begin(), invisibleIds->writable().end() );
+				invisibleIds->writable().erase(
+					std::unique( invisibleIds->writable().begin(), invisibleIds->writable().end() ),
+					invisibleIds->writable().end()
+				);
+			}
+			else
+			{
+				invisibleIds = new Int64VectorData( inactiveIds );
+			}
+		}
 	}
 
-	// Prototype paths
+	newPoints->setInvisibleIDs( invisibleIds );
 
-	const static bool g_relativePrototypes = checkEnvFlag( "IECOREUSD_POINTINSTANCER_RELATIVE_PROTOTYPES", false );
+	// Prototype paths
 
 	pxr::SdfPathVector targets;
 	Canceller::check( canceller );
@@ -134,17 +144,13 @@ IECore::ObjectPtr readPointInstancer( pxr::UsdGeomPointInstancer &pointInstancer
 	prototypeRoots.reserve( targets.size() );
 	for( const auto &t : targets )
 	{
-		if( !g_relativePrototypes || !t.HasPrefix( primPath ) )
+		if( !t.HasPrefix( primPath ) )
 		{
 			prototypeRoots.push_back( t.GetString() );
 		}
 		else
 		{
-			// The ./ prefix shouldn't be necessary - we want to just use the absence of a leading
-			// slash to indicate relative paths. We can remove the prefix here once we deprecate the
-			// GAFFERSCENE_INSTANCER_EXPLICIT_ABSOLUTE_PATHS env var and have Gaffer always require a leading
-			// slash for absolute paths.
-			prototypeRoots.push_back( "./" + t.MakeRelativePath( primPath ).GetString() );
+			prototypeRoots.push_back( t.MakeRelativePath( primPath ).GetString() );
 		}
 	}
 
@@ -178,3 +184,100 @@ bool pointInstancerMightBeTimeVarying( pxr::UsdGeomPointInstancer &instancer )
 ObjectAlgo::ReaderDescription<pxr::UsdGeomPointInstancer> g_pointInstancerReaderDescription( pxr::TfToken( "PointInstancer" ), readPointInstancer, pointInstancerMightBeTimeVarying );
 
 } // namespace
+
+//////////////////////////////////////////////////////////////////////////
+// Writing
+//////////////////////////////////////////////////////////////////////////
+
+namespace
+{
+
+const boost::container::flat_set<std::string> g_exportedAsAttributes = {
+	"prototypeRoots", "prototypeIndex", "P", "scale", "orientation",
+	"id", "invisibleIds"
+};
+
+bool writePointInstancer( const IECoreScene::PointInstancer *instancer, const pxr::UsdStagePtr &stage, const pxr::SdfPath &path, pxr::UsdTimeCode time )
+{
+	auto usdInstancer = pxr::UsdGeomPointInstancer::Define( stage, path );
+
+	// Export primitive variables with special meaning to attributes
+	// of the UsdGeomPointInstancer.
+
+	if( auto prototypes = instancer->getPrototypes() )
+	{
+		pxr::SdfPathVector targets;
+		targets.reserve( prototypes.size() );
+		for( const auto &prototype : prototypes )
+		{
+			pxr::SdfPath prototypePath;
+			if( boost::starts_with( prototype, "./" ) )
+			{
+				prototypePath = pxr::SdfPath( prototype.substr( 2 ) );
+			}
+			else
+			{
+				prototypePath = pxr::SdfPath( prototype );
+			}
+			if( !prototypePath.IsAbsolutePath() )
+			{
+				prototypePath = prototypePath.MakeAbsolutePath( path );
+			}
+			targets.push_back( prototypePath );
+		}
+		usdInstancer.CreatePrototypesRel().SetTargets( targets );
+	}
+
+	if( auto prototypeIndex = instancer->getPrototypeIndex() )
+	{
+		usdInstancer.CreateProtoIndicesAttr().Set( PrimitiveAlgo::toUSDExpanded( prototypeIndex ), time );
+	}
+
+	if( auto position = instancer->getPosition() )
+	{
+		usdInstancer.CreatePositionsAttr().Set( PrimitiveAlgo::toUSDExpanded( position ), time );
+	}
+
+	if( auto orientation = instancer->getOrientation() )
+	{
+		// USD uses `half` for orientation, but Cortex only has a data type for
+		// `float` quaternions, so convert.
+		pxr::VtArray<pxr::GfQuath> usdOrientation;
+		usdOrientation.reserve( orientation.size() );
+		for( const auto &o : orientation )
+		{
+			usdOrientation.push_back( pxr::GfQuath( DataAlgo::toUSD( o ) ) );
+		}
+		usdInstancer.CreateOrientationsAttr().Set( usdOrientation, time );
+	}
+
+	if( auto scale = instancer->getScale() )
+	{
+		usdInstancer.CreateScalesAttr().Set( PrimitiveAlgo::toUSDExpanded( scale ), time );
+	}
+
+	if( auto id = instancer->getID() )
+	{
+		usdInstancer.CreateIdsAttr().Set( PrimitiveAlgo::toUSDExpanded( id ), time );
+	}
+
+	if( auto invisibleIds = instancer->getInvisibleIDs() )
+	{
+		usdInstancer.CreateInvisibleIdsAttr().Set( PrimitiveAlgo::toUSDExpanded( invisibleIds ), time );
+	}
+
+	for( const auto &[name, primitiveVariable] : instancer->variables )
+	{
+		if( g_exportedAsAttributes.count( name ) )
+		{
+			continue;
+		}
+		PrimitiveAlgo::writePrimitiveVariable( name, primitiveVariable, pxr::UsdGeomPrimvarsAPI( usdInstancer ), time );
+	}
+
+	return true;
+}
+
+ObjectAlgo::WriterDescription<PointInstancer> g_pointInstancerWriterDescription( writePointInstancer );
+
+} // namespace

contrib/IECoreUSD/src/IECoreUSD/PrimitiveAlgo.cpp

@@ -153,17 +153,8 @@ struct VtValueFromExpandedData
 	template<typename T>
 	VtValue operator()( const IECore::TypedData<vector<T>> *data, const IECore::IntVectorData *indices, typename std::enable_if<!std::is_void<typename CortexTypeTraits<T>::USDType>::value>::type *enabler = nullptr ) const
 	{
-		using USDType = typename CortexTypeTraits<T>::USDType;
-		using ArrayType = VtArray<USDType>;
-		ArrayType array;
-		array.reserve( indices->readable().size() );
-		// Using universal reference (`&&`) for iteration for compatibility with the
-		// non-standard proxy returned by `vector<bool>`.
-		for( auto &&e : PrimitiveVariable::IndexedView<T>( data->readable(), &indices->readable() ) )
-		{
-			array.push_back( DataAlgo::toUSD( static_cast<const T &>( e ) ) );
-		}
-		return VtValue( array );
+		PrimitiveVariable::IndexedView<T> view( data->readable(), &indices->readable() );
+		return IECoreUSD::PrimitiveAlgo::toUSDExpanded( view );
 	}
 
 	VtValue operator()( const IECore::Data *data, const IECore::IntVectorData *indices ) const

contrib/IECoreUSD/src/IECoreUSD/USDScene.cpp

@@ -265,6 +265,10 @@ using PrimPredicate = bool (pxr::UsdPrim::*)() const;
 boost::container::flat_map<pxr::TfToken, PrimPredicate> g_schemaTypeSetPredicates = {
 	{ pxr::TfToken( "__cameras" ), &pxr::UsdPrim::IsA<pxr::UsdGeomCamera> },
 	{  pxr::TfToken( "__lights" ), &pxr::UsdPrim::HasAPI<pxr::UsdLuxLightAPI> },
+	/// \todo This was introduced before we had `IECoreScene::PointInstancer`, to allow
+	/// us to tell which `IECoreScene::PointsPrimitives` came from UsdGeomPointInstancer.
+	/// We'll need to keep it for backwards compatibility for a while, but it might make
+	/// sense to remove it at some point.
 	{ pxr::TfToken( "usd:pointInstancers" ), &pxr::UsdPrim::IsA<pxr::UsdGeomPointInstancer> }
 };