Skip to content

Commit

Permalink
Allow saving a muted SdfLayer.
Browse files Browse the repository at this point in the history
There appears to be no reason not to allow saving a muted layer.
It was most likely not allowed due to the implementation, which
sets the layer's _data to an empty data object while muted.  If
we saved that we'd get an empty layer.  So now we swap the real
data in for the save and back out again if the layer is muted.

This fixes a few places that either prevented saving a muted
layer or unmuted, saved, and re-muted.  We no longer need to do
any of that.

Note that you still can't edit a muted layer.  That would be bad
given the current implementation.  An alternative implementation
would be to not compose muted layers.  That feature already exists
in Pcp but there's a significant semantic difference:  it will not
include muted layers in any layer stack, unlike the SdfLayer
approach which leaves muted layers in the layer stack but makes it
so they have no opinions.  The Pcp approach drops any refPtr to the
muted layers, likely causing them to be destroyed.  Dropping muted
layers from the layer stack and possibly destroying them violate
our current expectations.

(Internal change: 2354751)
  • Loading branch information
pixar-oss committed Jan 28, 2025
1 parent aee9525 commit 03825bb
Showing 1 changed file with 75 additions and 7 deletions.
82 changes: 75 additions & 7 deletions pxr/usd/sdf/layer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2142,7 +2142,6 @@ SdfLayer::PermissionToSave() const
{
return _permissionToSave &&
!IsAnonymous() &&
!IsMuted() &&
Sdf_CanWriteLayerToPath(GetResolvedPath());
}

Expand Down Expand Up @@ -3910,6 +3909,13 @@ SdfLayer::_SetData(const SdfAbstractDataPtr &newData,
// Guard against setting an empty SdfData, which is invalid.
TF_VERIFY(!newData->IsEmpty() );

// XXX -- Should this be disallowed when the layer is muted? We
// should at least put the new data in _mutedLayerData which,
// incidentally, won't have an entry for this layer if the
// layer was clean when muted. In that case we should also
// not send any change notifications since the content was
// and remains muted.

// This code below performs a series of specific edits to mutate _data
// to match newData. This approach provides fine-grained change
// notification, which allows more efficient invalidation in clients
Expand Down Expand Up @@ -4953,6 +4959,71 @@ SdfLayer::_WriteToFile(const string &newFileName,
return false;
}

// Helper class for RAII and unmuting for save. If the layer is muted
// swap in the unmuted data in the c'tor and back out in the d'tor
// without any notification or change processing.
class _TemporaryUnmuter
{
public:
_TemporaryUnmuter(const std::string& mutedPath,
const SdfAbstractDataRefPtr* data)
: _mutedPath(mutedPath)
, _data(const_cast<SdfAbstractDataRefPtr*>(data))
, _wasMuted(false)
{
std::unique_lock<std::mutex> lock(*_mutedLayersMutex);
if (const auto i = _mutedLayerData->find(_mutedPath);
i != _mutedLayerData->end()) {
// Install the unmuted data.
_wasMuted = true;
_savedData = *_data;
*_data = i->second;
}
}

~_TemporaryUnmuter()
{
Unlock();
}

_TemporaryUnmuter(const _TemporaryUnmuter&) = delete;
_TemporaryUnmuter(_TemporaryUnmuter&&) = delete;
_TemporaryUnmuter& operator=(const _TemporaryUnmuter&) = delete;
_TemporaryUnmuter& operator=(_TemporaryUnmuter&&) = delete;

void Unlock()
{
if (_wasMuted) {
std::unique_lock<std::mutex> lock(*_mutedLayersMutex);
if (const auto i = _mutedLayerData->find(_mutedPath);
i != _mutedLayerData->end()) {
if (*_data == i->second) {
// This is the expected case.
}
else {
TF_CODING_ERROR("Layer data modified during save");
}
}
else {
TF_CODING_ERROR("Layer unmuted during save");
}
*_data = _savedData;
_savedData = TfNullPtr;
_wasMuted = false;
}
}

private:
std::string _mutedPath;
SdfAbstractDataRefPtr* _data;
SdfAbstractDataRefPtr _savedData;
bool _wasMuted;
};

// If the layer is muted then restore the contents temporarily while
// we save.
_TemporaryUnmuter unmuter(_GetMutedPath(), &_data);

// If the output file format has a different schema, then transfer content
// to an in-memory layer first just to validate schema compatibility.
const bool differentSchema = &fileFormat->GetSchema() != &GetSchema();
Expand All @@ -4977,6 +5048,9 @@ SdfLayer::_WriteToFile(const string &newFileName,
? fileFormat->SaveToFile(*this, newFileName, comment, args)
: fileFormat->WriteToFile(*this, newFileName, comment, args);

// Restore the muted data if necessary.
unmuter.Unlock();

// If we wrote to the backing file then we're now clean.
if (ok && isSave) {
_MarkCurrentStateAsClean();
Expand Down Expand Up @@ -5011,12 +5085,6 @@ SdfLayer::_Save(bool force) const
{
TRACE_FUNCTION();

if (IsMuted()) {
TF_CODING_ERROR("Cannot save muted layer @%s@",
GetIdentifier().c_str());
return false;
}

if (IsAnonymous()) {
TF_CODING_ERROR("Cannot save anonymous layer @%s@",
GetIdentifier().c_str());
Expand Down

0 comments on commit 03825bb

Please sign in to comment.