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

EMSUSD-1005 export materials without meshes #3785

Merged
merged 1 commit into from
May 27, 2024

Conversation

pierrebai-adsk
Copy link
Collaborator

  • Add the full list of Maya objects that were selected in the export job export arguments.
  • Pass the full object list when creating the UsdMayaJobExportArgs in the base export command, prim updater manager and export translator.
  • In specific material exporter, don't skip if the assignments are empty, since we are now exporting materials independently of meshes.
  • Avoid errors when filling the default prim UI since some objects in the selections might not be DAG objects.
  • Don't disable exporting materials UI when meshes are disabled since they are now independent.
  • Add isExportingMeshes, isExportingCameras and isExportingLights functions to UsdMayaJobExportArgs.
  • Use them instead of custom code during export.
  • When exporting selected, only include materials that are selected or used by a mesh.
  • When exporting all and mesh export is enabled, only export materials used by meshes.
  • When exporting all and mesh export is disabled, export all materials.
  • Test exporting materials with and without meshes.
  • Test exporting selection with and without meshes.

- Add the full list of Maya objects that were selected in the export job export arguments.
- Pass the full object list when creating the UsdMayaJobExportArgs in the base export command, prim updater manager and export translator.
- In specific material exporter, don't skip if the assignments are empty, since we are now exporting materials independently of meshes.
- Avoid errors when filling the default prim UI since some objects in the selections might not be DAG objects.
- Don't disable exporting materials UI when meshes are disabled since they are now independent.
- Add isExportingMeshes, isExportingCameras and isExportingLights functions to UsdMayaJobExportArgs.
- Use them instead of custom code during export.
- When exporting selected, only include materials that are selected or used by a mesh.
- When exporting all and mesh export is enabled, only export materials used by meshes.
- When exporting all and mesh export is disabled, export all materials.
- Test exporting materials with and without meshes.
- Test exporting selection with and without meshes.
@pierrebai-adsk pierrebai-adsk added adsk Related to Autodesk plugin import-export Related to Import and/or Export labels May 24, 2024
@pierrebai-adsk pierrebai-adsk self-assigned this May 24, 2024
@AramAzhari-adsk
Copy link
Collaborator

@pierrebai-adsk Does this remove the need to address EMSUSD-753 ? I see that it was closed even though it was meant to remove any nodes that are left with no connections. If that ticket isn't affected by this PR then I think we should ask the team to re-open it.

@pierrebai-adsk
Copy link
Collaborator Author

No, I doubt it has any consequence related to that.

@pierrebai-adsk pierrebai-adsk added the ready-for-merge Development process is finished, PR is ready for merge label May 25, 2024
@admsev
Copy link

admsev commented May 26, 2024

@gooroodev please review

@gooroodev
Copy link

Thanks for the tag, @admsev!

Summary of Changes

This pull request introduces the ability to export materials from Maya to USD without exporting meshes. The key changes include:

  1. Enhancements to UsdMayaJobExportArgs:

    • Added a new member fullObjectList to store the full list of objects initially requested for export.
    • Added helper methods to check if meshes, cameras, or lights are excluded from export.
  2. Modifications in Export Logic:

    • Updated various parts of the code to handle the new fullObjectList and to ensure that materials can be exported even if meshes are excluded.
    • Adjusted the logic in shading mode exporters to correctly handle the new export conditions.
  3. Update to MEL and Python Scripts:

    • Modified the MEL script mayaUsdTranslatorExport.mel to properly enable or disable UI elements based on the export settings.
    • Updated the Python helper script mayaUsd_exportHelpers.py to correctly handle exclusion of different node types.
  4. New Test Case:

    • Added a new test script testUsdExportMaterialsOnly.py to validate the functionality of exporting materials without meshes.

Issues, Bugs, or Typos

  1. Typo in Log Message:

    • In exportTranslator.cpp, the log message has a typo: "Nohting to export. Skipping."
      TF_WARN("Nohting to export. Skipping.");
      Proposed Fix:
      TF_WARN("Nothing to export. Skipping.");
  2. Inconsistent Function Naming:

    • The function _getRelatives in mayaUsd_exportHelpers.py is defined but not used correctly. It should be _getRelatives(item) instead of _getRelatives(i).
      def _getRelatives(item):
          listInfo = cmds.listRelatives(i, fullPath=True)
      Proposed Fix:
      def _getRelatives(item):
          listInfo = cmds.listRelatives(item, fullPath=True)

General Review of Code Quality and Style

  1. Code Quality:

    • The code is generally well-structured and follows good practices. The addition of helper methods and encapsulation of logic within functions improves readability and maintainability.
  2. Code Style:

    • The code adheres to standard coding conventions. Variable names are descriptive, and the use of constants for token values is appropriate.
  3. Documentation and Comments:

    • The code includes comments that explain the purpose of key functions and changes, which is helpful for understanding the modifications.
  4. Testing:

    • The addition of a comprehensive test script (testUsdExportMaterialsOnly.py) is a positive aspect, ensuring that the new functionality is well-tested.

Proposed Improvements

  1. Refactor shouldExportMaterial Function:

    • The shouldExportMaterial function has nested conditions that can be simplified for better readability.
      Current Code:
      if (exportArgs.exportSelected) {
          if (assignmentsToBind.size() == 0) {
              if (!exportArgs.fullObjectList.hasItem(surfaceShader)) {
                  return false;
              }
          }
      } else {
          if (exportArgs.isExportingMeshes()) {
              return assignmentsToBind.size() > 0;
          }
      }
      Proposed Improvement:
      if (exportArgs.exportSelected && assignmentsToBind.empty() && !exportArgs.fullObjectList.hasItem(surfaceShader)) {
          return false;
      }
      if (!exportArgs.exportSelected && exportArgs.isExportingMeshes() && assignmentsToBind.empty()) {
          return false;
      }
  2. Consistent Use of Braces:

    • Ensure consistent use of braces for all conditional statements to improve readability and maintainability.
      Current Code:
      for (size_t i = 0; i < count; ++i)
          if (args.excludeExportTypes.count(tokens[i]) > 0)
              return true;
      return false;
      Proposed Improvement:
      for (size_t i = 0; i < count; ++i) {
          if (args.excludeExportTypes.count(tokens[i]) > 0) {
              return true;
          }
      }
      return false;

Overall, the pull request is well-implemented and adds valuable functionality to the project. The proposed improvements and fixes are minor and aimed at enhancing readability and consistency.

Yours, Gooroo.dev. React or reply to let me know what you think!

@pierrebai-adsk
Copy link
Collaborator Author

About the automatic review:

  • It is right about _getRelative(),, but by happy accident, the value passed is always the variable i from the same scope, so the function uses the correct value. I have a follow-up PR that will do the suggested fix.
    • I will also fix the Nohting typo in that PR.

@seando-adsk seando-adsk removed the adsk Related to Autodesk plugin label May 27, 2024
@seando-adsk seando-adsk merged commit 23732b5 into dev May 27, 2024
11 checks passed
@seando-adsk seando-adsk deleted the bailp/EMSUSD-1005/export-materials-only branch May 27, 2024 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
import-export Related to Import and/or Export ready-for-merge Development process is finished, PR is ready for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants