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

PrimitiveVariableTweaks #6173

Draft
wants to merge 4 commits into
base: 1.5_maintenance
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ Features
- Inference : Loads ONNX models and performance inference using an array of input tensors.
- ImageToTensor : Converts images to tensors for use with the Inference node.
- TensorToImage : Converts tensors back to images following inference.
- PrimiitiveVariableTweaks : Added node for tweaking primitive variables. Can affect just part of a primitive based on ids or a mask.
Copy link
Member

Choose a reason for hiding this comment

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

PrimiitiveVariableTweaks -> PrimitiveVariableTweaks


API
---
Expand Down
47 changes: 34 additions & 13 deletions include/Gaffer/TweakPlug.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,30 +123,51 @@ class GAFFER_API TweakPlug : public Gaffer::ValuePlug
MissingMode missingMode = MissingMode::Error
) const;

template< typename T >
static T applyValueTweak(
const T &source,
const T &tweak,
TweakPlug::Mode mode,
const std::string &tweakName
);
Comment on lines +126 to +132
Copy link
Member

Choose a reason for hiding this comment

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

I'm not super comfortable with this way of factoring things, for a few reasons :

  • It leaves us duplicating a lot of the TweakPlug logic in a separate component, and that logic is not the most straightforward.
  • I think other TweakPlug clients could plausibly benefit from array operations (masked or unmasked).
  • We've managed to serve the needs of lots of different clients with a single API so far, and that API has successfully encapsulated all of the complexity (although I admit ShaderTweaks isn't as straightforward).

So, questions :

  • Could the existing applyTweak() function be modified to do non-masked operations on arrays, benefiting all existing clients?
  • Could an additional masked operation abstract away all the stuff to do with primitive variables? Maybe something like the below?
/// As above, but applying the tweak to individual elements of VectorTypedData,
/// as specified by `mask`.
template<class GetDataFunctor, class SetDataFunctor>
bool applyMaskedTweak(
	GetDataFunctor &&getDataFunctor,
	SetDataFunctor &&setDataFunctor,
        const vector<bool> &mask,
        // Size of array to make for `Create` mode.
        size_t createSize,
	MissingMode missingMode = MissingMode::Error
) const;


static const char *modeToString( Gaffer::TweakPlug::Mode mode );

private :

Gaffer::ValuePlug *valuePlugInternal();
const Gaffer::ValuePlug *valuePlugInternal() const;

void applyNumericTweak(
const IECore::Data *sourceData,
const IECore::Data *tweakData,
IECore::Data *destData,
template<typename T>
static T vectorAwareMin( const T &v1, const T &v2 );

template<typename T>
static T vectorAwareMax( const T &v1, const T &v2 );

template<typename T>
static std::vector<T> tweakedList( const std::vector<T> &source, const std::vector<T> &tweak, TweakPlug::Mode mode );

template< typename T >
static T applyNumericTweak(
const T &source,
const T &tweak,
TweakPlug::Mode mode,
const std::string &tweakName
) const;
);

void applyListTweak(
const IECore::Data *sourceData,
const IECore::Data *tweakData,
IECore::Data *destData,
template< typename T >
static T applyListTweak(
const T &source,
const T &tweak,
TweakPlug::Mode mode,
const std::string &tweakName
) const;

void applyReplaceTweak( const IECore::Data *sourceData, IECore::Data *tweakData ) const;
);

static const char *modeToString( Gaffer::TweakPlug::Mode mode );
template< typename T >
static T applyReplaceTweak(
const T &source,
const T &tweak
);

};

Expand Down
Loading