MAYA-104215: improve renaming restriction on prim objects #475

HamedSabri-adsk · 2020-04-29T21:20:37Z

PR Changes:

This PR brings new improvements to rename command and adds new restrictions:

Reworked rename command implementation.
Only allow edits on the target layer. error out if the object is not defined on the target layer.
Error out if object to rename has any definitions or overs on any stronger or weaker layers. A Reference to a weaker layer should be nameable if it meets all other criteria here.
Error out if the object to be renamed has been referenced by another layer. Internal references (references without a file path specified) from the same file, should be nameable.
Created utilities for common USD operations under lib\usd\utils\util.h

Unit tests:

Previous testRename is renamed to testRenameUndo.
Added 3 additional tests for newly added restriction logic.

testRename
testRenameUndo
testRenameRestrictionSameLayerDef
testRenameRestrictionOtherLayerOpinions
testRenameRestrictionExternalReference

…arget layer errors out: Unable to rename [object name] on current target layer, please set [Layername] as target layer to proceed.

HamedSabri-adsk · 2020-04-29T21:42:03Z

lib/mayaUsd/ufe/UsdUndoRenameCommand.cpp

+                                         prim.GetName().GetString(), 
+                                         possibleTargetLayer->GetDisplayName());
+        throw std::runtime_error(err.c_str());
+    }


We check here if the target layer has any opinions that affects selected prim. If it doesn't pass the check, we then find the possible target layer and append it's display name to the error message. Finally we throw a runtime_error message.

HamedSabri-adsk · 2020-04-29T21:44:05Z

lib/mayaUsd/ufe/UsdUndoRenameCommand.h

+    UsdSceneItem::Ptr m_fUfeSrcItem;
+	SdfPath m_fUsdSrcPath;
+	UsdSceneItem::Ptr m_fUfeDstItem;
+	SdfPath m_fUsdDstPath;


Following the C++ guideline, member variables need to be prefixed with m_

Actually, we use _ prefix - see https://github.com/Autodesk/maya-usd/blob/dev/doc/codingGuidelines.md#naming-file-type-variable-constant-function-namespace-macro-template-parameters-schema-names

New modules should start using these guidelines, but we still have to plan how to adapt the existing code.

My bad, I was sill thinking about "m_" :) Fixed in 54166d2

HamedSabri-adsk · 2020-04-29T21:44:25Z

lib/mayaUsd/ufe/Utils.cpp

@@ -109,8 +109,6 @@ SdfLayerHandle defPrimSpecLayer(const UsdPrim& prim)

 	SdfLayerHandle defLayer;
 	auto layerStack = prim.GetStage()->GetLayerStack();
-    auto stage = prim.GetStage();
-    auto primFromPath = stage->GetPrimAtPath(prim.GetPath());


remove unused variables.

HamedSabri-adsk · 2020-04-29T21:48:00Z

lib/mayaUsd/ufe/Utils.cpp

+    }
+
+    return true;
+}


To find out if a specific layer contains any opinions that affect a particular prim, we can use the Sdf's GetPrimAtPath() for that prim. If the primSpec result returns nothing that means the target layer doesn't contain any opinions on a particular prim.

HamedSabri-adsk · 2020-04-29T21:50:50Z

lib/mayaUsd/ufe/Utils.cpp

+    }
+    return targetLayer;
+}
+


This function is used to give the user a hint to what target layer to set in order to rename a particular prim.

lib/mayaUsd/ufe/Utils.h

ppt-adsk · 2020-04-30T14:10:58Z

lib/mayaUsd/ufe/Utils.cpp

@@ -124,6 +122,38 @@ SdfLayerHandle defPrimSpecLayer(const UsdPrim& prim)
 	return defLayer;
 }

+bool isTargetLayerHaveOpinion(const UsdPrim& prim)


Would read better as
doesTargetLayerHaveOpinion()
or just plain
targetLayerHasOpinion()
(shorter is often better).

ppt-adsk · 2020-04-30T14:19:58Z

lib/mayaUsd/ufe/Utils.cpp

+
+    // to know whether the target layer contains any opinions that 
+    // affect a particular prim, there must be a primSpec for that prim
+    if (!primSpec) {


Nit-pick: if SdfPrimSpec supports an
explicit operator bool()
(looks like it does, but couldn't find it)
could be
return bool(primSpec);

Good point @ppt-adsk but I don't find any "explicit operator bool" both in SdfSpec and SdfPrimSpec

ppt-adsk · 2020-04-30T15:05:44Z

lib/mayaUsd/ufe/Utils.cpp

+    return true;
+}
+
+SdfLayerHandle targetLayerWithOpion(const UsdPrim& prim)


Typo: "Opion" --> "Opinion".
Also, isn't it the case that this will return the highest-priority layer that has an opinion, either def or over? It would be good if the function would say that, e.g.
SdfLayerHandle strongestLayerWithOpinion(const UsdPrim& prim)
or
SdfLayerHandle strongestLayerWithPrimSpec(const UsdPrim& prim)

A minor point is that the existing function in the Utils file is called defPrimSpecLayer, i.e. uses "prim spec" rather than "opinion". Perhaps we should standardize on one terminology or the other.

Please see 54166d2

I agree with standardization on one terminology, I picked "prim spec" instead of "opinion" for now.
Regarding the function names, I made following changes and dropped the target. Let me know if this reads better:

Before:

isTargetLayerHaveOpinion targetLayerWithOpion

After:

doesLayerHavePrimSpec strongestLayerWithPrimSpec

lib/mayaUsd/ufe/Utils.h

ppt-adsk · 2020-04-30T15:17:24Z

lib/mayaUsd/ufe/Utils.h

@@ -56,6 +56,14 @@ bool isRootChild(const Ufe::Path& path);
 MAYAUSD_CORE_PUBLIC
 SdfLayerHandle defPrimSpecLayer(const UsdPrim& prim);

+//! Check if the target layer has any opinions that affects a particular prim
+MAYAUSD_CORE_PUBLIC
+bool isTargetLayerHaveOpinion(const UsdPrim& prim);


I feel the naming of these two functions is confusing, because they both talk about "target layer", but they're not related.
"isTargetLayerHaveOpinion" is really answering the question: does the edit target layer on the prim's stage have an opinion?
"targetLayerWithOpion" [sic] is answering the question: what is the strongest layer that has an opinion for this prim? From the point of view of this function, there is no target involved, and therefore I would remove the word "target" here.

Please see 54166d2

I agree. I made following changes and dropped the target. Let me know if this reads better:

Before:

isTargetLayerHaveOpinion

After:

doesLayerHavePrimSpec

"doesLayerHavePrimSpec" answers this question: Does the layer that you are currently on have any primSpec (a.k.a opinion) on the selected prim?

ppt-adsk · 2020-04-30T15:27:14Z

lib/mayaUsd/ufe/UsdUndoRenameCommand.cpp

 		std::string err = TfStringPrintf("No prim found at %s", prim.GetPath().GetString().c_str());
 		throw std::runtime_error(err.c_str());
 	}
+
+    // check if the target layer has any opinions that affects selected prim
+    if (!isTargetLayerHaveOpinion(prim)) {


So here if the edit target doesn't have an opinion, we tell the user to set the edit target to the strongest layer than does have an opinion. O.K., maybe that makes sense, even if the strongest layer only has "over" opinion.

But what if the edit target is on a weak layer and the weak layer DOES have an opinion, that is overridden by an opinion in a stronger layer? We're saying that's O.K.? The user will rename in the weak layer, but the stronger layer will still have its opinion and it will be composed into the final result, no? That doesn't seem to meet Will's acceptance criteria that "Trying to rename an object with opinions on stronger layers error out." Perhaps what you're showing here is really work in progress and you're going to fix this? Or is there something I haven't understood?

Very valid question and I should have elaborated more on the change here:

if (!MayaUsdUtils::doesLayerHavePrimSpec(prim)) solves this request:

Trying to rename an object that was not defined on the target layer errors out

but as you also mentioned it doesn't really solve this request:

Trying to rename an object with opinions on stronger layers error out.

I am working on addressing this as we speak.

@ppt-adsk please see 8f8295f.

- renamed usd utility functions related to "usd" and "sdf" operations - moved usd utility functions from mayaUsd\ufe\Utils.h to usd\utils\util.h under MayaUsdUtils namespace. - cleaned up variable names according to c++ guideline.

HamedSabri-adsk · 2020-05-01T11:48:26Z

lib/usd/utils/util.cpp

+}
+
+SdfLayerHandle
+MayaUsdUtils::strongestLayerWithPrimSpec(const UsdPrim& prim)


extra MayaUsdUtils:: in line 64 and 48 was added by accident. I will fix this in the next PR.

Error if object to rename has ANY definitions or overs on ANY stronger or weaker layers. This is the most restrictive we can be. We will revisit how much we loosen this up later. Cannot rename a prim with definitions or opinions on other layers. Opinions exist in [layer name 1], [layer name 2], [layer name 3], [layer name 4],etc...”

ppt-adsk · 2020-05-04T13:40:05Z

lib/usd/utils/util.cpp

+}
+
+std::vector<SdfLayerHandle>
+layersWithOpinion(const UsdPrim& prim)


Naming: the other functions in this translation units use primSpec, this one should as well.

please see 4c67b69

ppt-adsk · 2020-05-04T13:41:17Z

lib/usd/utils/util.cpp

+    // ordered from strongest to weakest opinion.
+    const auto& primStack = prim.GetPrimStack();
+
+    std::vector<SdfLayerHandle> layersWithOpion;


Typo: "layersWithOpion" --> "layersWithOpinion", but of course if you rename to primSpec this will disappear.

see 4c67b69

Some how I can't type opinion properly :)

ppt-adsk · 2020-05-04T13:43:18Z

lib/usd/utils/util.cpp

+}
+
+bool
+doesLayerHavePrimSpec(const UsdPrim& prim)


I think the naming would be much clearer as
doesEditTargetLayerHavePrimSpec()
I know it's wordy, maybe you can do better, but this name (or a similar name) tells you which layer we're talking about, the edit target layer.

Good point, I like your suggested name.

see 4c67b69

ppt-adsk · 2020-05-04T13:44:48Z

lib/usd/utils/util.h

+    MAYA_USD_UTILS_PUBLIC
+    bool doesLayerHavePrimSpec(const UsdPrim&);
+
+    //! Return the layer that has any opinions on a particular prim


Would be clearer as
"Return the strongest layer that has an opinion on the argument prim."

see 4c67b69

ppt-adsk · 2020-05-04T13:57:03Z

lib/mayaUsd/ufe/UsdUndoRenameCommand.cpp

+	_ufeSrcItem = srcItem;
+	_usdSrcPath = prim.GetPath();
+
+    // Every call to rename() (through execute(), undo() or redo()) removes


Nit-pick: indentation changed.

see 4c67b69

ppt-adsk · 2020-05-04T13:58:30Z

lib/mayaUsd/ufe/UsdUndoRenameCommand.cpp

 		std::string err = TfStringPrintf("No prim found at %s", prim.GetPath().GetString().c_str());
 		throw std::runtime_error(err.c_str());
 	}
+
+    // if the current layer doesn't have any opinions that affects selected prim
+    if (!MayaUsdUtils::doesLayerHavePrimSpec(prim)) {


Isn't this just _layer != theEditTargetLayer?

Good point. Correct, we could also use _layer != prim.GetStage()->GetEditTarget().GetLayer() instead. I keep using doesEditTargetLayerHavePrimSpec which is a bit more generic. Hope this is Ok.

ppt-adsk · 2020-05-04T14:00:11Z

lib/mayaUsd/ufe/UsdUndoRenameCommand.cpp

+            for (auto layer : layers) {
+                layerNames.append("[" + layer->GetDisplayName() + "]" + ",");
+            }
+            layerNames.pop_back();


Why omit the last layer?

see 4c67b69

I rename layerNames to layerDisplayNames so it reads better.

I am not omitting any layers and rather the extra "," at the end of layerDisplayNames string.

This is the message the design wants to show up as part of the error message:

Opinions exist in [layer name 1], [layer name 2], [layer name 3], [layer name 4].”

So when I iterate through the layers, I get their GetDisplayName and concatenate them in the
layerDisplayNames string separated by ","

for (auto layer : layers) { layerDisplayNames.append("[" + layer->GetDisplayName() + "]" + ","); } layerDisplayNames.pop_back();

There are better ways of doing this, but this is one way I did.

- edited code logics in testRename to use usdCylinder.ma scene since with new rename restrictions top_layer.ma doesn't meet the requirements for testing undo/redo - refactored code around opening maya test scenes and introduced three utility functions (openCylinderScene, openTwoSpheresScene, openSphereAnimatedRadiusScene)

test/lib/ufe/testRename.py

HamedSabri-adsk · 2020-05-05T14:40:05Z

test/lib/ufe/testRename.py

+        # expect the exception happens
+        with self.assertRaises(RuntimeError):
+           newName = 'Ball_35_Renamed'
+           cmds.rename(newName)


@ppt-adsk I added two test cases for rename restriction operations:

testRenameRestriction is to test if the prim that we are about to rename is in the correct layer

testRenameRestriction2 is to test if the prim that we are about to rename doesn't have any definitions or opinions on other layers.

If you think testRenameRestriction2 is ugly, I am happy to change it with something more meaningful.

test/lib/ufe/ufeTestUtils/mayaUtils.py

ppt-adsk

Almost there! Minor tweak to just rename the test functions, and we're all done.

ppt-adsk · 2020-05-05T14:57:15Z

test/lib/ufe/testRename.py

        self.assertEqual(len(propsChildren), len(propsChildrenPre))

+    def testRenameRestriction(self):


How about "testRenameRestrictionSameLayerDef", or similar?

@ppt-adsk Merci Pierre. please see 581932b

ppt-adsk · 2020-05-05T14:58:12Z

test/lib/ufe/testRename.py

+            newName = 'Ball_35_Renamed'
+            cmds.rename(newName)
+
+    def testRenameRestriction2(self):


How about "testRenameRestrictionOtherLayerOpinions", or similar?

ppt-adsk · 2020-05-05T15:19:15Z

Great job Hamed!

tbb\concurrent_vector.h(177): fatal error C1017: invalid integer constant expression

HamedSabri-adsk · 2020-05-06T13:30:31Z

lib/mayaUsd/ufe/UsdUndoRenameCommand.cpp


-            Ufe::ObjectRename notification(_ufeDstItem, ufeSrcPath);
-            Ufe::Scene::notifyObjectPathChange(notification);
+            sendRenameNotification(_ufeDstItem, _ufeSrcItem->path());


minor refactoring here.

HamedSabri-adsk · 2020-05-06T13:30:36Z

lib/mayaUsd/ufe/UsdUndoRenameCommand.cpp

+        {
+            UsdEditContext ctx(_stage, _layer);
+            status = _stage->RemovePrim(_ufeSrcItem->prim().GetPath());
+        }


With the new restriction in place it is now safe to remove the prim from the scene graph rather than deactivating it.When RemovePrim is called, I simply pass the _ufeSrcItem->prim().GetPath() rather than creating a new srcPrim which would be really unnecessary.

Redo operation:
1- Call SdfCopySpec to copy ALL the child specs from _usdSrcPath to _usdDstPath. This is a crucial call.
2- Remove the prim at _ufeSrcItem USD path.
3- set _ufeDstItem by calling createSiblingSceneItem
4- send rename notification

HamedSabri-adsk · 2020-05-06T13:32:32Z

lib/mayaUsd/ufe/UsdUndoRenameCommand.h

-	UsdSceneItem::Ptr _ufeDstItem;
-	SdfPath _usdDstPath;
+    UsdSceneItem::Ptr _ufeDstItem;
+    SdfPath _usdDstPath;

 }; // UsdUndoRenameCommand



I have converted tabs to dots. If you find them distracting, Github allow hiding whitespace changes during review:

Please see
https://medium.com/@kevinpmcc/hide-whitespace-for-easier-github-code-reviews-d3f476d03863

HamedSabri-adsk · 2020-05-06T13:35:51Z

@ppt-adsk I have the first stab at removing the cruft in USD files as objects are named and renamed. please see 20be9ac

HamedSabri-adsk · 2020-05-06T13:54:33Z

@ppt-adsk Ah....... I am starting to think why we need to rely on SdfCopySpec any more.

I honestly think we can simply rename a prim in place. I just realized that SdfPrimSpec has a routine called SetName to set the prim name:

https://graphics.pixar.com/usd/docs/api/class_sdf_prim_spec.html#aa1f241d28f26488867a276eca6d54405

This would really simplify a lot of things. Stay tuned.....

This change simplifies the rename operation by simply renaming a prim in place via SdfPrimSpec's SetName routine and removes the old work-flow involving SdfCopySpec.

…operations in undo/redo. - To read the code better, I added a new member variable _oldName that holds the prim's original name.

…sd file for testing purposes. - Renamed testRename to testRenameUndo - Added new logics for testing new way of renaming a prim name via SdfPrimSpec.

…s around the stage behavior that deserves an explanation.

- Add a simple testRenameRestrictionExternalReference unit test.

…ternal reference and create a new utility for it.

… we do still want the opinion detection in other layers happens.

…things more readable. - change the container type for layersWithContribution method from std::vector to std::set to guarantee having unique layers.

…me_logics Improve rename logics

…ved from the prim argument.

HamedSabri-adsk · 2020-05-14T16:57:21Z

lib/mayaUsd/ufe/UsdUndoRenameCommand.cpp

+    // SdfLayer is a "simple" container, and all it knows about defaultPrim is that it is a piece of token-valued layer metadata.  
+    // It is only the higher-level Usd and Pcp modules that know that it is identifying a prim on the stage.  
+    // One must use the SdfLayer API for setting the defaultPrim when you rename the prim it identifies.
+    if(primNameStr == stageDefPrimNameStr){


@ppt-adsk We don't need to do a comparison here. if(!stageDefPrimNameStr.empty()) does the work. Will fix it in the next commit,

Hmm, I don't think that would work now that I think about it. please ignore my last comment.

@ppt-adsk please see d84d82f

…"Question around SdfPrimSepc's SetName routine" thread.

ppt-adsk

Just that minor out of date comment to remove, but otherwise looks good to me!

ppt-adsk · 2020-05-15T17:23:06Z

lib/mayaUsd/ufe/UsdUndoRenameCommand.cpp

-	// MAYA-92264: Pixar bug prevents undo from working.  Try again with USD
-	// version 0.8.5 or later.  PPT, 7-Jul-2018.
-	try {
+    // MAYA-92264: Pixar bug prevents undo from working.  Try again with USD


This comment is no longer valid, right?

MAYA-104215: Trying to rename an object that was not defined on the t…

c9100f7

…arget layer errors out: Unable to rename [object name] on current target layer, please set [Layername] as target layer to proceed.

HamedSabri-adsk commented Apr 29, 2020

View reviewed changes

lib/mayaUsd/ufe/Utils.h Show resolved Hide resolved

HamedSabri-adsk requested review from mattyjams and ppt-adsk April 29, 2020 21:53

HamedSabri-adsk added core Related to core library ufe-usd Related to UFE-USD plugin in Maya-Usd unit test Related to unit tests (both python or c++) labels Apr 29, 2020

ppt-adsk reviewed Apr 30, 2020

View reviewed changes

Address feedbacks:

54166d2

- renamed usd utility functions related to "usd" and "sdf" operations - moved usd utility functions from mayaUsd\ufe\Utils.h to usd\utils\util.h under MayaUsdUtils namespace. - cleaned up variable names according to c++ guideline.

HamedSabri-adsk commented May 1, 2020

View reviewed changes

Hamed Sabri added 2 commits May 1, 2020 15:45

Fix compiler errors.

e21a130

ppt-adsk reviewed May 4, 2020

View reviewed changes

Hamed Sabri added 2 commits May 4, 2020 11:28

Address Feedbacks: rename functions and variables to read better.

4c67b69

HamedSabri-adsk commented May 5, 2020

View reviewed changes

test/lib/ufe/testRename.py Show resolved Hide resolved

HamedSabri-adsk commented May 5, 2020

View reviewed changes

test/lib/ufe/ufeTestUtils/mayaUtils.py Show resolved Hide resolved

ppt-adsk requested changes May 5, 2020

View reviewed changes

Address feedback: rename test case names to something more meaningful.

581932b

ppt-adsk approved these changes May 5, 2020

View reviewed changes

Fix MacOS build and make sure to pass the error messages by char*

761143d

ppt-adsk approved these changes May 5, 2020

View reviewed changes

Fix debug build:

d58a028

tbb\concurrent_vector.h(177): fatal error C1017: invalid integer constant expression

HamedSabri-adsk commented May 6, 2020

View reviewed changes

Improve rename logics:

ae2a7e0

This change simplifies the rename operation by simply renaming a prim in place via SdfPrimSpec's SetName routine and removes the old work-flow involving SdfCopySpec.

HamedSabri-adsk marked this pull request as ready for review May 6, 2020 17:03

Hamed Sabri and others added 16 commits May 6, 2020 17:30

- Added early checks to make sure we are in a valid state when doing …

bb1da2e

…operations in undo/redo. - To read the code better, I added a new member variable _oldName that holds the prim's original name.

- Added tree.ma and tree.usda to the test-samples. This is a useful u…

c4f247a

…sd file for testing purposes. - Renamed testRename to testRenameUndo - Added new logics for testing new way of renaming a prim name via SdfPrimSpec.

Updated test prim. tesRename does pass but there are still some issue…

a45779c

…s around the stage behavior that deserves an explanation.

- Add new logics for handling internal vs external references.

b207895

- Add a simple testRenameRestrictionExternalReference unit test.

Eliminate the need to stores _prim and _oldName member variables.

890372d

Address feedback: Re-factored the logics for detecting internal vs ex…

a305af9

…ternal reference and create a new utility for it.

Address feedback: break out of the loop once the condition is met.

610f2ce

Address feedback: Don't return if we have an internal reference since…

80aa35e

… we do still want the opinion detection in other layers happens.

- change the wording form opinion to contribute/contribution to make …

3cf2acc

…things more readable. - change the container type for layersWithContribution method from std::vector to std::set to guarantee having unique layers.

Merge pull request #483 from Autodesk/sabrih/MAYA-104215/improve_rena…

7bb77b3

…me_logics Improve rename logics

Merge branch 'dev' into sabrih/MAYA-104215/improve_usd_rename_operation

24001e3

Fix build after merging dev branch into this PR.

7e66d41

Fix reference file path.

22d6fc9

Make sure maya scene examples are using relative path.

df3ff44

Nit-pick: No need to pass stage as an argument since it can be retrie…

67fd431

…ved from the prim argument.

Added more logics for handling defaultprim rename.

7e8bef6

HamedSabri-adsk commented May 14, 2020

View reviewed changes

Hamed Sabri added 2 commits May 14, 2020 16:44

Nit-Pick: do an object comparison instead of string.

d84d82f

For future reference, added the feedback I received from Spiff under …

5d81f6c

…"Question around SdfPrimSepc's SetName routine" thread.

ppt-adsk approved these changes May 15, 2020

View reviewed changes

Remove out of date comment. MAYA-92264 is now internally closed.

ade83fa

kxl-adsk merged commit 2ae67f5 into dev May 20, 2020

kxl-adsk deleted the sabrih/MAYA-104215/improve_usd_rename_operation branch May 20, 2020 01:34

		self.assertEqual(len(propsChildren), len(propsChildrenPre))

		def testRenameRestriction(self):

MAYA-104215: improve renaming restriction on prim objects #475

MAYA-104215: improve renaming restriction on prim objects #475

Conversation

HamedSabri-adsk commented Apr 29, 2020 • edited Loading

PR Changes:

Unit tests:

HamedSabri-adsk Apr 29, 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

HamedSabri-adsk Apr 29, 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

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

HamedSabri-adsk May 5, 2020 • edited Loading

Choose a reason for hiding this comment

ppt-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

ppt-adsk commented May 5, 2020

Choose a reason for hiding this comment

HamedSabri-adsk May 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HamedSabri-adsk commented May 6, 2020

HamedSabri-adsk commented May 6, 2020

HamedSabri-adsk May 14, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ppt-adsk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HamedSabri-adsk commented Apr 29, 2020 •

edited

Loading

HamedSabri-adsk Apr 29, 2020 •

edited

Loading

HamedSabri-adsk Apr 29, 2020 •

edited

Loading

HamedSabri-adsk May 5, 2020 •

edited

Loading

HamedSabri-adsk May 6, 2020 •

edited

Loading

HamedSabri-adsk May 14, 2020 •

edited

Loading