-
Notifications
You must be signed in to change notification settings - Fork 214
EMSUSD-2761 - Asset Resolver Search Path preferences #4337
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
Conversation
ef1b73b to
9c85fd4
Compare
| text -label `getMayaUsdString("KArMappingFile")` -ann `getMayaUsdString("KArMappingFileAnn")`; | ||
| textField -text (`optionVar -q mayaUsd_AdskAssetResolverMappingFile`) -cc ("mayaUsdArMappingFilePathUpdate") mayaUsdArMappingFilePath; | ||
| symbolButton -image "navButtonBrowse.png" -c "mayaUsdArMappingFileBrowser()" browseBtn; | ||
| button |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way for us to get CMake variables in .mel? If AR_FOUND is false we dont want to show this UI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not directly no. You can configure the file, which I don't think we want to do here.
Another way I can think of is to add a new flag to mayaUsdInfoCommand.cpp which lives in adsk plugin. The flag could be something like "-ar" which returns true when the Asset Resolver is there. You can use the define WANT_AR_BUILD which you set above to know when to return true/false. Then in both MEL you can use the command
if `mayaUsdInfo -ar`
And in python:
if cmds.mayaUsdInfo(ar=True)
9c85fd4 to
b170ea1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only looked at the cmake and changes to our existing code. I didn't review the new AR files as I assume they were reviewed previously in the shared code.
cmake/modules/FindAR.cmake
Outdated
| message(STATUS " AR include dir is ${AR_INCLUDE_DIR}") | ||
| message(STATUS " AR library is ${AR_LIBRARY}") | ||
| else() | ||
| message(ERROR "AR not found") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You shouldn't need this error message. The line 37 above will output a cmake error if the two required vars are not set.
lib/usd/ui/CMakeLists.txt
Outdated
|
|
||
| add_subdirectory(layerEditor) | ||
| add_subdirectory(importDialog) | ||
| add_subdirectory(assetResolver) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| add_subdirectory(assetResolver) | |
| if (AR_FOUND) | |
| add_subdirectory(assetResolver) | |
| endif() |
lib/usd/ui/CMakeLists.txt
Outdated
| message(STATUS "AR include dir is ${AR_INCLUDE_DIR}") | ||
| target_include_directories(${PROJECT_NAME} PRIVATE ${AR_INCLUDE_DIR}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need the message here. Its already in your FindAR.cmake.
| message(STATUS "AR include dir is ${AR_INCLUDE_DIR}") | |
| target_include_directories(${PROJECT_NAME} PRIVATE ${AR_INCLUDE_DIR}) | |
| if (AR_FOUND) | |
| target_include_directories(${PROJECT_NAME} PRIVATE ${AR_INCLUDE_DIR}) | |
| endif() |
lib/usd/ui/CMakeLists.txt
Outdated
| Qt::Core | ||
| Qt::Gui | ||
| Qt::Widgets | ||
| ${AR_LIBRARY} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be okay even if AR_FOUND is false, since this will just be empty.
| if(NOT AR_FOUND) | ||
| return() | ||
| endif() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this as the check should be one level higher in the parent CMakeLists.txt
| #if defined(WANT_QT_BUILD) | ||
| #include <mayaUsdUI/ui/USDImportDialogCmd.h> | ||
| #include <mayaUsdUI/ui/initStringResources.h> | ||
| #include <mayaUsdUI/ui/AssetResolverDialogCmd.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the CMakeLists.txt you need to set a define when AR_FOUND is true, so that you can check it in cpp files. See how WANT_QT_BUILD is set. You could make yours WANT_AR_BUILD.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a variable called WANT_AR_BUILD
| status = MayaUsd::AssetResolverDialogCmd::initialize(plugin); | ||
| if (!status) { | ||
| MString err("registerCommand"); | ||
| err += MayaUsd::AssetResolverDialogCmd::name; | ||
| status.perror(err); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code should be wrapped in you new define.
| text -label `getMayaUsdString("KArMappingFile")` -ann `getMayaUsdString("KArMappingFileAnn")`; | ||
| textField -text (`optionVar -q mayaUsd_AdskAssetResolverMappingFile`) -cc ("mayaUsdArMappingFilePathUpdate") mayaUsdArMappingFilePath; | ||
| symbolButton -image "navButtonBrowse.png" -c "mayaUsdArMappingFileBrowser()" browseBtn; | ||
| button |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not directly no. You can configure the file, which I don't think we want to do here.
Another way I can think of is to add a new flag to mayaUsdInfoCommand.cpp which lives in adsk plugin. The flag could be something like "-ar" which returns true when the Asset Resolver is there. You can use the define WANT_AR_BUILD which you set above to know when to return true/false. Then in both MEL you can use the command
if `mayaUsdInfo -ar`
And in python:
if cmds.mayaUsdInfo(ar=True)
a6c7dd0 to
87b88ce
Compare
| const char* MayaUsdBuildInfo::buildDate() { return MAYAUSD_BUILD_DATE; } | ||
| bool MayaUsdBuildInfo::buildAR() | ||
| { | ||
| #ifdef WANT_AR_BUILD |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Declared this buildAR method to check if AdskAssetResolver is built
| PrimUpdaterManager::getInstance(); | ||
| #endif | ||
|
|
||
| #ifdef WANT_AR_BUILD |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AR related code is wrapped in WANT_AR_BUILD
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of small changes to make.
CMakeLists.txt
Outdated
|
|
||
| find_package(AR) | ||
| if (AR_FOUND) | ||
| message(STATUS "Found AdskAssetResolver") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't really need this message here since you have the message in FindAR.cmake.
| @@ -0,0 +1,123 @@ | |||
| // | |||
| // Copyright 2019 Autodesk | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this 2019?
| @@ -0,0 +1,53 @@ | |||
| # | |||
| # Copyright 2021 Autodesk | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2025?
plugin/adsk/plugin/CMakeLists.txt
Outdated
| target_compile_definitions(${TARGET_NAME} | ||
| PRIVATE | ||
| MAYAUSD_PLUGIN_EXPORT | ||
| $<$<BOOL:${AR_FOUND}>:WANT_AR_BUILD> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just add this line 61 under line 55 below.
plugin/adsk/plugin/CMakeLists.txt
Outdated
| ${UFE_INCLUDE_DIR} | ||
| ) | ||
| if (AR_FOUND) | ||
| target_include_directories(${TARGET_NAME} PRIVATE ${AR_INCLUDE_DIR}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can add this in the target_include_directories just above, using the generator expression again. Put this just after line 92.
$<$BOOL:${AR_FOUND}:${AR_INCLUDE_DIR}>
719b684 to
3df9135
Compare
3df9135 to
a015695
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks better. You can still fix plugin/adsk/plugin/CMakeLists.txt using a generator expression instead of the extra if.
401692f to
4a5b288
Compare
4a5b288 to
e226140
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good. Thanks.
No description provided.