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

Shall we automatically extract nested geometries from facade elements? #1373

Open
pawelbaran opened this issue Jun 19, 2023 · 3 comments
Open
Assignees
Labels
type:feature New capability or enhancement type:question Ask for further details or start conversation

Comments

@pawelbaran
Copy link
Member

Description:

Recently I bumped against an issue related to querying solids and physical bounds of a curtain wall. I noticed that although a curtain element has a bounding box and solid representation with volume and area, its faces and edges collections are empty 🤯

image

This results with GeometryPrimitives query returning nothing due to the following lines:

instanceGeometry = instanceGeometry.Where(x => !(x is Solid && ((Solid)x).Faces.IsEmpty)).ToList();
geometry = geometry.Where(x => !(x is Solid && ((Solid)x).Faces.IsEmpty)).ToList();

The above trickles down to Solids, Faces, PhysicalBounds etc. queries, which all return nothing now. As long as this is correct if we map the process 1:1 against the curtain Element object, is seems a bit counterintuitive that we need to query each individual panel and mullion each time we want to get the geometry of a facade element.

It tempted to me to add a few lines extracting panels and mullions inside GeometryPrimitives query (using BH.Revit.Engine.Core.Query.ICurtainGrids), but it feels like a risky shortcut taken it touches the core of the entire geometry extraction process. So I wanted to check with everyone (@enarhi in particular, taken it is facade business) how do you feel about making the change:

  1. Shall we extract nested elements from curtain ones automatically when querying geometry
  2. If so, any opinions where should that happen?
  3. What workflows will be affected and need to be tested?

All comments welcome.

@pawelbaran pawelbaran added type:feature New capability or enhancement type:question Ask for further details or start conversation labels Jun 19, 2023
@vietle-bh
Copy link
Contributor

You've probably already noticed this: edges and faces are empty because the area and volume you see are of the wall's invisible enclosing box (in blue):

I would pick Yes on question 1, but don't know the project enough to answer 2 & 3.

image

@enarhi
Copy link
Member

enarhi commented Jul 20, 2023

I'd say yes for 1, but make sure the current handling of pulling the elements within a CurtainWall does not result in duplicate geometry. For 3, the only workflow I'm aware of that would be effected is our general CurtainWall pull, which is covered as part of the PullFacadeElements test file

I'm not sure on 2, but happy to test to ensure it makes sense how it is implemented.

@pawelbaran
Copy link
Member Author

For all interested, I will have another look at this issue from the perspective of diffing, so might come back with concrete proposals for actions 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New capability or enhancement type:question Ask for further details or start conversation
Projects
None yet
Development

No branches or pull requests

5 participants