BHoM / Revit_Toolkit

A set of tools enabling exchange of information between BHoM and Revit.
GNU Lesser General Public License v3.0
27 stars 13 forks source link

Pull Geometry With ModelInstances #708

Closed enarhi closed 3 years ago

enarhi commented 4 years ago

Description:

When pulling from Revit returns a ModelInstance, it would be great to have some kind of default behavior that pulls the default Revit geomoetry associated with the element as one of the ModelInstance's properties for usage downstream. This could also alleviate development requirements for handling all sorts of Revit element type fringe cases, as for most cases (generic models, isolated foundations, etc) this geo along with current pull capabilities is enough to enable downstream workflows.

rwemay commented 4 years ago

Do you mean the mesh geometry and/or edges/surfaces? I've raise similar before (if I understand correctly) - if there isn't a BHoM representation, pull the Revit geometry. I'd probably err on the side of not doing this by default now though, having pulled a few meshes. Until we've more 1:1 representations it could slow down pull quite a bit. Can always set pull geometry on the geometric representation settings.

pawelbaran commented 4 years ago

I would agree with @rwemay - defaulting pull behaviour to converting geometry for all ModelInstances sounds really risky to me. On the other hand, I do see where @enarhi's request is coming from.

Instead of changing default behaviour, what do you think of adding GeometryRequest and RepresentationRequest that would take an already pulled element in a form of ModelInstance (or any other type) and query it for its geometry or representation using PullGeometryConfig or PullRepresentationConfig respectively? This allow making the process more granular, if needed.

enarhi commented 4 years ago

@pawelbaran I think that kind of approach could definitely work. Totally understand not doing it by default as @rwemay said, I think it is just about making sure we take advantage of Revit_Toolkit to pull all geometry when needed, since that is essential for some workflows and I'd hate for people to not take advantage of it just because edge cases like certain foundations and other elements are not able to come through.

pawelbaran commented 4 years ago

@enarhi as a workaround, one can sift off the BHoM objects that need some extra geometry pull => input them into FilterByElementIds (there is one that takes pulled BHoM objects and retrieves the related ElementIds for you) => pull again with RevitPullConfig requesting geometry.

In the meantime, I will look into how much work would be needed to implement GeometryRequest and RepresentationRequest.

Does that sound reasonable?

enarhi commented 4 years ago

@pawelbaran yea, reasonable enough for now. Suppose long term we need to decide whether we want a generic pull option that works with geometry or continued specific support for all categories identified as essential, and all geometry options that they may contain.

rwemay commented 4 years ago

Just to add to this... it could be good to include a bool on the geometry config to ignore mesh pulls for objects where geometry IS converted. I'm pulling from a 150mb arch model and having to pull in stages - I think will be 5gb of json files once I've fully pulled....

pawelbaran commented 4 years ago

Not sure if I understand @rwemay. The mesh gets pulled only if you set the config to do so, how is that related to geometry converts?

rwemay commented 4 years ago

So if you set true on pull meshes, you get meshes of objects that we support the native/primitive geometry for (e.g. beams). Typically you’d want a mesh if you can’t get the geometry through BHoM, so an option to only pull mesh, edges or surfaces if there isn’t a BHoM geometry convert. Similar to @enarhi ‘s suggestion, but not setting pull meshes on by default.

pawelbaran commented 4 years ago

Hmm, this could be done, but sounds a bit like a temporary hack. Not sure if it would not be more efficient to spend some more effort on surface converts (which should not be rocket science in the end) and make the functionality work as expected?

What I am also not sure about your idea: surfaces get converted one by one, so converting the failing ones into meshes could lead to a weird pool of surfaces and meshes.

rwemay commented 4 years ago

Sorry, didn't explain very well. I didn't mean pulling combinations of meshes and surfaces, rather, if there is a 1:1 conversion of an object, don't pull the geometry at all, since this can be created/derived from the 1:1 properties (e.g. floors/framing elements etc.). It would need to be an option - as you might was 'all meshes' - irrespective of whether they have a 1:1 BHoM representation.

pawelbaran commented 4 years ago

Would it mean that the new functionality would lead to getting the meshes pulled when a Revit element gets converted into a ModelInstance? This should be relatively easy to hack with the current functionality - one can simply filter out the pulled ModelInstances and pull them again with meshes.

I am not against the idea as such, but would like to make sure the implementation will provide an added value in long-term - RevitPullConfig is already quite complex, I would try to be as minimalistic with supplementary features as possible.

pawelbaran commented 3 years ago

This discussion went stale and the more I think about it, the more I believe the decision about what to pull should lie on the user: @rwemay would like to pull meshes only when native geometry does not exist, @enarhi would expect the geometry to be pulled with ModelInstances, others may have even different preference.

Therefore, I am closing this issue with a proposal of a simple solution based on FilterByType method, available here. FYI, I have also raised https://github.com/BHoM/BHoM_Engine/issues/2015 to make filtering by type even easier than it is now.

image

Please feel free to reopen this issue in case you strongly believe the problem has not been resolved and development on the side of Revit_Toolkit is required. Thanks!