usd: Implement SdfFileFormat::SaveToFile in UsdUsdFileFormat and

UsdUsdcFileFormat so we can reliably detect "Export" operations and run
the output through a temporary CrateData object.  Also ensure we drop
the ArAsset instance early enough when writing to file so that in the
case of a "Save", the atomic rename will succeed on Windows.  Windows
doesn't allow you to overwrite a file that's currently opened by your
process.  Enable the testUsdFileFormats_asset test on Windows now that
this is fixed.

Fixes #2773

(Internal change: 2320168)

pxr/usd/usd/CMakeLists.txt

@@ -942,18 +942,13 @@ pxr_register_test(testUsdFileFormats
     EXPECTED_RETURN_CODE 0
 )
 
-# XXX:
-# Temporarily disable this test on Windows; the ArAsset-based
-# codepath in the .usdc file format code has issues.
-if (NOT WIN32)
-    pxr_register_test(testUsdFileFormats_asset
-        PYTHON
-        COMMAND "${CMAKE_INSTALL_PREFIX}/tests/testUsdFileFormats"
-        ENV
-            USDC_USE_ASSET=1
-        EXPECTED_RETURN_CODE 0
-    )
-endif()
+pxr_register_test(testUsdFileFormats_asset
+    PYTHON
+    COMMAND "${CMAKE_INSTALL_PREFIX}/tests/testUsdFileFormats"
+    ENV
+        USDC_USE_ASSET=1
+    EXPECTED_RETURN_CODE 0
+)
 
 pxr_register_test(testUsdFileFormats_pread
     PYTHON

pxr/usd/usd/crateData.cpp

@@ -121,10 +121,6 @@ class Usd_CrateDataImpl
 
     string const &GetAssetPath() const { return _crateFile->GetAssetPath(); }
 
-    bool CanIncrementalSave(string const &fileName) {
-        return _crateFile->CanPackTo(fileName);
-    }
-
     bool Save(string const &fileName) {
         TfAutoMallocTag tag("Usd_CrateDataImpl::Save");
 
@@ -1115,20 +1111,26 @@ Usd_CrateData::Save(string const &fileName)
         return false;
     }
 
-    if (_impl->CanIncrementalSave(fileName)) {
-        return _impl->Save(fileName);
-    }
-    else {
-        // We copy to a temporary data and save that.
-        //
-        // Usd_CrateData currently reloads the underlying asset to reinitialize
-        // its internal members after a save. We use a non-detached
-        // Usd_CrateData here to avoid any expense associated with detaching
-        // from the asset.
-        Usd_CrateData tmp(/* detached = */ false);
-        tmp.CopyFrom(SdfAbstractDataConstPtr(this));
-        return tmp.Save(fileName);
+    return _impl->Save(fileName);
+}
+
+bool
+Usd_CrateData::Export(string const &fileName)
+{
+    if (fileName.empty()) {
+        TF_CODING_ERROR("Tried to save to empty fileName");
+        return false;
     }
+
+    // To Export, we copy to a temporary data and save that, since we need this
+    // CrateData object to stay associated with its existing backing store.
+    //
+    // Usd_CrateData currently reloads the underlying asset to reinitialize its
+    // internal members after a save. We use a non-detached Usd_CrateData here
+    // to avoid any expense associated with detaching from the asset.
+    Usd_CrateData tmp(/* detached = */ false);
+    tmp.CopyFrom(SdfAbstractDataConstPtr(this));
+    return tmp.Save(fileName);
 }
 
 bool

pxr/usd/usd/crateData.h

@@ -56,6 +56,8 @@ class Usd_CrateData : public SdfAbstractData
 
     bool Save(const std::string &fileName);
 
+    bool Export(const std::string &fileName);
+
     bool Open(const std::string &assetPath,
               bool detached);
     bool Open(const std::string &assetPath, 

pxr/usd/usd/crateFile.cpp

@@ -2437,23 +2437,6 @@ CrateFile::~CrateFile()
     _DeleteValueHandlers();
 }
 
-bool
-CrateFile::CanPackTo(string const &fileName) const
-{
-    if (_assetPath.empty()) {
-        return true;
-    }
-    // Try to open \p fileName and get its filename.
-    bool result = false;
-    if (FILE *f = ArchOpenFile(fileName.c_str(), "rb")) {
-        if (ArchGetFileName(f) == _fileReadFrom) {
-            result = true;
-        }
-        fclose(f);
-    }
-    return result;
-}
-
 CrateFile::Packer
 CrateFile::StartPacking(string const &fileName)
 {
@@ -2496,6 +2479,15 @@ CrateFile::Packer::Close()
 
     // Write contents. Always close the output asset even if writing failed.
     bool writeResult = _crate->_Write();
+
+    if (writeResult) {
+        // Abandon the asset here to release resources that the subsequent call
+        // to CloseOutputAsset() might need.  E.g. on Windows,
+        // CloseOutputAsset() may try to overwrite the file that _assetSrc has
+        // open for read.
+        _crate->_assetSrc.reset();
+    }
+
     writeResult &= _crate->_packCtx->CloseOutputAsset();
 
     // If we wrote successfully, store the fileName.

pxr/usd/usd/crateFile.h

@@ -721,10 +721,6 @@ class CrateFile
         CrateFile *_crate;
     };
 
-    // Return true if this CrateFile object wasn't populated from a file, or if
-    // the given \p fileName is the file this object was populated from.
-    bool CanPackTo(string const &fileName) const;
-
     Packer StartPacking(string const &fileName);
 
     string const &GetAssetPath() const { return _assetPath; }

pxr/usd/usd/usdFileFormat.cpp

@@ -316,36 +316,37 @@ UsdUsdFileFormat::WriteToFile(
     // arguments, just use that.
     SdfFileFormatConstPtr fileFormat = _GetFileFormatForArguments(args);
 
-    // Otherwise, if we are saving a .usd layer (i.e., calling SdfLayer::Save),
-    // we want to maintain that layer's underlying format. For example,
-    // calling Save() on a text .usd file should produce a text file
-    // and not convert it to binary.
-    // 
-    // If we are exporting to a .usd layer (i.e., calling SdfLayer::Export),
-    // we use the default underlying format for .usd. This ensures consistent
-    // behavior -- creating a new .usd layer always uses the default format
-    // unless otherwise specified.
+    // When exporting to a .usd layer (i.e., calling SdfLayer::Export), we use
+    // the default underlying format for .usd. This ensures consistent behavior
+    // -- creating a new .usd layer always uses the default format unless
+    // otherwise specified.
     if (!fileFormat) {
-        // Note that SdfLayer::GetRealPath is *not* the same as realpath(3); 
-        // it does not follow symlinks. Hence, we use TfRealPath to determine 
-        // if the source and destination files are the same. If so, we know 
-        // we're saving the layer, not exporting it to a new location.
-        auto layerRealPath = 
-            TfRealPath(layer.GetRealPath(), 
-                       /* allowInaccessibleSuffix = */ true);
-        auto destRealPath = 
-            TfRealPath(filePath, /* allowInaccessibleSuffix = */ true);
-        const bool isSavingLayer = (layerRealPath == destRealPath);
-        if (isSavingLayer) {
-            fileFormat = _GetUnderlyingFileFormatForLayer(layer);
-        }
+        fileFormat = _GetDefaultFileFormat();
     }
 
+    // XXX: Is it weird that we're not passing through the \p args?
+    return fileFormat->WriteToFile(layer, filePath, comment);
+}
+
+bool
+UsdUsdFileFormat::SaveToFile(
+    const SdfLayer& layer,
+    const std::string& filePath,
+    const std::string& comment,
+    const FileFormatArguments& args) const
+{
+    // If we are saving a .usd layer (i.e., calling SdfLayer::Save), we want to
+    // maintain that layer's underlying format. For example, calling Save() on a
+    // text .usd file should produce a text file and not convert it to binary.
+    // 
+    SdfFileFormatConstPtr fileFormat = _GetUnderlyingFileFormatForLayer(layer);
+
     if (!fileFormat) {
         fileFormat = _GetDefaultFileFormat();
     }
 
-    return fileFormat->WriteToFile(layer, filePath, comment);
+    // XXX: Is it weird that we're not passing through the \p args?
+    return fileFormat->SaveToFile(layer, filePath, comment);
 }
 
 bool 

pxr/usd/usd/usdFileFormat.h

@@ -81,6 +81,13 @@ class UsdUsdFileFormat : public SdfFileFormat
         const std::string& comment = std::string(),
         const FileFormatArguments& args = FileFormatArguments()) const override;
 
+    USD_API
+    virtual bool SaveToFile(
+        const SdfLayer& layer,
+        const std::string& filePath,
+        const std::string& comment = std::string(),
+        const FileFormatArguments& args = FileFormatArguments()) const override;
+
     USD_API
     virtual bool ReadFromString(
         SdfLayer* layer,

pxr/usd/usd/usdcFileFormat.cpp

@@ -164,19 +164,40 @@ UsdUsdcFileFormat::WriteToFile(const SdfLayer& layer,
     if (auto const *constCrateData =
         dynamic_cast<Usd_CrateData const *>(get_pointer(dataSource))) {
         auto *crateData = const_cast<Usd_CrateData *>(constCrateData);
-        return crateData->Save(filePath);
+        return crateData->Export(filePath);
     }
 
     // Otherwise we're dealing with some arbitrary data object, just copy the
     // contents into the binary data.
     if (auto dataDest = 
         TfDynamic_cast<Usd_CrateDataRefPtr>(InitData(FileFormatArguments()))) {
         dataDest->CopyFrom(dataSource);
-        return dataDest->Save(filePath);
+        return dataDest->Export(filePath);
     }
     return false;
 }
 
+bool
+UsdUsdcFileFormat::SaveToFile(const SdfLayer& layer,
+                              const std::string& filePath,
+                              const std::string& comment,
+                              const FileFormatArguments& args) const
+{
+    SdfAbstractDataConstPtr dataSource = _GetLayerData(layer);
+
+    // XXX: WBN to avoid const-cast -- saving can't be non-mutating in general.
+    if (auto const *constCrateData =
+        dynamic_cast<Usd_CrateData const *>(get_pointer(dataSource))) {
+        auto *crateData = const_cast<Usd_CrateData *>(constCrateData);
+        return crateData->Save(filePath);
+    }
+
+    TF_CODING_ERROR("Called UsdUsdcFileFormat::SaveToFile with "
+                    "non-Crate-backed layer @%s@",
+                    layer.GetIdentifier().c_str());
+    return false;
+}
+
 bool 
 UsdUsdcFileFormat::ReadFromString(SdfLayer* layer,
                                   const std::string& str) const

pxr/usd/usd/usdcFileFormat.h

@@ -67,6 +67,12 @@ class UsdUsdcFileFormat : public SdfFileFormat
         const string& comment = string(),
         const FileFormatArguments& args = FileFormatArguments()) const override;
 
+    virtual bool SaveToFile(
+        const SdfLayer& layer,
+        const string& filePath,
+        const string& comment = string(),
+        const FileFormatArguments& args = FileFormatArguments()) const override;
+
     virtual bool ReadFromString(SdfLayer* layer,
                                 const string& str) const override;