CoordinateSystem: Backwards compatibility - Read legacy transform #1504

yannci · 2025-12-11T22:46:37Z

CoordinateSystem used to derive from the deprecated and removed StateRenderable holding additional transform information for the renderer. A better approach that is also used in Gaffer, the transform information is better stored as separate transform information. Cortex 10.6 also removed all DCC integrations, i.e. IECoreMaya, which stored, for example Locator transformation on the CoordinateSystem. We provide a way to still retrieve this information from old SceneCaches, although not fully transparent and backwards compatible. Clients of CoordinateSystem will need to be adjusted if needed.

The M44fData is stored as LegacyTransform in the BlindData of the CoordinateSystem

Related Issues

List any Issues this PR addresses or solves

Checklist

I have read the contribution guidelines.
I have updated the documentation, if applicable.
I have tested my change(s) in the test suite, and added new test cases where necessary.
My code follows the Cortex project's prevailing coding style and conventions.

yannci · 2025-12-11T22:54:29Z

@johnhaddon Thanks again for giving me pointers for the different approaches. I think the less complex way of reading the matrix directly in CoordinateSystem::load and putting it in the BlindData is sufficient. Yes, client code needs to be adjusted but I am unsure how many people outside of IE actually need that kind of backwards compatibility, hence why I considered not to add this to the CoordindateSystem test suite. Let me know if my implementation is overly complicated for the matter or if there's anything I might have missed.

Cheers,
Yannic

johnhaddon

Hi @yannci, thanks for taking this on.

Let me know if my implementation is overly complicated for the matter or if there's anything I might have missed.

I've made a few notes on the implementation, but I'll leave it to you and @ivanimanishi to decide if they're relevant to you or not. Since none of this affects Gaffer users I think it's your call rather than mine.

I considered not to add this to the CoordindateSystem test suite

Again, your call. But personally if I was depending on it I think I would want some test coverage (unless perhaps you have some internally).

Cheers...
John

johnhaddon · 2025-12-15T10:44:24Z

src/IECoreScene/CoordinateSystem.cpp

+		for ( auto levelDirectory : levelDirectories )
+		{
+			ConstIndexedIOPtr subdirectory = directory->subdirectory( levelDirectory, IndexedIO::MissingBehaviour::NullIfMissing );
+			if( subdirectory )
+			{
+			    findMatrixEntry( subdirectory );
+			}


Are you using this recursive search because you need to support MatrixMotionTransform as well as MatrixTransform, and it's convenient to deal with both using the same code?

I solely focused on MatrixTransform. I don't think any on the DCC modules nor anything else in the IE codebase use it. The reason I went for the recursive search is, because it felt like the cleanest option, so I'd be happy to hear about a better way!

I initially wanted to go directly to the matrix entry, but IndexedIO::directory takes an EntryIDList type path. Constructing that path left me with a very ugly initializer list of interned strings since I need to add all the intermediate data directories as well (data/transform/data/MatrixTransform/data/matrix is the path underneath the current container). I figured I could just push back the required children in a loop, but somehow a more generic recursive searched felt best at the time.

To me, a naive reader, the recursive search says "I want to find any entry called matrix, no matter where it might be in an arbitrary tree of things". If we're actually looking for one specific thing, and we know where it should be, then recursive search seems fragile - it might find unexpected things. Hardcoding the expected path is more robust, and also helps a reader understand exactly what we're looking for.

To satisfy my curiosity, I tried this more direct approach. I don't know what's up with IndexedIO::directory() but I couldn't get it to work, so I did have to iterate the directories by hand : johnhaddon@c128789.

I agree with you on the impression the recursion causes and I had preferred a different approach.
That being said, Today I Learned that auto in a loop can take an initializer list. This makes the more robust solution much cleaner and also way more readable than my initial attempts.
I took the liberty to apply your changes on top of mine, squashed it together and added a test case for it.

johnhaddon · 2025-12-15T10:45:47Z

src/IECoreScene/CoordinateSystem.cpp

+		}
+	};
+
+	findMatrixEntry( container );


Perhaps it's worth starting the search from container->subdirectory( "transform" ), so we're sure we can't accidentally pick up fields from elsewhere? This would also avoid a little bit of work when loading new files, where the subdirectory won't even exist.

Similarly to the comment about using a recursive search, if I recall correctly, subdirectory did not give me the transform since it's one level deeper under data. I think container::load takes this into account when loading the actual object.

Here again, I might have missed something and am very open to a different and better solution

johnhaddon · 2025-12-15T10:47:22Z

src/IECoreScene/CoordinateSystem.cpp

+		if( directory->hasEntry( matrixEntry ) )
+		{
+			float *f = matrix.getValue();
+			directory->read( matrixEntry, f, 16 );


Should this return as soon as a matrix is found? And if so, should we stop the iteration through levelDirectories when it does?

johnhaddon · 2025-12-15T10:50:59Z

src/IECoreScene/CoordinateSystem.cpp

+		}
+
+		IndexedIO::EntryIDList levelDirectories;
+		directory->entryIds( levelDirectories, IndexedIO::EntryType::Directory );


I have a feeling that entryIds() doesn't return things in a stable order from one run to the next (the order is based on InternedString comparisons, which can give different results from process to process). So in the case of MatrixMotionTransform, you may not always be picking up the same time sample.

yannci

Thanks for the initial review John. I have focused on two of your comments to see if there is a better approach than using a recursive search.

Based on that, I will have a look at the other comments, i.e. early exit and starting from a subdirectory. We in theory have tests for IECoreMaya and IECoreHoudini which I will update to cover both cases but I might reconsider adding one to Cortex itself.

Thanks again,
Yannic

yannci · 2025-12-15T20:43:11Z

src/IECoreScene/CoordinateSystem.cpp

+		for ( auto levelDirectory : levelDirectories )
+		{
+			ConstIndexedIOPtr subdirectory = directory->subdirectory( levelDirectory, IndexedIO::MissingBehaviour::NullIfMissing );
+			if( subdirectory )
+			{
+			    findMatrixEntry( subdirectory );
+			}


I solely focused on MatrixTransform. I don't think any on the DCC modules nor anything else in the IE codebase use it. The reason I went for the recursive search is, because it felt like the cleanest option, so I'd be happy to hear about a better way!

I initially wanted to go directly to the matrix entry, but IndexedIO::directory takes an EntryIDList type path. Constructing that path left me with a very ugly initializer list of interned strings since I need to add all the intermediate data directories as well (data/transform/data/MatrixTransform/data/matrix is the path underneath the current container). I figured I could just push back the required children in a loop, but somehow a more generic recursive searched felt best at the time.

yannci · 2025-12-15T20:48:26Z

src/IECoreScene/CoordinateSystem.cpp

+		}
+	};
+
+	findMatrixEntry( container );


Similarly to the comment about using a recursive search, if I recall correctly, subdirectory did not give me the transform since it's one level deeper under data. I think container::load takes this into account when loading the actual object.

Here again, I might have missed something and am very open to a different and better solution

CoordinateSystem used to have a transform property which has been removed in favour of accessing transforms independently of objects. We provide a way to retrieve this information from SceneInterfaces written prior to removal, as blind data.

johnhaddon reviewed Dec 15, 2025

View reviewed changes

yannci commented Dec 15, 2025

View reviewed changes

CoordinateSystem: Backwards compatibility

bbd4a07

CoordinateSystem used to have a transform property which has been removed in favour of accessing transforms independently of objects. We provide a way to retrieve this information from SceneInterfaces written prior to removal, as blind data.

vfxenjoyer force-pushed the coordinateSystemLegacyTransform branch from 2a57b9c to bbd4a07 Compare December 19, 2025 21:51

CoordinateSystem: Backwards compatibility - Read legacy transform #1504

Are you sure you want to change the base?

CoordinateSystem: Backwards compatibility - Read legacy transform #1504

Uh oh!

Conversation

yannci commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related Issues

Checklist

Uh oh!

yannci commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

johnhaddon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yannci left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

yannci commented Dec 11, 2025 •

edited

Loading

yannci commented Dec 11, 2025 •

edited

Loading