Sunday, August 14, 2016

The future minefield of DirectShape Elements.

When Autodesk released the DirectShape API a few years back - I thought it was fantastic. Finally a way to be able to create arbitrary geometry that could live on within Revit - rather than only the elements that the API let us create in the ways that the API let us create them.

We used this to interesting effect in our own Scan to BIM product - providing a mechanism to capture irregular shapes as Revit geometry. What's also interesting - beyond the creation of geometry, the API allows you to assign these objects to be just about any category you want. Think about that for a moment - you can make any arbitrary piece of geometry, and declare that it is a wall, room, stairs, etc.
This felt incredibly cool but also incredibly dangerous to me, from a BIM perspective. And while assigning some characteristics of a wall come from assigning the wall category - it's not an actual wall element, and there are a lot of limitations there.

This Week
This week I experienced for the first time the down side of this approach. A customer was using one of our other tools on one of their models, and it was throwing a weird exception in our app. We were looking through the logs, and based on what we were seeing, it seemed like a model corruption issue. We had a method which retrieved all of the phases that had rooms assigned to that phase - and one of the rooms apparently had a phase which was not actually a phase in this model. How could this happen? maybe corruption? maybe some kind of issue with the cut-and-paste between models?

We started working on a workaround from that angle - but a day later, the customer was able to share the model with us. When we actually ran it via the debugger, we saw which room the phase was associated with: -1  (nothing).  This hadn't seemed possible in our experience. Then we looked closer - the element in question was not actually a Room - it was a DirectShape element. How the heck did that get in there?!?

The real problem in the code, which we had written stuff like this a thousand times over the past 6 or 7 years (since the FilteredElementCollector was invented), looked something like this:

FilteredElementCollector coll = new FilteredElementCollector( myDoc );
IList elements = coll.OfCategory( BuiltInCategory.OST_Rooms ).ToElements();

Historically, if all you wanted was the elements that were rooms, this was all that you needed.
That said - some API developer had created an addin that made a DirectShape box and assigned it to the Room category.

So - going forward, we as API developers can no longer rely on the category (or even Category/IsElementNotElementType) as reliable indicators of the .NET type in the Revit API.

Our quick fix for this particular issue was:

IList elements = 
   coll.OfCategory( BuiltInCategory.OST_Rooms ).OfClass( typeof(SpatialElement) ).ToList();

This would ignore any DirectShape elements that were showing up as rooms.  We quickly found other places (in other methods) where we had made the bad assumption, and had casting errors like:

IList rooms = 
  coll.OfCategory( BuiltInCategory.OST_Rooms).Cast().ToList();

The DirectShape elements failed the Cast at runtime.

Even when fixed, these issues are bound to get confusing. I believe in many cases the DirectShape elements may still schedule as their assigned category, so our customers will believe they have N rooms in the model, when in fact only some of them are "real" rooms.

I'm still wrapping my head around it, but I think ultimately you'll just always have to have an "OfClass(typeof(MyDesiredClass))" on the collector. If you don't, you're liable to get things you don't expect.

All-in-all, it's not too bad to address it, if you know about it. The key is that it's a loophole that other developers can open, and all of us who make software where you don't know what other addins have been used will have to make sure that we're double-careful about our assumptions. And I'm not looking forward to digging back through all the code I've ever written and am still supporting to find all of the bad assumptions I made.

No comments: