From 7929cf80b1a682ab1a25bf422ae3d6afd3867dc1 Mon Sep 17 00:00:00 2001
From: sunyab <sunyab@users.noreply.github.com>
Date: Wed, 16 Oct 2024 09:12:28 -0700
Subject: [PATCH] pcp: Disable incremental changes for layer operations by
 default

Flip PCP_ENABLE_MINIMAL_CHANGES_FOR_LAYER_OPERATIONS to off
by default while we continue development. In the meantime,
this can be reenabled for testing/experimentation by setting
this environment variable to 1.

(Internal change: 2344776)
---
 pxr/usd/pcp/changes.cpp                       |  2 +-
 pxr/usd/pcp/testenv/testPcpChanges.py         | 12 ++++++--
 .../testenv/testPcpExpressionComposition.py   | 30 +++++++++++++++----
 3 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/pxr/usd/pcp/changes.cpp b/pxr/usd/pcp/changes.cpp
index 0ee2c19e30..56db6b26a0 100644
--- a/pxr/usd/pcp/changes.cpp
+++ b/pxr/usd/pcp/changes.cpp
@@ -26,7 +26,7 @@
 PXR_NAMESPACE_OPEN_SCOPE
 
 TF_DEFINE_ENV_SETTING(
-    PCP_ENABLE_MINIMAL_CHANGES_FOR_LAYER_OPERATIONS, true,
+    PCP_ENABLE_MINIMAL_CHANGES_FOR_LAYER_OPERATIONS, false,
     "If enabled, pcp will compute a minimal amount of targeted change entries "
     "for layer operations. This can result in a significant performance "
     "improvement when muting/unmuting layer or adding/removing sublayers.");
diff --git a/pxr/usd/pcp/testenv/testPcpChanges.py b/pxr/usd/pcp/testenv/testPcpChanges.py
index a003950b40..41ba6a5125 100644
--- a/pxr/usd/pcp/testenv/testPcpChanges.py
+++ b/pxr/usd/pcp/testenv/testPcpChanges.py
@@ -9,6 +9,9 @@
 import unittest
 from contextlib import contextmanager
 
+INCREMENTAL_CHANGES = Tf.GetEnvSetting(
+    'PCP_ENABLE_MINIMAL_CHANGES_FOR_LAYER_OPERATIONS')
+
 class TestPcpChanges(unittest.TestCase):
     def test_EmptySublayerChanges(self):
         subLayer1 = Sdf.Layer.CreateAnonymous()
@@ -153,8 +156,13 @@ def "B"
 
             # With incremental changes these changes should only cause a resync
             # of /A and /B.
-            self.assertEqual(cp.GetSignificantChanges(), 
-                             [Sdf.Path('/A'), Sdf.Path('/B')])
+            if INCREMENTAL_CHANGES:
+                self.assertEqual(cp.GetSignificantChanges(), 
+                                 [Sdf.Path('/A'), Sdf.Path('/B')])
+            else:
+                self.assertEqual(cp.GetSignificantChanges(),
+                                 [Sdf.Path('/')])
+
             self.assertEqual(cp.GetSpecChanges(), [])
             self.assertEqual(cp.GetPrimChanges(), [])
 
diff --git a/pxr/usd/pcp/testenv/testPcpExpressionComposition.py b/pxr/usd/pcp/testenv/testPcpExpressionComposition.py
index 93d724928d..4810511eff 100644
--- a/pxr/usd/pcp/testenv/testPcpExpressionComposition.py
+++ b/pxr/usd/pcp/testenv/testPcpExpressionComposition.py
@@ -7,7 +7,10 @@
 
 import unittest
 
-from pxr import Pcp, Sdf
+from pxr import Pcp, Sdf, Tf
+
+INCREMENTAL_CHANGES = Tf.GetEnvSetting(
+    'PCP_ENABLE_MINIMAL_CHANGES_FOR_LAYER_OPERATIONS')
 
 def LoadPcpCache(rootLayer, sessionLayer = None):
     l = Sdf.Layer.FindOrOpen(rootLayer)
@@ -208,7 +211,12 @@ def test_SublayerAuthoring(self):
         # Since B.sdf is not empty, this should incur a significant resync.
         with Pcp._TestChangeProcessor(pcpCache) as changes:
             rootLayer.subLayerPaths.append('`"./B.sdf"`')
-            self.assertEqual(changes.GetSignificantChanges(), ['/Test'])
+
+            if INCREMENTAL_CHANGES:
+                self.assertEqual(changes.GetSignificantChanges(), ['/Test'])
+            else:
+                self.assertEqual(changes.GetSignificantChanges(), ['/'])
+
             self.assertEqual(changes.GetSpecChanges(), [])
 
         pi, err = pcpCache.ComputePrimIndex('/Test')
@@ -232,7 +240,12 @@ def test_SublayerAuthoring(self):
         # Remove the sublayer we just added to reverse the changes.
         with Pcp._TestChangeProcessor(pcpCache) as changes:
             del rootLayer.subLayerPaths[-1]
-            self.assertEqual(changes.GetSignificantChanges(), ['/Test'])
+
+            if INCREMENTAL_CHANGES:
+                self.assertEqual(changes.GetSignificantChanges(), ['/Test'])
+            else:
+                self.assertEqual(changes.GetSignificantChanges(), ['/'])
+
             self.assertEqual(changes.GetSpecChanges(), [])
 
         pi, err = pcpCache.ComputePrimIndex('/Test')
@@ -319,7 +332,11 @@ def test_SublayerAuthoringAndVariableChange(self):
                 rootLayer.expressionVariables = {'X':'B'}
                 rootLayer.subLayerPaths.append('`"./${X}.sdf"`')
 
-            self.assertEqual(changes.GetSignificantChanges(), ['/BaseRef'])
+            if INCREMENTAL_CHANGES:
+                self.assertEqual(changes.GetSignificantChanges(), ['/BaseRef'])
+            else:
+                self.assertEqual(changes.GetSignificantChanges(), ['/'])
+
             self.assertEqual(changes.GetSpecChanges(), [])
 
         pi, err = pcpCache.ComputePrimIndex('/BaseRef')
@@ -1017,7 +1034,10 @@ def test_ExpressionVarChanges_SignificantLayerStackChanges(self):
                 rootLayer.expressionVariables = {'A':'C'}
                 rootLayer.subLayerPaths.append('sig_changes/sub.sdf')
 
-            self.assertEqual(changes.GetSignificantChanges(), ['/Dummy'])
+            if INCREMENTAL_CHANGES:
+                self.assertEqual(changes.GetSignificantChanges(), ['/Dummy'])
+            else:
+                self.assertEqual(changes.GetSignificantChanges(), ['/'])
 
         self.AssertVariables(
             pcpCache, '/Root1',