New Import UI feature - WIP #206

seando-adsk · 2020-01-16T21:03:45Z

MAYA-101325 - Implement Qt Model base class that supports/exposes a USD stage

MAYA-101326 - Implement filtering proxy on top of USD stage model

MAYA-101566 - Hook up the new Import pop-up dialog options to import command

MAYA-101565 - Add VariantSets to import dialog and allow selection of a Variant

Added new build flag --qt-location with corresponding define QT_LOCATION.
Building of Qt UI is optional based on 'CMAKE_WANT_QT_BUILD' (default ON) and QT_LOCATION being passed in.
Added optional SUBDIR arg when promoting files.
Created a USD ImportUI dialog from internal design.
Added usd/ui folder with new ImportUI dialog.
From the mayaUsd plugin register a new command to open the ImportUI dialog.
Added the mayaUsd translator import options based on design specs.
New import data, which supports:
** Root prim path.
** Stage population mask for importing partial usd file.
** Stage load set.
** Variant sets associated with root prim and invididual prims.
** Added clearData flag to usdImportDialog command.
** Merged variants into single column.
** Added ItemDelegate for customized widget creation for Variants. Use persistent editor with label/combobox for variant selection.
** Moved tree item creation logic from factory to tree item.
** Tree checking & stage population mask.
** New tree header with checkbox for load column - not hooked up yet.
** Commented out all the usd import options (for now) as none of them are implemented yet.

…SD stage MAYA-101326 - Implement filtering proxy on top of USD stage model MAYA-101566 - Hook up the new Import pop-up dialog options to import command MAYA-101565 - Add VariantSets to import dialog and allow selection of a Variant * Added new build flag --qt-location with corresponding define QT_LOCATION. * Building of Qt UI is optional based on 'CMAKE_WANT_QT_BUILD' (default ON) and QT_LOCATION being passed in. * Added optional SUBDIR arg when promoting files. * Created a USD ImportUI dialog from internal design. * Added usd/ui folder with new ImportUI dialog. * From the mayaUsd plugin register a new command to open the ImportUI dialog. * Added the mayaUsd translator import options based on design specs. * New import data, which supports: ** Root prim path. ** Stage population mask for importing partial usd file. ** Stage load set. ** Variant sets associated with root prim and invididual prims. ** Added clearData flag to usdImportDialog command. ** Merged variants into single column. ** Added ItemDelegate for customized widget creation for Variants. Use persistent editor with label/combobox for variant selection. ** Moved tree item creation logic from factory to tree item. ** Tree checking & stage population mask. ** New tree header with checkbox for load column - not hooked up yet. ** Commented out all the usd import options (for now) as none of them are implemented yet.

seando-adsk · 2020-01-16T21:08:02Z

build.py

@@ -371,6 +375,9 @@ def RunTests(context,extraArgs):
 parser.add_argument("--debug-python", dest="debug_python", action="store_true",
                      help="Define Boost Python Debug if your Python library comes with Debugging symbols (default: %(default)s).")

+parser.add_argument("--qt-location", type=str,
+                    help="Directory where Qt is installed.")
+


To build the new UI (which uses) Qt you need to specify where to find Qt (should be the same one that Maya is using).

seando-adsk · 2020-01-16T21:08:28Z

cmake/modules/FindMaya.cmake

@@ -169,19 +168,6 @@ find_path(MAYA_INCLUDE_DIR
        "Maya's headers path"
 )

-find_path(MAYA_LIBRARY_DIR


This is not needed. It is duplicated from above in each platform specific section.

seando-adsk · 2020-01-16T21:09:05Z

lib/fileio/importData.cpp

@@ -0,0 +1,146 @@
+//


New file containing all the import data, such as root prim path, stage pop mask, variants, etc.

seando-adsk · 2020-01-16T21:09:52Z

lib/fileio/jobs/readJob.cpp

@@ -60,14 +60,12 @@ PXR_NAMESPACE_OPEN_SCOPE

 UsdMaya_ReadJob::UsdMaya_ReadJob(
        const std::string &iFileName,
-        const std::string &iPrimPath,
-        const std::map<std::string, std::string>& iVariants,
+        const MayaUsd::ImportData &iImportData,


You'll see a few changes like this. I've replaced the primPath and variants arg, with my new import data (which contains both those and more).

Shouldn't iFileName be lumped in with the import data also then?

Yes good idea. I've moved it into import data.

seando-adsk · 2020-01-16T21:10:16Z

lib/fileio/jobs/readJob.cpp

@@ -103,7 +102,22 @@ UsdMaya_ReadJob::Read(std::vector<MDagPath>* addedDagPaths)

    // Layer and Stage used to Read in the USD file
    UsdStageCacheContext stageCacheContext(UsdMayaStageCache::Get());
-    UsdStageRefPtr stage = UsdStage::Open(rootLayer, sessionLayer);
+    UsdStageRefPtr stage;
+    if (mImportData.empty())


Change to how we open the stage depending on what the import data contains.

Following up on comments above, I'm not convinced the import data or the file name should ever be empty here. If we made it this far, then we must have a filename from the SdfLayer::FindOrOpen() above.

I think this could/should just be:

if (mImportData.hasPopulationMask()) { stage = UsdStage::OpenMasked( rootLayer, sessionLayer, mImportData.stagePopulationMask(), mImportData.stageInitialLoadSet()); } else { stage = UsdStage::Open( rootLayer, sessionLayer, mImportData.stageInitialLoadSet()); }

The default load set for UsdStage::Open...() is LoadAll, so that hasn't changed here.

Yes correct, now that I've moved the filename into the import data it won't be empty. I've changed the stage open as you've suggested.

seando-adsk · 2020-01-16T21:10:52Z

lib/fileio/jobs/readJob.cpp

+    // Set the variants on all the import data prims.
+    for (auto& varPrim : mImportData.primVariantSelections()) {
+        for(auto& variant : varPrim.second) {
+            UsdPrim usdVarPrim = stage->GetPrimAtPath(varPrim.first);
+            usdVarPrim
+                .GetVariantSet(variant.first)
+                .SetVariantSelection(variant.second);
+        }
+    }


New loop to set all the individual prim variants.

Also following up from above, if root and non-root prim variant selections were unified, we'd only need the latter loop here.

seando-adsk · 2020-01-16T21:11:39Z

lib/usd/CMakeLists.txt

+if(CMAKE_WANT_QT_BUILD)
+    if(DEFINED QT_LOCATION)
+        message(STATUS "Building with Qt UI features enabled.")
+        add_subdirectory(ui)
+    else()
+        message(STATUS "QT_LOCATION not set. Building UI features will be disabled.")
+    endif()
+endif()


When the new cmake option is on (enabled by default) and we got a QT_LOCATION passed to build, we'll add the new "ui" subdir.

Should this be more strongly enforced, and/or moved to the top-level CMakeLists.txt? Would it ever be meaningful to enable CMAKE_WANT_QT_BUILD and not specify a QT_LOCATION? Maybe that should be an error instead?

I had originally added CMAKE_WANT_QT_BUILD to make it consistent with UFE, but they aren't really the same. We don't need this option, so I've removed it. I've moved the find_package(Qt5) to the top-level cmake and here I will include the folder "ui" only when Qt5_FOUND is true.

seando-adsk · 2020-01-16T21:11:59Z

lib/usd/ui/api.h

@@ -0,0 +1,47 @@
+//


New api file - similar to the one from lib folder.

seando-adsk · 2020-01-16T21:13:40Z

lib/usd/ui/CMakeLists.txt

+if (BUILD_TESTS)
+	# A simple executable used to test the import dialog.
+	add_subdirectory(demo)
+endif()


This builds a standalone executable that opens the import ui dialog. Useful during development.

seando-adsk · 2020-01-16T21:14:14Z

lib/usd/ui/cmds/USDImportDialogCmd.cpp

+	MStatus st;
+	MArgParser argData(syntax(), args, &st);
+
+	if(argData.isFlagSet(kClearDataFlag))


The clear data flag is called by the import dialog when you click on a new file.

I'm still unsure about this. Seems like the import dialog should just own an instance of ImportData rather than manipulating a singleton.

The reason this is a singleton and not owned by the import dialog is because the import dialog is created by the "Hierarchy View" button in the plugin import translator options (plugin\adsk\scripts\mayaUsdTranslatorImport.mel). That dialog can be opened by the user and then they set the prims they want to import and the variants. When the dialog is closed it will write to the importData and then the dialog is destroyed. We feel this data is much to heavy to be set via the "query" action of the translator import options.

seando-adsk · 2020-01-16T21:14:33Z

lib/usd/ui/cmds/USDImportDialogCmd.cpp

+	syntax.enableQuery(false);
+	syntax.enableEdit(false);
+	syntax.addFlag(kClearDataFlag, kClearDataFlagLong);
+	syntax.setObjectType(MSyntax::kStringObjects, 1, 1);


The command expects a file path to the USD file.

seando-adsk · 2020-01-16T21:15:07Z

lib/usd/ui/demo/CMakeLists.txt

+add_executable(${PROJECT_NAME}
+	testMayaUsdUI.cpp
+)


Simple test application to open the dialog. So I can test as standalone without Maya.

seando-adsk · 2020-01-16T21:16:33Z

lib/usd/ui/views/ItemDelegate.cpp

@@ -0,0 +1,260 @@
+//


Item delegate used to customize the drawing of the variant columns.

seando-adsk · 2020-01-16T21:18:13Z

lib/usd/ui/views/TreeHeader.cpp

@@ -0,0 +1,76 @@
+//


To have a checkbox in the header.

seando-adsk · 2020-01-16T21:19:11Z

lib/usd/ui/views/TreeItem.h

+ * displayed to the User when interacting with Tree content.
+ */
+class MAYAUSD_UI_PUBLIC TreeItem : public QStandardItem
+{


Note: there is no Q_OBJECT here as QStandardItem does not derive from QObject. So there cannot be signals/slots in this class.

seando-adsk · 2020-01-16T21:19:48Z

lib/usd/ui/views/TreeModel.cpp

+	emit dataChanged(rMinIndex, rMaxIndex);
+}
+
+void TreeModel::fillStagePopulationMask(UsdStagePopulationMask& popMask, const QModelIndex& parent)


Helper to fill the stage pop mask depending on what is checked.

seando-adsk · 2020-01-16T21:20:38Z

lib/usd/ui/views/TreeModel.cpp

+	return ParentClass::setData(index, value, role);
+}
+
+void TreeModel::setParentsCheckState(const QModelIndex &child, Qt::CheckState state)


This method (and next one) are used to check/uncheck items parent/child items when the user interactively checks/unchecks an item in the tree.

seando-adsk · 2020-01-16T21:22:45Z

lib/usd/ui/views/TreeModel.cpp

+	}
+}
+
+void TreeModel::openPersistentEditors(QTreeView* tv, const QModelIndex& parent)


Helper called after tree is created to create all the variant set widgets on each row (that has variant sets).

In Qt terms this is called a persistent editor. Meaning it's always there, rather than only being created when you click in a cell.

seando-adsk · 2020-01-16T21:23:21Z

lib/usd/ui/views/USDImportDialog.cpp

+	fUI->findPrev->setHidden(true);
+	fUI->findNext->setHidden(true);
+	fUI->findAll->setHidden(true);


WIP - I hid these buttons for now as they aren't implemented yet.

I think it would be preferable for work-in-progress functionality to not be included in a PR. When the functionality is ready, a PR dedicated to adding the new buttons and the code that drives them becomes easier to review, as does this PR.

Yes okay. I will created a separate commit where I remove all the WIP stuff.

seando-adsk · 2020-01-16T21:24:09Z

lib/usd/ui/views/USDImportDialog.ui

@@ -0,0 +1,278 @@
+<?xml version="1.0" encoding="UTF-8"?>


Created as output from Qt designer.

seando-adsk · 2020-01-16T21:24:50Z

plugin/adsk/plugin/CMakeLists.txt

+if(CMAKE_WANT_QT_BUILD AND DEFINED QT_LOCATION)
+    target_link_libraries(${PLUGIN_PACKAGE}
+        PRIVATE
+            mayaUsdUI


This plugin needs the new UI lib to register the command.

seando-adsk · 2020-01-16T21:25:23Z

plugin/adsk/scripts/mayaUsdTranslatorImport.mel

+        rowLayout -numberOfColumns 3 -cw 1 $cw1 -cat 1 "right" 0;
+            text -label "Scope and Variants: " -al "right";
+            textField -ed false -nbg true -text "Please select a file to enable this option." -w $cw2 mayaUsdTranslator_SelectFileField;
+            button -label "View Hierarchy" -c "string $usdFile = currentFileDialogSelection(); usdImportDialog $usdFile;" mayaUsdTranslator_ViewHierBtn;


The button for opening dialog.

seando-adsk · 2020-01-16T21:25:57Z

plugin/adsk/scripts/mayaUsdTranslatorImport.mel

+//        checkBoxGrp -label "Load Payloads: " -cw 1 $cw1 mayaUsdTranslator_LoadPayloadsCheckBox;
+//
+//        optionMenuGrp -label "Coordinate System: " -cw 1 $cw1 mayaUsdTranslator_CoordSystemOptionMenu;
+//            menuItem -label "Local space";
+//            menuItem -label "Object space";
+//            menuItem -label "World space";
+//        optionMenuGrp -e -sl 1 mayaUsdTranslator_CoordSystemOptionMenu;
+//
+//        optionMenuGrp -label "Materials: " -cw 1 $cw1 mayaUsdTranslator_MaterialsOptionMenu;
+//            menuItem -label "None";
+//            menuItem -label "Display Colors";
+//            menuItem -label "USD Shade";
+//        optionMenuGrp -e -sl 2 mayaUsdTranslator_MaterialsOptionMenu;
+//
+//        checkBoxGrp -label "InstancedPrims: " -label1 "Convert to Instances" -cw 1 $cw1 mayaUsdTranslator_InstancedPrimsCheckBox;
+//
+//        optionMenuGrp -label "Include Metadata: " -cw 1 $cw1 mayaUsdTranslator_IncludeMetadataOptionMenu;
+//            menuItem "All";
+//            menuItem "None";
+//            menuItem "Custom";
+//        optionMenuGrp -e -sl 3 mayaUsdTranslator_IncludeMetadataOptionMenu;
+//
+//        textFieldGrp -label "Specify Fields: " -cw 1 $cw2 -cw 2 150 mayaUsdTranslator_SpecifyFields;
+//
+//        textFieldGrp -label "Prefix Name: " -cw 1 $cw2 -cw 2 50 mayaUsdTranslator_PrefixName;
+//
+//        optionMenuGrp -label "Include Custom Attributes: " -cw 1 $cw1 mayaUsdTranslator_IncludeCustomAttribOptionMenu;
+//            menuItem "All";
+//            menuItem "None";
+//            menuItem "Custom";
+//        optionMenuGrp -e -sl 2 mayaUsdTranslator_IncludeCustomAttribOptionMenu;
+//
+//        checkBoxGrp -label "Animation Data: " -cw 1 $cw1 mayaUsdTranslator_AnimDataCheckBox;
+//
+//        intFieldGrp -label "Custom Frame Range Start/End: " -nf 2 -cw 1 $cw2 mayaUsdTranslator_CustomFrameRange;


The rest of the options area here (which I showed at demo). But are all commented for now as they aren't implemented yet.

seando-adsk · 2020-01-16T21:26:14Z

plugin/adsk/scripts/mayaUsdTranslatorImport.mel

    } else if ($action == "query") {
        string $currentOptions = "";

        eval($resultCallback+" \""+$currentOptions+"\"");

+    } else if ($action == "fileselected") {


This new section is the callback when user selects something in import dialog. I use it to hide/show the button.

seando-adsk · 2020-01-16T21:37:21Z

@murphyeoin Would anyone at Animal Logic be interested in reviewing this?

@mattyjams I modified the Pixar plugin, so I included you.

@elrond79 A lot of new Qt code, would you like to review?

* WIP - disable some widgets as they aren't full implemented yet. * Rename importer from "mayaUsdImport" to "USD Import". * Added "Animation Data" and "Custom Frame Range" options to USD Import.

mattyjams

I think there are a few things here that should be addressed first, so I left some notes inline.

Some general notes:

I like the idea of ImportData, but I'm not sold on there being a global singleton instance of it. Also, if it's meant to encapsulate everything about an import, we should use its filename property too and stop passing that as a separate argument for the functions that now take an ImportData parameter.
I think we may want to discourage the use of assert, since failed asserts will kill the process.
The whitespace/indentation level/tab usage seems inconsistent in some files.

mattyjams · 2020-01-29T01:42:09Z

cmake/modules/FindMaya.cmake

@@ -6,12 +6,12 @@
 # Variables that will be defined:


The changes in this file all look fine, but I don't think they're directly related to this new feature, are they? This would be a great candidate for its own pull request if that's the case.

No in the end they are not. I had originally made some changes to this file that were relevant for my changes, but then discovered a better way (it was for Qt). I will revert these changes and they will be done in a separate PR (from Hamed) where he is doing some cmake cleanup.

mattyjams · 2020-01-29T01:45:06Z

cmake/utils.cmake

+#
+# mayaUsd_promoteHeaderList(
+#                        [SUBDIR <optional sub-directory>])
+#                        [FILES <list of files>]
+#
+#   SUBDIR        - optional sub-directory in which to promote files.
+#   FILES         - list of files to promote.
+#
 function(mayaUsd_promoteHeaderList)
-    foreach(header ${ARGV})
+    cmake_parse_arguments(PREFIX 
+        ""          # options
+        "SUBDIR"    # one_value keywords
+        "HEADERS"   # multi_value keywords
+        ${ARGN}
+    )
+
+    set(DEST_DIR ${CMAKE_BINARY_DIR}/include/mayaUsd)
+    if(PREFIX_SUBDIR)
+        set(DEST_DIR ${DEST_DIR}/${PREFIX_SUBDIR})
+    endif()
+
+     if(PREFIX_HEADERS)
+        set(headerFiles ${PREFIX_HEADERS})
+    else()
+        message(FATAL_ERROR "HEADERS keyword is not specified.")
+    endif()
+
+    foreach(header ${headerFiles})
        set(srcFile ${CMAKE_CURRENT_SOURCE_DIR}/${header})
-        set(dstFile ${CMAKE_BINARY_DIR}/include/mayaUsd/${header})
+        set(dstFile ${DEST_DIR}/${header})


The changes to these CMake functions and their existing usages would also be nice to see as a dedicated pull request. The new feature only adds one new usage of mayaUsd_promoteHeaderList() that takes advantage of the change, but many existing call sites (unrelated to the feature) have to change to absorb this.

@mattyjams great point. I am going to have a separate PR addressing all that after this PR is merged into dev.

Matt, you are right. But in this case this is one of our backlog branch issues which was waiting for PR154 to go in.

mattyjams · 2020-01-29T02:02:40Z

lib/CMakeLists.txt

@@ -49,6 +49,7 @@ list(APPEND mayaUsd_src
    #
    base/debugCodes.cpp
    #
+    fileio/importData.cpp


Should go just before fileio/instancedNodeWriter.cpp to keep these in order.

mattyjams · 2020-01-29T02:02:51Z

lib/CMakeLists.txt

@@ -237,6 +238,7 @@ list(APPEND mayaUsdBase_headers
 )

 list(APPEND mayaUsdFileio_headers
+    fileio/importData.h


Should go just before fileio/instancedNodeWriter.h to keep these in order.

mattyjams · 2020-01-29T02:04:12Z

lib/fileio/importData.cpp

+	fRootVariants.clear();
+	fPrimVariants.clear();
+	fFilename.clear();
+	fRootPrimPath.clear();


If a default-constructed ImportData has / as its root path, should this be setting fRootPrimPath to that instead of clearing it?

Yes you are correct. Good catch. I've fixed it.

mattyjams · 2020-01-29T22:48:02Z

plugin/adsk/plugin/importTranslator.cpp

-    UsdMaya_ReadJob mUsdReadJob(fileName, primPath, variants, jobArgs);
+    UsdMaya_ReadJob mUsdReadJob(
+        fileName,
+        MayaUsd::ImportData::instance(),


Seems like we should be constructing an ImportData from the arguments rather than pulling from a singleton.

This is the only case where we want to pull from the singleton as this is the translator that has the "Hierarchy View" button which opens the import dialog. I've modified it to where it will use an empty import data if the filenames don't match. Otherwise we need the import data as that is where the variant selections and stage population mask are set.

mattyjams · 2020-01-29T22:48:45Z

plugin/adsk/scripts/mayaUsdTranslatorImport.mel

+//        checkBoxGrp -label "Load Payloads: " -cw 1 $cw1 mayaUsdTranslator_LoadPayloadsCheckBox;
+//
+//        optionMenuGrp -label "Coordinate System: " -cw 1 $cw1 mayaUsdTranslator_CoordSystemOptionMenu;
+//            menuItem -label "Local space";
+//            menuItem -label "Object space";
+//            menuItem -label "World space";
+//        optionMenuGrp -e -sl 1 mayaUsdTranslator_CoordSystemOptionMenu;
+//
+//        optionMenuGrp -label "Materials: " -cw 1 $cw1 mayaUsdTranslator_MaterialsOptionMenu;
+//            menuItem -label "None";
+//            menuItem -label "Display Colors";
+//            menuItem -label "USD Shade";
+//        optionMenuGrp -e -sl 2 mayaUsdTranslator_MaterialsOptionMenu;
+//
+//        checkBoxGrp -label "InstancedPrims: " -label1 "Convert to Instances" -cw 1 $cw1 mayaUsdTranslator_InstancedPrimsCheckBox;
+//
+//        optionMenuGrp -label "Include Metadata: " -cw 1 $cw1 mayaUsdTranslator_IncludeMetadataOptionMenu;
+//            menuItem "All";
+//            menuItem "None";
+//            menuItem "Custom";
+//        optionMenuGrp -e -sl 3 mayaUsdTranslator_IncludeMetadataOptionMenu;
+//
+//        textFieldGrp -label "Specify Fields: " -cw 1 $cw2 -cw 2 150 mayaUsdTranslator_SpecifyFields;
+//
+//        textFieldGrp -label "Prefix Name: " -cw 1 $cw2 -cw 2 50 mayaUsdTranslator_PrefixName;
+//
+//        optionMenuGrp -label "Include Custom Attributes: " -cw 1 $cw1 mayaUsdTranslator_IncludeCustomAttribOptionMenu;
+//            menuItem "All";
+//            menuItem "None";
+//            menuItem "Custom";
+//        optionMenuGrp -e -sl 2 mayaUsdTranslator_IncludeCustomAttribOptionMenu;


Remove all of this?

Yes okay. I will created a separate commit where I remove all the WIP stuff.

mattyjams · 2020-01-29T22:49:54Z

plugin/pxr/maya/lib/usdMaya/importCommand.cpp

@@ -201,7 +201,11 @@ UsdMayaImportCommand::doIt(const MArgList & args)
                /* importWithProxyShapes = */ false,
                timeInterval);

-    mUsdReadJob = new UsdMaya_ReadJobWithSceneAssembly(mFileName, mPrimPath, mVariants, jobArgs);
+    MayaUsd::ImportData importData;
+    importData.setRootVariantSelections(std::move(mVariants));


Add mFileName to the importData too?

Yes agreed - done.

mattyjams · 2020-01-29T22:50:36Z

plugin/pxr/maya/lib/usdMaya/importTranslator.cpp

@@ -111,8 +109,9 @@ UsdMayaImportTranslator::reader(
            userArgs,
            /* importWithProxyShapes = */ false,
            timeInterval);
+    MayaUsd::ImportData importData;


Set the filename on the importData?

Yes agreed - done.

mattyjams · 2020-01-29T22:51:14Z

plugin/pxr/maya/lib/usdMaya/referenceAssembly.cpp

@@ -1530,9 +1530,13 @@ bool UsdMayaRepresentationHierBase::activate()
            UsdMayaJobImportArgs::CreateFromDictionary(
                userArgs, shouldImportWithProxies,
                GfInterval::GetFullInterval());
+
+    MayaUsd::ImportData importData;
+    importData.setRootVariantSelections(std::move(variantSetSelections));


Add usdFilePath to importData?

Yes agreed - done.

seando-adsk

I'll push two commits with changes: 1) will contain code review comment changes and 2) will remove all the WIP code.

seando-adsk · 2020-01-31T21:18:24Z

lib/usd/ui/views/USDImportDialog.cpp

+
+MAYAUSD_NS_DEF {
+
+IUSDImportView::~IUSDImportView() { }


It's defined as pure in the header (=0) yet we still need an implementation because the derived class invokes the destructor and without it we have an undefined symbol.

seando-adsk · 2020-02-03T15:33:22Z

lib/usd/ui/views/USDImportDialog.cpp

+std::string USDImportDialog::filename() const
+{
+	return fFilename;
+}
+
+std::string USDImportDialog::rootPrimPath() const


seando-adsk · 2020-02-03T19:48:37Z

cmake/utils.cmake

+#
+# mayaUsd_promoteHeaderList(
+#                        [SUBDIR <optional sub-directory>])
+#                        [FILES <list of files>]
+#
+#   SUBDIR        - optional sub-directory in which to promote files.
+#   FILES         - list of files to promote.
+#
 function(mayaUsd_promoteHeaderList)
-    foreach(header ${ARGV})
+    cmake_parse_arguments(PREFIX 
+        ""          # options
+        "SUBDIR"    # one_value keywords
+        "HEADERS"   # multi_value keywords
+        ${ARGN}
+    )
+
+    set(DEST_DIR ${CMAKE_BINARY_DIR}/include/mayaUsd)
+    if(PREFIX_SUBDIR)
+        set(DEST_DIR ${DEST_DIR}/${PREFIX_SUBDIR})
+    endif()
+
+     if(PREFIX_HEADERS)
+        set(headerFiles ${PREFIX_HEADERS})
+    else()
+        message(FATAL_ERROR "HEADERS keyword is not specified.")
+    endif()
+
+    foreach(header ${headerFiles})
        set(srcFile ${CMAKE_CURRENT_SOURCE_DIR}/${header})
-        set(dstFile ${CMAKE_BINARY_DIR}/include/mayaUsd/${header})
+        set(dstFile ${DEST_DIR}/${header})


Matt, you are right. But in this case this is one of our backlog branch issues which was waiting for PR154 to go in.

seando-adsk · 2020-02-03T19:50:37Z

lib/fileio/importData.h

+
+#pragma once
+
+#include "../base/api.h"


We have an open discussion about include directives. Once we get to the agreement, we will address all of them.

seando-adsk · 2020-02-03T19:52:59Z

lib/fileio/importData.h

+/*!
+ *  \brief Singleton class to hold USD UI import data.
+*/
+class MAYAUSD_CORE_PUBLIC ImportData


On Windows by exporting the class all the methods are exported.

seando-adsk · 2020-02-05T15:02:04Z

lib/fileio/importData.cpp

+std::string ImportData::rootPrimPath() const
+{
+	return fRootPrimPath;
+}


Agreed. Done.

seando-adsk · 2020-02-05T15:21:14Z

lib/fileio/importData.cpp

+
+void ImportData::setStagePopulationMask(UsdStagePopulationMask&& mask)
+{
+	static_assert(std::is_move_assignable<UsdStagePopulationMask>::value, "UsdStagePopulationMask is not move enabled");


Yeah your right technically they aren't needed. It was just for safety sake. I've removed them.

seando-adsk · 2020-02-05T15:49:56Z

lib/fileio/importData.cpp

+const ImportData::VariantSelections& ImportData::rootVariantSelections() const
+{
+	return fRootVariants;
+}
+
+const ImportData::PrimVariantSelections& ImportData::primVariantSelections() const
+{
+	return fPrimVariants;
+}


The import/readjob code was already using the concept of root prim variants selections. These were being passed in as an arg to the readjob, etc. I added a new feature to have variant selections on any prim. I wanted to keep these separate.

seando-adsk · 2020-02-05T15:55:21Z

lib/fileio/jobs/readJob.cpp

@@ -60,14 +60,12 @@ PXR_NAMESPACE_OPEN_SCOPE

 UsdMaya_ReadJob::UsdMaya_ReadJob(
        const std::string &iFileName,
-        const std::string &iPrimPath,
-        const std::map<std::string, std::string>& iVariants,
+        const MayaUsd::ImportData &iImportData,


Yes good idea. I've moved it into import data.

seando-adsk · 2020-02-05T16:00:11Z

lib/fileio/jobs/readJob.cpp

@@ -103,7 +102,22 @@ UsdMaya_ReadJob::Read(std::vector<MDagPath>* addedDagPaths)

    // Layer and Stage used to Read in the USD file
    UsdStageCacheContext stageCacheContext(UsdMayaStageCache::Get());
-    UsdStageRefPtr stage = UsdStage::Open(rootLayer, sessionLayer);
+    UsdStageRefPtr stage;
+    if (mImportData.empty())


Yes correct, now that I've moved the filename into the import data it won't be empty. I've changed the stage open as you've suggested.

MAYA-101325 - Implement Qt Model base class that supports/exposes a USD stage MAYA-101326 - Implement filtering proxy on top of USD stage model MAYA-101566 - Hook up the new Import pop-up dialog options to import command MAYA-101565 - Add VariantSets to import dialog and allow selection of a Variant Code Review comments: * Moved find_package(Qt5) to top-level cmake. * Sorted cmake file lists * Made some return values const& * Moved filename input arg from readjob/translator into import data. * Replaced ImportData::VariantSelections typedef with SdfVariantSelectionMap * USDImportDialogCmd - use default constructor/destructor * Replaced TreeModelFactory::SdfPathHash struct with SdfPath::Hash

…SD stage MAYA-101326 - Implement filtering proxy on top of USD stage model MAYA-101566 - Hook up the new Import pop-up dialog options to import command MAYA-101565 - Add VariantSets to import dialog and allow selection of a Variant * Removing all work-in-progress and non-used or not fully implemented code.

seando-adsk · 2020-02-05T19:49:59Z

@mattyjams Matt, I've pushed two commits. The first one (1115a2e) addresses all your code review comments. The 2nd (fac4479) removes all the work-in-progress. I've replied to all your comments. Thanks for having a look at this pull-request. I know it was large, so I really appreciate your time and input.

to this PR.

kxl-adsk · 2020-02-07T18:21:08Z

@mattyjams were requested changes made? if so, please approve the PR.

mattyjams

Cool! There were just two CMakeLists.txt files where I noticed we still had policy statements, but other than that, looks good to me!

mattyjams · 2020-02-07T20:00:38Z

lib/fileio/importData.cpp

+bool ImportData::empty() const
+{
+	// If we don't have a filename set then we are empty.
+	return fFilename.empty();
+}


I could still imagine cases where you might want to import from an open stage as opposed to a file, but I don't think we have any existing usage like that currently. Seems like we have what we need for now.

mattyjams · 2020-02-07T20:02:20Z

lib/usd/ui/CMakeLists.txt

+if (POLICY CMP0074)
+    cmake_policy(SET CMP0074 OLD)
+endif()


Still have a policy requirement here?

Thanks Matt. I remember adding this a while ago to remove some cmake warnings. Since we've now updated cmake min version I'll check these.

FYI, I looked into these policy statements and I don't think they are needed anymore. There were three places where we were setting 0074 to OLD. I removed all of them and didn't see any cmake warnings. I'll make sure these get removed in a future PR.

mattyjams · 2020-02-07T20:04:05Z

lib/usd/ui/demo/CMakeLists.txt

+if (POLICY CMP0074)
+    cmake_policy(SET CMP0074 OLD)
+endif()


Still have a policy requirement here?

seando-adsk added the enhancement New feature or request label Jan 16, 2020

seando-adsk commented Jan 16, 2020

View reviewed changes

lib/usd/ui/api.h

@@ -0,0 +1,47 @@

//

Copy link

Collaborator Author

seando-adsk Jan 16, 2020

Choose a reason for hiding this comment

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

New api file - similar to the one from lib folder.

seando-adsk commented Jan 16, 2020

View reviewed changes

lib/usd/ui/views/TreeHeader.cpp Outdated

@@ -0,0 +1,76 @@

//

Copy link

Collaborator Author

seando-adsk Jan 16, 2020

Choose a reason for hiding this comment

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

To have a checkbox in the header.

seando-adsk commented Jan 16, 2020

View reviewed changes

seando-adsk requested review from mattyjams and pmolodo January 16, 2020 21:27

Fixing mistake in testing (demo) code with header path.

6af359c

seando-adsk added 2 commits January 17, 2020 15:59

Fixing linking problem with ui demo application.

991b68e

MAYA-103070 - Minimal Import UI for PR112

503dbcf

* WIP - disable some widgets as they aren't full implemented yet. * Rename importer from "mayaUsdImport" to "USD Import". * Added "Animation Data" and "Custom Frame Range" options to USD Import.

mattyjams suggested changes Jan 29, 2020

View reviewed changes

seando-adsk commented Feb 5, 2020

View reviewed changes

seando-adsk added 2 commits February 5, 2020 12:03

seando-adsk added 2 commits February 5, 2020 15:22

Forgot to commit these two files as part of the code review changes.

b5e84bb

Removing changes to FindMaya.cmake as it is not related

189497c

to this PR.

kxl-adsk removed the request for review from pmolodo February 7, 2020 18:20

mattyjams approved these changes Feb 7, 2020

View reviewed changes

Merge branch 'dev' into donnels/MAYA-101325/new_import_ui_feature_wip

3446485

kxl-adsk merged commit a792378 into dev Feb 10, 2020

kxl-adsk deleted the donnels/MAYA-101325/new_import_ui_feature_wip branch February 10, 2020 20:26

mattyjams mentioned this pull request Feb 11, 2020

identify Qt 5.12 as the minimum required version for Qt-dependent features #272

Merged

boudrey mentioned this pull request Feb 24, 2022

MAYA-120860 Fix TestALUSDMayaPython_LayerManager random fails #2140

Merged


		MAYAUSD_NS_DEF {

		IUSDImportView::~IUSDImportView() { }


		#pragma once

		#include "../base/api.h"

New Import UI feature - WIP #206

New Import UI feature - WIP #206

Conversation

seando-adsk commented Jan 16, 2020

MAYA-101325 - Implement Qt Model base class that supports/exposes a USD stage

MAYA-101326 - Implement filtering proxy on top of USD stage model

MAYA-101566 - Hook up the new Import pop-up dialog options to import command

MAYA-101565 - Add VariantSets to import dialog and allow selection of a Variant

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seando-adsk Jan 16, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seando-adsk commented Jan 16, 2020

mattyjams left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seando-adsk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seando-adsk Jan 16, 2020 •

edited

Loading