Skip to content

Commit

Permalink
FIX : Warn for partially missing normals
Browse files Browse the repository at this point in the history
  • Loading branch information
danieldresser-ie committed Sep 10, 2024
1 parent a8eabf2 commit 99271ae
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 3 deletions.
25 changes: 22 additions & 3 deletions python/GafferSceneTest/IECoreScenePreviewTest/PrimitiveAlgoTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,22 @@ def testMergePrimitivesSimpleMeshes( self ) :
IECore.V3fData( imath.V3f( 0.1, 0.2, 0.3 ), IECore.GeometricData.Interpretation.Point )
)

with IECore.CapturingMessageHandler() as mh:
mergedWithMissingN = PrimitiveAlgo.mergePrimitives( [( mesh1, imath.M44f() ), ( mesh2, imath.M44f() ) ] )
self.assertEqual( len( mh.messages ), 1 )
self.assertEqual( mh.messages[0].context, "mergePrimitives" )
self.assertEqual( mh.messages[0].message, 'Primitive variable N missing on some input primitives, defaulting to zero length normals.' )

# The effect of not specifying a normal is the same as a Constant, zero normal
mesh2["N"] = IECoreScene.PrimitiveVariable( Interpolation.Constant,
IECore.V3fData( imath.V3f( 0 ), IECore.GeometricData.Interpretation.Normal )
)

merged = PrimitiveAlgo.mergePrimitives( [( mesh1, imath.M44f() ), ( mesh2, imath.M44f() ) ] )
print( merged["N"], "\n\n", mergedWithMissingN["N"] )
self.assertEqual( merged["N"], mergedWithMissingN["N"] )
self.assertEqual( merged, mergedWithMissingN )

self.assertTrue( merged.arePrimitiveVariablesValid() )
self.assertEqual( merged.interpolation, "linear" )
self.assertEqual( merged.verticesPerFace, IECore.IntVectorData( [ 4, 3 ] ) )
Expand Down Expand Up @@ -436,8 +451,6 @@ def randomC():
self.assertEqual( len( mh.messages ), numDiscarded )

for k in allInterpsRef.keys():
#if k == "P":
#continue

kInterp = allInterps[k].interpolation

Expand Down Expand Up @@ -621,7 +634,13 @@ def testMergeMeshes( self ) :
self.resamplePrimVars( referencePlane, [ "altUv", "unindexedFaceVarying" ], Interpolation.FaceVarying )

# Merge 3 meshes - complex mesh, shifted complex mesh, and a plane
merged = PrimitiveAlgo.mergePrimitives( [( source, imath.M44f() ), ( sourcePlane, imath.M44f() ), ( sourceShifted, imath.M44f() )] )

with IECore.CapturingMessageHandler() as mh:
merged = PrimitiveAlgo.mergePrimitives(
[( source, imath.M44f() ), ( sourcePlane, imath.M44f() ), ( sourceShifted, imath.M44f() )]
)
self.assertEqual( len( mh.messages ), 1 )
self.assertEqual( mh.messages[0].message, 'Primitive variable N missing on some input primitives, defaulting to zero length normals.' )

referenceSource["N"] = IECoreScene.PrimitiveVariable( Interpolation.Vertex, IECore.V3fVectorData( [imath.V3f( 0 ) ] * referenceSource.variableSize( Interpolation.Vertex ) ) )
referenceShifted["N"] = IECoreScene.PrimitiveVariable( Interpolation.Vertex, IECore.V3fVectorData( [imath.V3f( 0 ) ] * referenceSource.variableSize( Interpolation.Vertex ) ) )
Expand Down
15 changes: 15 additions & 0 deletions src/GafferScene/IECoreScenePreview/PrimitiveAlgo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1029,6 +1029,8 @@ IECoreScene::PrimitivePtr mergePrimitivesInternal(


// We also need to count the amount of data for this primvar contributed by each primitive.

bool incomplete = false;
for( unsigned int i = 0; i < primitives.size(); i++ )
{

Expand All @@ -1046,6 +1048,7 @@ IECoreScene::PrimitivePtr mergePrimitivesInternal(
// the indexed case.
varInfo.numData[i] = 1;
varInfo.indexed = true;
incomplete = true;
continue;
}

Expand All @@ -1058,6 +1061,18 @@ IECoreScene::PrimitivePtr mergePrimitivesInternal(
varInfo.indexed = true;
}
}

if( incomplete )
{
if( name == "N" )
{
// Using default initialized normals is particularly likely to produce confusion, so we have a special
// warning for this case.
msg( Msg::Warning, "mergePrimitives",
"Primitive variable N missing on some input primitives, defaulting to zero length normals."
);
}
}
}

//
Expand Down

0 comments on commit 99271ae

Please sign in to comment.