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

Catalogue : Fix additional images created by light group or crop window edits #5531

Merged

Conversation

johnhaddon
Copy link
Member

This fixes #4633 and #4267.

This reverts commit 8fd7b41. That commit may or may not have been necessary at the time, but it's definitely not necessary after GafferHQ#4864 added a fallback for the `image:viewName` context variable. Note that the code changed here only applies when the image is a render, and in that case we are already guaranteed to only have a default view anyway.
Instead output a diagnostic message.

This isn't something we expect to ever happen in normal GUI usage, because we push a single call handler and leave it there forever. But in the unit tests, we regularly push and pop handlers using the `ParallelAlgoTest.UIThreadCallHandler` context manager. Here, throwing was incredibly unhelpful, due to the following sequence :

1. Enter a UIThreadCallHandler.
2. Do something that will cause calls to be made on a background thread.
3. Assert something that fails, raising an exception and exiting the UIThreadCallHandler.
4. Background thread call continues and finds that there is no handler, and throws, causing process termination due to unhandled an exception.
5. We never get to see the assertion failure from 3, which is the key piece of information we need.
@johnhaddon johnhaddon self-assigned this Nov 7, 2023
The change to `testDisplayDriverAOVGrouping` is necessary because it was exploiting a bug in the previous implementation. As soon as the first image was sent the Catalogue was saving the image (because the driver was closed), but then still allowing other drivers to be added to the image (causing another save after that). We now reject new drivers as soon as we start a save, so the test needs to keep the first driver open until the second has been created. The test now matches more closely the sequence of events we'd see in a real render.
This stops the Catalogue from thinking the render has completed in cases where Arnold has to close all drivers before opening new ones.

Fixes GafferHQ#4633.
Fixes GafferHQ#4267.
@johnhaddon johnhaddon force-pushed the catalogueUnwantedDuplicates branch from a3b955c to 0164805 Compare November 7, 2023 15:42
In Arnold 7.1, Arnold bug #12282 prevents us from using the `layerName` parameter.
@danieldresser-ie
Copy link
Contributor

LGTM

@johnhaddon johnhaddon merged commit dbe896b into GafferHQ:1.3_maintenance Nov 8, 2023
4 checks passed
@johnhaddon johnhaddon deleted the catalogueUnwantedDuplicates 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.

2 participants