Blocks POC (Rhino, Acad) #3519

didimitrie · 2024-06-17T17:16:12Z

Unfortunately does more things that originally intended 😅

What/why? Adds support for sending blocks (non-dynamic!) between rhino<>rhino, acad<>acad, and rhino<>acad. It's also driving this in a new, "cleaner" approach that was discussed... since pretty much the retreat in Athens 😅

Tl;dr:

Adds IInstanceObjectManager interface
Implements InstanceObjectManager in Rhino
Implements InstanceObjectManager in Acad

The above provide support for sending and receiving blocks (static blocks, not dynamic blocks) to and from Rhino and Acad.

Extras:

Have to deal with conversion of Matrix4x4 to native app in the InstanceObjectManager, this means that there's some scaling code in there. The good news is there's no dependency on the old Transform class.
Had to add a RhinoLayerManager class to simplify conversions

A question that arises fast but i do not want to solve in this PR: formalisation of the root commit object structure: yes, it's a collection, but where do we store definition proxy objects? currently attaching as a dynamic prop, so we're relying on convention. Imho, good enough to merge for now as I'd want to implement one extra similar case before we generalise (eg., rhino groups? materials?)

more notes: https://www.notion.so/speckle/Blocks-Journal-e358f6a4db7c46859ae419ff2c7dbdd6
jira tix: https://spockle.atlassian.net/browse/DUI3-120

…baking to instance manager, etc.

more testing still needed!

...Connectors/Autocad/Speckle.Connectors.AutocadShared/Bindings/AutocadBasicConnectorBinding.cs

BovineOx

I need more context but some observations....

I think there are some opportunities to simplify and improve unit testing of this in the future.
I do wonder if there's a better way to obtain the order list of instances and proxies and I'd like to understand if the index depth works
There are some direct references to the acad and probably rhino docs that should be replaced

DYI the Rhino interfacing out is about done also. We will need probably Bilal to give Revit and Rhino a good bashing when he is back as these complex changes may have added some regressions that are hard for Adam to spot but it's a hump and lump worth swallowing so we can test things

I also stopped after Acad as I wonder if I'd start being a bit repetitive

BovineOx · 2024-06-19T14:29:02Z

Core/Core/Models/Instances/InstanceProxy.cs

+  /// <summary>
+  /// The units of the host application file.
+  /// </summary>
+  public string Units { get; set; } = Kits.Units.Meters;


How dare you mention Kits ;)

We should move that definition and avoid binding tighter to it

I am not sure how at this stage? I'm hesitant to move this out of core right now...

BovineOx · 2024-06-19T14:31:07Z

Core/Core/Models/Instances/InstanceDefinitionProxy.cs

+/// <summary>
+/// A proxy class for an instance definition.
+/// </summary>
+public class InstanceDefinitionProxy : Base, IInstanceComponent


InstanceDefinitionProxy will be serialiseable and hence is derives from Base? I am circling Base atm with a view to us setting fire to it, at least in its current state, and moving to more concrete types, though I don't imagine we can avoid using it here if it is going to be serialized in place

Yep! We can move things away from base when we get to it imho

BovineOx · 2024-06-19T14:33:58Z

Core/Core/Models/Instances/IInstanceComponent.cs

+/// <summary>
+/// Abstracts over <see cref="InstanceProxy"/> and <see cref="InstanceDefinitionProxy"/> for sorting and grouping in receive operations.
+/// </summary>
+public interface IInstanceComponent


This seems a very small interface and the name and the prop seem a bit incongruous, possibly you're expecting the interface to grow to have more props/ Just wondering if this is the right place. I can sort of see I think, but maybe worth a discussion.

I don't expect it to grow (yet), i just need to sort both by max depth in the same pass. Not too clean, i agree, but then again could we live with this as a POC?

...Connectors/Autocad/Speckle.Connectors.AutocadShared/Bindings/AutocadBasicConnectorBinding.cs

BovineOx · 2024-06-20T19:57:38Z

...tors/Autocad/Speckle.Connectors.AutocadShared/Operations/Receive/AutocadHostObjectBuilder.cs

+
+    var objectGraph = _traversalFunction.Traverse(rootObject).Where(obj => obj.Current is not Collection);
+
+    // POC: these are not captured by traversal, so we need to re-add them here


Should traversal be updated?

Nope, traversal is fine as is. I'd revisit this once https://spockle.atlassian.net/browse/DUI3-10 is ironed out

...tors/Autocad/Speckle.Connectors.AutocadShared/Operations/Receive/AutocadHostObjectBuilder.cs

BovineOx · 2024-06-20T20:07:47Z

...tors/Autocad/Speckle.Connectors.AutocadShared/Operations/Receive/AutocadHostObjectBuilder.cs

+  {
+    _instanceObjectsManager.PurgeInstances(baseLayerPrefix);
+
+    using var transaction = Application.DocumentManager.CurrentDocument.Database.TransactionManager.StartTransaction();


context stack :)

BovineOx · 2024-06-20T20:09:22Z

DUI3-DX/Connectors/Rhino/Speckle.Connectors.Rhino7/Extensions/RhinoUnitsExtension.cs

+
+public static class RhinoUnitsExtension
+{
+  public static string ToSpeckleString(this UnitSystem unitSystem)


Again, we should have a neater version of this I believe?

BovineOx · 2024-06-20T20:10:44Z

DUI3-DX/Connectors/Rhino/Speckle.Connectors.Rhino7/HostApp/RhinoInstanceObjectsManager.cs

+/// <inheritdoc/>
+///  Expects to be a scoped dependency per send or receive operation.
+/// </summary>
+public class RhinoInstanceObjectsManager : IInstanceObjectsManager<RhinoObject, List<string>>


We are into Rhino now and my brain is melting and it's probably worth considering my ACAD comments first.

Also I wonder if these can be made to be more DRY though I suspect not, maybe that's one for later IF

They could be with an abstract class in between, but i'd settle with them as they are as "good enough" for now. We'd need to test this against other host apps to fully understand the patterns at play first (eg., tekla, revit)

BovineOx

Generally okay but lots of other changes making it tricky to read, probably unavoidable atm

BovineOx · 2024-06-26T12:32:39Z

DUI3-DX/Connectors/Rhino/Speckle.Connectors.Rhino7/HostApp/RhinoInstanceObjectsManager.cs

@@ -52,14 +53,15 @@ private void UnpackInstance(InstanceObject instance, int depth = 0)
  {
    var instanceId = instance.Id.ToString();
    var instanceDefinitionId = instance.InstanceDefinition.Id.ToString();
+    var currentDoc = RhinoDoc.ActiveDoc; // POC: too much right now to interface around


Can we have the ticket for this and drop that into the POC comment? This is easily unit testable and this would block that, we could make the ticket to complete this an add some unit tests

BovineOx · 2024-06-26T12:42:05Z

I did just ask for a ticket to make it interfaced out and add some tests and a question about splitting the interface but that's all

adamhathcock · 2024-06-27T15:40:37Z

Needs specklesystems/speckle-sharp-sdk#6

…-poc

didimitrie added 22 commits June 11, 2024 16:37

feat(dui3): adds instance def and instance proxy classes to core

9c65d1a

feat(dui3): rhino blocks send wip

0648654

feat(dui3): wip (works!) rhino blocks send & receive

6313b22

some cleanup

43471a2

wip fixes

0c07f75

wip cleanup

4137515

wip scaffolds acad

96be32d

Merge branch 'dui3/alpha' into dim/dui3/blocks-poc

2eb1f93

wip acad send instances and co

1e92a17

wip acad - getting close to wrapping up send

6d3b3db

wip - units

31da29f

feat(dui3): blocks poc wip: adds rhino layer manager, moves instance …

c69a781

…baking to instance manager, etc.

feat(dui3): blocks poc WIP - acad receive

5c1c560

wip acad instance receives

4460265

acad blocks - receive ok

f6ff940

feat(dui3): acad receive blocks ✅

e08d946

more testing still needed!

feat(dui3): blocks poc minor refactors

6cca93d

feat(dui3): acad blocks proper cleanup

97d3f65

feat(dui3): blocks poc ready(ish) rhino & acad

ba4556e

Merge branch 'dui3/alpha' into dim/dui3/blocks-poc

850eef8

chore(dui3): reverts accidental change

6ae0a3f

feat(dui3): adds comments/docs

bfd5d21

didimitrie commented Jun 17, 2024

View reviewed changes

...Connectors/Autocad/Speckle.Connectors.AutocadShared/Bindings/AutocadBasicConnectorBinding.cs Outdated Show resolved Hide resolved

didimitrie marked this pull request as ready for review June 18, 2024 11:57

chore(dui3): comments

536fc6e

didimitrie requested review from oguzhankoral and clairekuang June 18, 2024 11:57

didimitrie changed the title ~~Blocks POC (Rhino, Acad) - WIP~~ Blocks POC (Rhino, Acad) Jun 18, 2024

didimitrie requested a review from BovineOx June 18, 2024 11:59

BovineOx suggested changes Jun 20, 2024

View reviewed changes

JR-Morgan self-requested a review June 20, 2024 21:26

didimitrie added 3 commits June 24, 2024 16:57

fix(dui3): throws ex upwards

3991441

fix(dui3): blocks correctly handled in acad

6ffe054

Merge branch 'dui3/alpha' into dim/dui3/blocks-poc

844dd02

BovineOx approved these changes Jun 26, 2024

View reviewed changes

didimitrie assigned oguzhankoral Jun 26, 2024

Merge branch 'dui3/alpha' into dim/dui3/blocks-poc

0ee51df

adamhathcock added 3 commits June 27, 2024 17:50

Merge remote-tracking branch 'origin/dui3/alpha' into dim/dui3/blocks…

33b2614

…-poc

update core reference and fix to not use interfaces

88cb939

remove extra files

52d1c65

oguzhankoral approved these changes Jun 27, 2024

View reviewed changes

oguzhankoral merged commit 0568a29 into dui3/alpha Jun 27, 2024
33 checks passed

oguzhankoral deleted the dim/dui3/blocks-poc branch June 27, 2024 20:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Blocks POC (Rhino, Acad) #3519

Blocks POC (Rhino, Acad) #3519

didimitrie commented Jun 17, 2024 •

edited

Loading

BovineOx left a comment

BovineOx Jun 19, 2024

didimitrie Jun 25, 2024

BovineOx Jun 19, 2024

didimitrie Jun 25, 2024

BovineOx Jun 19, 2024

didimitrie Jun 25, 2024

BovineOx Jun 20, 2024

didimitrie Jun 25, 2024

BovineOx Jun 20, 2024

BovineOx Jun 20, 2024

BovineOx Jun 20, 2024

didimitrie Jun 25, 2024

BovineOx left a comment

BovineOx Jun 26, 2024

BovineOx commented Jun 26, 2024

adamhathcock commented Jun 27, 2024


		var objectGraph = _traversalFunction.Traverse(rootObject).Where(obj => obj.Current is not Collection);

		// POC: these are not captured by traversal, so we need to re-add them here

Blocks POC (Rhino, Acad) #3519

Blocks POC (Rhino, Acad) #3519

Conversation

didimitrie commented Jun 17, 2024 • edited Loading

BovineOx 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

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

BovineOx left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BovineOx commented Jun 26, 2024

adamhathcock commented Jun 27, 2024

didimitrie commented Jun 17, 2024 •

edited

Loading