Skip to content

Conversation

@gregcotten
Copy link
Contributor

Plugins should be able to opt-in to host's LUT generators (or other non-spatial renders), and hosts need a way of indicating to the plugin that the current render requires rendering without spatial awareness.

Resolve has already defined an extension OfxImageEffectPropNoSpatialAwareness, so I propose using that for both the plugin descriptor and the read-only render argument passed by the host.

Closes #127

@gregcotten gregcotten force-pushed the feature/no-spatial-awareness branch from 452fd74 to b287152 Compare May 6, 2025 16:49
Copy link
Contributor

@garyo garyo left a comment

Choose a reason for hiding this comment

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

LGTM! Once this is merged, please add a note in release-notes.md for it.

@gregcotten
Copy link
Contributor Author

LGTM! Once this is merged, please add a note in release-notes.md for it.

Would it be better to add it in this PR instead?

@gregcotten
Copy link
Contributor Author

@barretpj at your convenience, does this look OK to you?

@gregcotten
Copy link
Contributor Author

gregcotten commented May 6, 2025

OK, I'm all set. Just need to know if @garyo would like the change to release-notes.md added to this PR

@garyo
Copy link
Contributor

garyo commented May 6, 2025

If you don't mind rebasing this onto latest main, where I just started a 1.6 section of release-notes.md, then yes please, that would be great. Rebase and add to the relnotes.

@gregcotten gregcotten force-pushed the feature/no-spatial-awareness branch 2 times, most recently from d09f169 to 1737d41 Compare May 6, 2025 19:16
@gregcotten
Copy link
Contributor Author

If you don't mind rebasing this onto latest main, where I just started a 1.6 section of release-notes.md, then yes please, that would be great. Rebase and add to the relnotes.

done

@gregcotten gregcotten force-pushed the feature/no-spatial-awareness branch from cfaaaa4 to 7162917 Compare May 6, 2025 19:22
@garyo garyo merged commit 8ac2337 into AcademySoftwareFoundation:main May 6, 2025
9 checks passed
@revisionfx
Copy link
Contributor

I saw a comment about kOfxImageEffectPropRenderQualityDraft
What would be more useful to us is kOfxImageEffectPropRenderQualityThumbnails

Pierre

@gregcotten gregcotten deleted the feature/no-spatial-awareness branch May 7, 2025 15:55
@gregcotten
Copy link
Contributor Author

I saw a comment about kOfxImageEffectPropRenderQualityDraft What would be more useful to us is kOfxImageEffectPropRenderQualityThumbnails

Might want to make a new issue about that. This one is closed!

@barretpj
Copy link
Contributor

barretpj commented May 7, 2025

I saw a comment about kOfxImageEffectPropRenderQualityDraft What would be more useful to us is kOfxImageEffectPropRenderQualityThumbnails

Might want to make a new issue about that. This one is closed!

Done - #193

@barretpj
Copy link
Contributor

(It should really be NoTemporal as well...)

@revisionfx
Copy link
Contributor

DO you mean multi-frame processing (get 3 frames from input) - Yes|OK - But what things like basic time offsets performed by plugin (even if only a single frame is requested)?.. Some hosts have a single thumbnail (a one time thing using first or current frame) while other populates on a timeline multiple frames.

@barretpj
Copy link
Contributor

This is nothing to do with thumbnails. This is when we want to do a render which only changes the colour of pixels based on their input colour, so the host can analyse the overall render and approximate the changes with a LUT. If the plugin is considering other pixels, or pixels from images at other times, then the image analysis in the host will most likely fail.

@gregcotten
Copy link
Contributor Author

I believe @barretpj is referring to a render with NoSpatialAwareness set to "true" should also mean it has "no temporal awareness"

@garyo
Copy link
Contributor

garyo commented May 12, 2025

This PR is already merged, and we're using the prop name from Resolve. And yes I agree w/ @barretpj . but I don't think we should change the prop name.

@gregcotten
Copy link
Contributor Author

This is nothing to do with thumbnails. This is when we want to do a render which only changes the colour of pixels based on their input colour, so the host can analyse the overall render and approximate the changes with a LUT. If the plugin is considering other pixels, or pixels from images at other times, then the image analysis in the host will most likely fail.

Sensible. We can adjust the description accordingly.

@barretpj
Copy link
Contributor

@revisionfx Plugins must only set NoSpatialAwareness="true" if they are able to operate in a change-current-frame-pixel-colour-only mode (either all the time, or when requested by the host via NoSpatialAwareness="true" in the render args)

@revisionfx
Copy link
Contributor

revisionfx commented May 12, 2025

Got it, I thought we were on thumbnail thread :)

@garyo - another option would be to properly name it and add a #define // for backward compatibility with Resolve property that means the same

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a "LUT" render argument

4 participants