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

CollectImages : Add addLayerPrefix plug #5524

Merged
merged 3 commits into from
Nov 3, 2023

Conversation

johnhaddon
Copy link
Member

This is motivated by the discussion at https://groups.google.com/u/1/g/gaffer-dev/c/kJQ5fuKIHP8, where folks are trying to collect light group outputs from Arnold, where they are inevitably already prefixed with the layer name in the EXR file.

This removes quadratic performance in number of layer names, because we were calling `sourceLayerAndChannel()` once per output channel, and it was looping over all the input layers. This gives a modest 30% reduction in runtime for `testHighLayerCountPerformance`. That's not the main motivation for MappingData though - it provides the necessary foundations for the next commit.
This is motivated by the discussion at https://groups.google.com/u/1/g/gaffer-dev/c/kJQ5fuKIHP8, where folks are trying to collect light group outputs from Arnold, where they are inevitably already prefixed with the layer name in the EXR file.
@johnhaddon johnhaddon self-assigned this Nov 2, 2023
@danieldresser-ie
Copy link
Contributor

Oh, I see Murray is assigned as reviewer on this - but I took a skim through it, and the only issue I see is that when addLayerPrefix is off, the description of rootLayers isn't accurate.

@johnhaddon
Copy link
Member Author

the only issue I see is that when addLayerPrefix is off, the description of rootLayers isn't accurate.

Good point. I've pushed a fixup containing a fairly lame attempt at improving that. I tried to write something more helpful but couldn't find the good words - and decided that maybe the comment on addLayerPrefix was the right place to say it anyway.

@danieldresser-ie
Copy link
Contributor

Fixup seems reasonable.

Copy link
Contributor

@murraystevenson murraystevenson left a comment

Choose a reason for hiding this comment

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

Looks good to me, and does enable the desired workflow described in the gaffer-dev thread. 👍

@johnhaddon johnhaddon merged commit dc6d4a2 into GafferHQ:1.3_maintenance Nov 3, 2023
4 checks passed
@johnhaddon johnhaddon deleted the prefixWithLayerName branch November 8, 2023 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants