Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep the revit element with Node when Disable Transaction #2761

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@


## 0.3.8
* Upgrade Greg, GregRevitOAuth, and RestSharp to RestSharp 106.12.0 to address a security issue.

## 0.3.7
* Add Transaction controls - "Sync with Revit" Toggle button.
* Upgrade Greg, GregRevitOAuth, and RestSharp to RestSharp 106.12.0 to address a security issue.
* Keep ElementBinding when set Transaction disabled.

## 0.3.6
* Upgrade DynamoRevit to 2.13.0.
Expand Down
6 changes: 4 additions & 2 deletions src/DynamoRevit/DynamoRevit.cs
Original file line number Diff line number Diff line change
Expand Up @@ -576,16 +576,18 @@ private static void AddSyncWithRevitControls(DynamoView dynamoView)
var runsettingStackPanel = runsettingsGrid.Children.OfType<StackPanel>().FirstOrDefault();

var srcDic = Dynamo.UI.SharedDictionaryManager.DynamoModernDictionary;
if (TransactionManager.Instance.DisableTransactions)
TransactionManager.Instance.DisableTransactions = false;

var toggleItem = new System.Windows.Controls.Primitives.ToggleButton
{
Width = 40,
Height = 20,
IsChecked = true,
IsChecked = !TransactionManager.Instance.DisableTransactions,
VerticalContentAlignment = System.Windows.VerticalAlignment.Center,
ToolTip = Resources.SyncWithRevitToolTip
};

toggleItem.SetValue(System.Windows.Controls.Primitives.ToggleButton.StyleProperty, srcDic["EllipseToggleButton1"]);

toggleItem.Click += OnReadOnlyModeToggleChecked;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Autodesk.DesignScript.Runtime;
using Autodesk.Revit.DB;
using Revit.GeometryConversion;
using RevitServices.Persistence;
using RevitServices.Transactions;
Expand Down
21 changes: 16 additions & 5 deletions src/Libraries/RevitNodes/Elements/AdaptiveComponent.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public class AdaptiveComponent : AbstractFamilyInstance
/// <param name="familyInstance"></param>
private AdaptiveComponent(Autodesk.Revit.DB.FamilyInstance familyInstance)
{
SafeInit(() => InitAdaptiveComponent(familyInstance));
SafeInit(() => InitAdaptiveComponent(familyInstance), true);
}

/// <summary>
Expand Down Expand Up @@ -64,9 +64,7 @@ private AdaptiveComponent(double[] parms, Reference c, FamilyType ft)
/// <param name="familyInstance"></param>
private void InitAdaptiveComponent(Autodesk.Revit.DB.FamilyInstance familyInstance)
{
TransactionManager.Instance.EnsureInTransaction(Document);
InternalSetFamilyInstance(familyInstance);
TransactionManager.Instance.TransactionTaskDone();
}

/// <summary>
Expand Down Expand Up @@ -511,11 +509,24 @@ private static AdaptiveComponent[] InternalByPoints(Point[][] points, FamilyType
if (oldInstances != null) countOfOldInstances = oldInstances.Count();
int reusableCount = Math.Min(countToBeCreated, countOfOldInstances);

TransactionManager.Instance.EnsureInTransaction(Document);

List<Autodesk.Revit.DB.FamilyInstance> instances = new List<Autodesk.Revit.DB.FamilyInstance>();
List<AdaptiveComponent> components = new List<AdaptiveComponent>();

if (TransactionManager.Instance.DisableTransactions)
{
if(oldInstances != null)
{
foreach (var instance in oldInstances)
{
var adpCom = new AdaptiveComponent(instance);
components.Add(adpCom);
}
return components.ToArray();
}
}

TransactionManager.Instance.EnsureInTransaction(Document);

try
{
// Reuse the adaptive components that can be reused if possible
Expand Down
6 changes: 4 additions & 2 deletions src/Libraries/RevitNodes/Elements/Ceiling.cs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,8 @@ public static Ceiling ByOutlineTypeAndLevel(Curve[] outlineCurves, CeilingType c
}

var ceiling = ByOutlineTypeAndLevel(PolyCurve.ByJoinedCurves(outlineCurves), ceilingType, level);
DocumentManager.Regenerate();
if (!TransactionManager.Instance.DisableTransactions)
DocumentManager.Regenerate();
return ceiling;
}

Expand Down Expand Up @@ -161,7 +162,8 @@ public static Ceiling ByOutlineTypeAndLevel(PolyCurve outline, CeilingType ceili
}

var ceiling = new Ceiling(loops, ceilingType.InternalCeilingType, level.InternalLevel);
DocumentManager.Regenerate();
if (!TransactionManager.Instance.DisableTransactions)
DocumentManager.Regenerate();
return ceiling;
}

Expand Down
32 changes: 29 additions & 3 deletions src/Libraries/RevitNodes/Elements/Element.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,18 @@ public abstract class Element : IDisposable, IGraphicItem, IFormattable
/// <param name="init"></param>
protected void SafeInit(Action init)
{
if(TransactionManager.Instance.DisableTransactions)
{
var elementManager = ElementIDLifecycleManager<int>.GetInstance();
var element = ElementBinder.GetElementFromTrace<Autodesk.Revit.DB.Element>(Document);

if (element != null)
{
SetInternalElement(element);
elementManager.UnRegisterAssociation(element.Id.IntegerValue, this);
return;
}
}
TransactionManager.Instance.EnsureInTransaction(DocumentManager.Instance.CurrentDBDocument);

SafeInitImpl(init);
Expand Down Expand Up @@ -205,9 +217,21 @@ public bool AreJoined(Element otherElement)
/// A reference to the element
/// </summary>
[SupressImportIntoVM]
public abstract Autodesk.Revit.DB.Element InternalElement
public virtual Autodesk.Revit.DB.Element InternalElement
{
get;
set;
}

/// <summary>
/// Set Internal Element from a exsiting element.
/// </summary>
/// <param name="element"></param>
internal void SetInternalElement(Autodesk.Revit.DB.Element element)
{
InternalElement = element;
InternalElementId = element.Id;
InternalUniqueId = element.UniqueId;
}

private ElementId internalId;
Expand Down Expand Up @@ -280,7 +304,8 @@ public virtual void Dispose()
// closing homeworkspace or the element itself is frozen.
if (DisposeLogic.IsShuttingDown || DisposeLogic.IsClosingHomeworkspace || IsFrozen)
return;

if (TransactionManager.Instance.DisableTransactions)
return;
bool didRevitDelete = ElementIDLifecycleManager<int>.GetInstance().IsRevitDeleted(Id);

var elementManager = ElementIDLifecycleManager<int>.GetInstance();
Expand All @@ -302,7 +327,8 @@ public virtual void Dispose()
throw new InvalidOperationException(string.Format(Properties.Resources.CantCloseLastOpenView, this.ToString()));
}
}
DocumentManager.Instance.DeleteElement(new ElementUUID(InternalUniqueId));
if(!TransactionManager.Instance.DisableTransactions)
DocumentManager.Instance.DeleteElement(new ElementUUID(InternalUniqueId));
}
else
{
Expand Down
6 changes: 4 additions & 2 deletions src/Libraries/RevitNodes/Elements/Floor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,8 @@ public static Floor ByOutlineTypeAndLevel(Curve[] outlineCurves, FloorType floor
}

var floor = ByOutlineTypeAndLevel(PolyCurve.ByJoinedCurves(outlineCurves), floorType, level);
DocumentManager.Regenerate();
if (!TransactionManager.Instance.DisableTransactions)
DocumentManager.Regenerate();
return floor;
}

Expand Down Expand Up @@ -173,7 +174,8 @@ public static Floor ByOutlineTypeAndLevel(PolyCurve outline, FloorType floorType
}

var floor = new Floor(loops, floorType.InternalFloorType, level.InternalLevel, offset);
DocumentManager.Regenerate();
if(!TransactionManager.Instance.DisableTransactions)
DocumentManager.Regenerate();
return floor;
}

Expand Down
15 changes: 15 additions & 0 deletions src/Libraries/RevitNodes/Elements/PathOfTravel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,21 @@ private static PathOfTravel[] InternalByViewEndPoints(Rvt.View rvtView, IEnumera
{
List<PathOfTravel> pathsOfTravel = new List<PathOfTravel>();

if(TransactionManager.Instance.DisableTransactions)
{
IEnumerable<RvtAnalysis.PathOfTravel> persistRvtElements = ElementBinder.GetElementsFromTrace<RvtAnalysis.PathOfTravel>(Document);
if(persistRvtElements != null)
{
foreach (var ele in persistRvtElements)
{
var persisEle = new PathOfTravel(ele);
pathsOfTravel.Add(persisEle);
}

return pathsOfTravel.ToArray();
}
}

TransactionManager.Instance.EnsureInTransaction(Document);

try
Expand Down
2 changes: 1 addition & 1 deletion src/Libraries/RevitNodes/Elements/RevisionCloud.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ private RevisionCloud(Autodesk.Revit.DB.View view, List<Autodesk.Revit.DB.Curve>
/// <summary>
/// Set the internal Element, ElementId, and UniqueId
/// </summary>
/// <param name="wall"></param>
/// <param name="element"></param>
private void InternalSetElement(Autodesk.Revit.DB.RevisionCloud element)
{
InternalRevitElement = element;
Expand Down
8 changes: 5 additions & 3 deletions src/Libraries/RevitNodes/Elements/Roof.cs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ private void InitRoof(CurveArray curves, Autodesk.Revit.DB.Level level, Autodesk
/// <summary>
/// Set the InternalRoof property and the associated element id and unique id
/// </summary>
/// <param name="Roof"></param>
/// <param name="roof"></param>
private void InternalSetRoof(Autodesk.Revit.DB.RoofBase roof)
{
InternalRoof = roof;
Expand Down Expand Up @@ -165,7 +165,8 @@ public static Roof ByOutlineTypeAndLevel(Curve[] outline, RoofType roofType, Lev
});

var roof = new Roof(ca, level.InternalLevel,roofType.InternalRoofType);
DocumentManager.Regenerate();
if (!TransactionManager.Instance.DisableTransactions)
DocumentManager.Regenerate();
return roof;
}

Expand Down Expand Up @@ -194,7 +195,8 @@ public static Roof ByOutlineExtrusionTypeAndLevel(PolyCurve outline, RoofType ro
});

var roof = new Roof(ca, plane.InternalReferencePlane, level.InternalLevel, roofType.InternalRoofType, extrusionStart, extrusionEnd);
DocumentManager.Regenerate();
if (!TransactionManager.Instance.DisableTransactions)
DocumentManager.Regenerate();
return roof;
}

Expand Down
2 changes: 1 addition & 1 deletion src/Libraries/RevitNodes/Elements/Room.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public override Autodesk.Revit.DB.Element InternalElement
{
get { return InternalRevitElement; }
}

#endregion

#region Private constructors
Expand Down
2 changes: 1 addition & 1 deletion src/Libraries/RevitNodes/Elements/TextElement.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public override Autodesk.Revit.DB.Element InternalElement
{
get { return InternalRevitElement; }
}

/// <summary>
/// Reference to the Element
/// </summary>
Expand Down
23 changes: 17 additions & 6 deletions src/Libraries/RevitNodes/Elements/Views/Sheet.cs
Original file line number Diff line number Diff line change
Expand Up @@ -398,12 +398,12 @@ private void InternalAddViewsToSheetView(IEnumerable<Autodesk.Revit.DB.View> vie
/// <summary>
/// Set the InternalViewSheet property and the associated element id and unique id
/// </summary>
/// <param name="floor"></param>
private void InternalSetViewSheet(Autodesk.Revit.DB.ViewSheet floor)
/// <param name="viewSheet"></param>
private void InternalSetViewSheet(Autodesk.Revit.DB.ViewSheet viewSheet)
{
this.InternalViewSheet = floor;
this.InternalElementId = floor.Id;
this.InternalUniqueId = floor.UniqueId;
this.InternalViewSheet = viewSheet;
this.InternalElementId = viewSheet.Id;
this.InternalUniqueId = viewSheet.UniqueId;
}

/// <summary>
Expand Down Expand Up @@ -737,9 +737,20 @@ public static Sheet DuplicateSheet(Sheet sheet, bool duplicateWithContents = fal

Sheet newSheet = null;

if(TransactionManager.Instance.DisableTransactions)
{
var oldSheet = ElementBinder.GetElementFromTrace<Autodesk.Revit.DB.ViewSheet>(Document);
if(oldSheet != null)
{
newSheet = new Sheet(oldSheet);

return newSheet;
}
}

try
{
RevitServices.Transactions.TransactionManager.Instance.EnsureInTransaction(Application.Document.Current.InternalDocument);
TransactionManager.Instance.EnsureInTransaction(Application.Document.Current.InternalDocument);

var oldElements = ElementBinder.GetElementsFromTrace<Autodesk.Revit.DB.Element>(Document);
List<ElementId> elementIds = new List<ElementId>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,13 @@ public int UnRegisterAssociation(T elementID, Object wrapper)
//ID already existed, check we're not over adding
if (existingWrappers.Contains(wrapper))
{
int index = existingWrappers.FindIndex((x) => object.ReferenceEquals(x, wrapper));
existingWrappers.RemoveAt(index);
var removeList = existingWrappers.FindAll((x) => object.ReferenceEquals(x, wrapper));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what case triggered this change? Could index ever be -1? (not found?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I debug disable/enable transaction, I found that existingWrappers contains multiple wrapper, but no one can be referenceEqueals.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be debugging code, I will solve this, but I think it would be better to add a judgment protection to the index.

Copy link
Collaborator

@ShengxiZhang ShengxiZhang Nov 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @ZiyunShang for the update.
I still have concern about this implementation. A lot of nodes need to check the state of DisableTransaction, which requires Dynamo developers to keep in mind that the node may run in DisableTransaction mode, and need to deal with the behavior. And it also requires Dynamo developers to be familiar with the API internal implementation, know if the API calls regen or starts Transaction internally (API can call regen itself, and UI API may start and commit transactions)
Moreover, even doing all above, this only works for the built-in nodes. It doesn't work for package nodes, python nodes, custom nodes, which are also in the AC list.
@saintentropy @QilongTang any ideas?

for (int i = 0; i < removeList.Count; i++)
{
int index = existingWrappers.FindIndex((x) => object.ReferenceEquals(x, wrapper));
existingWrappers.RemoveAt(index);
}

if (existingWrappers.Count == 0)
{
wrappers.Remove(elementID);
Expand Down