BHoM / BHoM_Engine

Internal manipulation of the BHoM
GNU Lesser General Public License v3.0
26 stars 13 forks source link

Spatial Engine: Element2D Area Query not working #2081

Closed michaelhoehn closed 3 years ago

michaelhoehn commented 3 years ago

Description:

Cannot successfully query the area of any Element2D using this method. Interestingly the comment displays the error found in the Area(Element1D) query method. Also interesting that there are other query methods in other engines that can be used successfully, but these methods rely on ExternalEdges rather than OutlineCurves.

image

Steps to reproduce:

Create a panel and query it's area.

Expected behaviour:

Test file(s):

Test File

FraserGreenroyd commented 3 years ago

After discussion and further investigation with @michaelhoehn it appears this was caused by Spatial_Engine having an Area method which takes an IElement1D argument. Meanwhile, the polycurve openings in the Area(IElement2D) method implement ICurve, which in turn implements IElement1D. Because the Area(ICurve) method sits in Geometry_Engine, the code is likely looking for the closest match it can use, which happens to be the IElement1D one.

In theory, changing this to tell it to explicitly call the Geometry_Engine Area method will fix it.

However, a wider discussion might be needed on the error/warning message of Area(IElement1D) because it says it can't be done, and yet it can if your IElement1D is actually a Geometry object. But that's for later I imagine.

IsakNaslundBh commented 3 years ago

In theory, changing this to tell it to explicitly call the Geometry_Engine Area method will fix it.

Think we might as well switch to explicit call here to be safe for the future! Worst case could be we add some curve that we do make into a IELement2D which could mean we end up with stack overflow in this method if we do not change!

However, a wider discussion might be needed on the error/warning message of Area(IElement1D) because it says it can't be done, and yet it can if your IElement1D is actually a Geometry object. But that's for later I imagine.

Do not really see a reason for keeping those methods tbh. Can not really think now why we added them in in the first place... Worth a discussion if we should simply remove all the non-2D IElement methods, including the interface method.

FraserGreenroyd commented 3 years ago

Looking at git blame, it was @pawelbaran who brought the methods in, following a PR review by @al-fisher and @LMarkowski so hopefully they can help shed some light as to why this was done. It was 17 days ago so hopefully still fresh enough to remember.

pawelbaran commented 3 years ago

Yeah I was simply putting the missing, simple-to-implement (I thought that safe-to-implement too) methods in. No bigger philosophy behind that, all blame on me 🙈