Merge branch 'master' into careminster

3 files changed, 59 insertions, 1 deletions

diff --git a/OpenSim/Region/Framework/Scenes/SceneGraph.cs b/OpenSim/Region/Framework/Scenes/SceneGraph.cs
index cd4bd42..0a1b921 100644
--- a/OpenSim/Region/Framework/Scenes/SceneGraph.cs
+++ b/OpenSim/Region/Framework/Scenes/SceneGraph.cs
@@ -101,6 +101,9 @@ namespace OpenSim.Region.Framework.Scenes
         /// </summary>
         protected internal Dictionary<uint, SceneObjectGroup> SceneObjectGroupsByLocalPartID = new Dictionary<uint, SceneObjectGroup>();        
 
+        /// <summary>
+        /// Lock to prevent object group update, linking and delinking operations from running concurrently.
+        /// </summary>
         private Object m_updateLock = new Object();
 
         #endregion
diff --git a/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs b/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs
index 6f3a084..7bd6c11 100644
--- a/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs
+++ b/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs
@@ -2584,6 +2584,10 @@ namespace OpenSim.Region.Framework.Scenes
         /// <summary>
         /// Link the prims in a given group to this group
         /// </summary>
+        /// <remarks>
+        /// Do not call this method directly - use Scene.LinkObjects() instead to avoid races between threads.
+        /// FIXME: There are places where scripts call these methods directly without locking.  This is a potential race condition.
+        /// </remarks>
         /// <param name="objectGroup">The group of prims which should be linked to this group</param>
         public void LinkToGroup(SceneObjectGroup objectGroup)
         {
@@ -2714,6 +2718,11 @@ namespace OpenSim.Region.Framework.Scenes
         /// Delink the given prim from this group.  The delinked prim is established as
         /// an independent SceneObjectGroup.
         /// </summary>
+        /// <remarks>
+        /// FIXME: This method should not be called directly since it bypasses update locking, allowing a potential race
+        /// condition.  But currently there is no
+        /// alternative method that does take a lonk to delink a single prim.
+        /// </remarks>
         /// <param name="partID"></param>
         /// <returns>The object group of the newly delinked prim.  Null if part could not be found</returns>
         public SceneObjectGroup DelinkFromGroup(uint partID)
@@ -2725,6 +2734,11 @@ namespace OpenSim.Region.Framework.Scenes
         /// Delink the given prim from this group.  The delinked prim is established as
         /// an independent SceneObjectGroup.
         /// </summary>
+        /// <remarks>
+        /// FIXME: This method should not be called directly since it bypasses update locking, allowing a potential race
+        /// condition.  But currently there is no
+        /// alternative method that does take a lonk to delink a single prim.
+        /// </remarks>
         /// <param name="partID"></param>
         /// <param name="sendEvents"></param>
         /// <returns>The object group of the newly delinked prim.  Null if part could not be found</returns>
@@ -2750,6 +2764,11 @@ namespace OpenSim.Region.Framework.Scenes
         /// Delink the given prim from this group.  The delinked prim is established as
         /// an independent SceneObjectGroup.
         /// </summary>
+        /// <remarks>
+        /// FIXME: This method should not be called directly since it bypasses update locking, allowing a potential race
+        /// condition.  But currently there is no
+        /// alternative method that does take a lonk to delink a single prim.
+        /// </remarks>
         /// <param name="partID"></param>
         /// <param name="sendEvents"></param>
         /// <returns>The object group of the newly delinked prim.</returns>
diff --git a/OpenSim/Region/Framework/Scenes/Tests/SceneObjectSpatialTests.cs b/OpenSim/Region/Framework/Scenes/Tests/SceneObjectSpatialTests.cs
index fffa3bd..9fea3c6 100644
--- a/OpenSim/Region/Framework/Scenes/Tests/SceneObjectSpatialTests.cs
+++ b/OpenSim/Region/Framework/Scenes/Tests/SceneObjectSpatialTests.cs
@@ -111,8 +111,44 @@ namespace OpenSim.Region.Framework.Scenes.Tests
             Assert.That(childPart.GetWorldPosition(), Is.EqualTo(childPosition));
             Assert.That(childPart.RelativePosition, Is.EqualTo(childOffsetPosition));
             Assert.That(childPart.OffsetPosition, Is.EqualTo(childOffsetPosition));
+        }
+
+        [Test]
+        public void TestGetChildPartPositionAfterObjectRotation()
+        {
+            TestHelpers.InMethod();
+
+            Vector3 rootPartPosition = new Vector3(10, 20, 30);
+            Vector3 childOffsetPosition = new Vector3(2, 3, 4);
+
+            SceneObjectGroup so
+                = SceneHelpers.CreateSceneObject(2, m_ownerId, "obj1", 0x10);
+            so.AbsolutePosition = rootPartPosition;
+            so.Parts[1].OffsetPosition = childOffsetPosition;
+
+            m_scene.AddNewSceneObject(so, false);
+
+            so.UpdateGroupRotationR(Quaternion.CreateFromEulers(0, 0, -90 * Utils.DEG_TO_RAD));
+
+            // Calculate child absolute position.
+            Vector3 rotatedChildOffsetPosition
+                = new Vector3(childOffsetPosition.Y, -childOffsetPosition.X, childOffsetPosition.Z);
+
+            Vector3 childPosition = new Vector3(rootPartPosition + rotatedChildOffsetPosition);
+
+            SceneObjectPart childPart = so.Parts[1];
 
-            // TODO: Write test for child part position after rotation.
+            // FIXME: Should be childPosition after rotation?
+            Assert.That(childPart.AbsolutePosition, Is.EqualTo(rootPartPosition + childOffsetPosition));
+
+            Assert.That(childPart.GroupPosition, Is.EqualTo(rootPartPosition));
+            Assert.That(childPart.GetWorldPosition(), Is.EqualTo(childPosition));
+
+            // Relative to root part as (0, 0, 0)
+            Assert.That(childPart.RelativePosition, Is.EqualTo(childOffsetPosition));
+
+            // Relative to root part as (0, 0, 0)
+            Assert.That(childPart.OffsetPosition, Is.EqualTo(childOffsetPosition));
         }
     }
 }
\ No newline at end of file