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

added CompositeVariantWithCallback #200

Closed
wants to merge 1 commit into from

Conversation

rdeioris
Copy link
Contributor

This patch adds a CompositeVariantWithCallback function allowing to define which variant to load during composition based on a custom callback:

bool VariantSelectionCallback(const tinyusdz::PrimSpec& PrimSpec, std::map<std::string, std::string>* VariantSelection, std::string* Warnings, std::string* Errors, void* UserData) {
  // fill VariantSelection map
}

@syoyo
Copy link
Collaborator

syoyo commented Oct 28, 2024

Please describe the purpose of this PR

@rdeioris
Copy link
Contributor Author

rdeioris commented Oct 28, 2024

Sure, you are right:

Composition process (at least in the current implementation) is a very recursive operation, where actually you should traverse all of the PrimSpec "discovered" by a composition step (like references or inheritance resolution) and check if a new variantset is available (and eventually overwrite the PrimSpec to select the wanted variant).

This makes "generalization" of variants management pretty hard (you need to inject your logic after every step of composition, or you need to apply all of the composition steps except variants and do everything at the end, potentially triggering another round of references resolution caused by a specific variant).

The proposed function allows the user that knows from the begining which variant to apply (and this is pretty common for runtime loaders) to apply on the fly during the recursive process (like in tusdcat)

As an example if i know from the beginnign that i want the "HighDef" variant for all of the PrimSpec i can simply have simple callback setting it on the fly:

bool VariantSelectionCallback(const tinyusdz::PrimSpec& PrimSpec, std::map<std::string, std::string>* VariantSelection, std::string* Warnings, std::string* Errors, void* UserData) {
  for(const auto& pair : PrimSpec.variantSets())
  {
    (*VariantSelection)[pair.first] = "HighDef";
  }
}

@syoyo
Copy link
Collaborator

syoyo commented Oct 28, 2024

The proposed function allows the user that knows from the begining which variant to apply

AFAIK I have prepared ApplyVariantSelector for that purpose(no implementation yet)

bool ApplyVariantSelector(const Layer &layer, const VariantSelectorMap &vsmap,

So I guess implementing ApplyVariantSelector will be a preferred way instead of providing callback function.

Also it is better to submit an issue or raise a discussion(in "Discussion" pane https://github.com/lighttransport/tinyusdz/discussions ) before sending a PR if you add new feature.

@rdeioris
Copy link
Contributor Author

@syoyo thanks for the hint, going to close it end i will send a pull request for ApplyVariantSelector

@rdeioris rdeioris closed this Oct 29, 2024
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.

2 participants