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

Allow conversion of spatial elements to Geometry via BHoMGeometry component #488

Open
pawelbaran opened this issue Mar 13, 2020 · 13 comments
Assignees
Labels
type:feature New capability or enhancement type:question Ask for further details or start conversation

Comments

@pawelbaran
Copy link
Member

Description:

One of my colleagues was just working with BHoM panels and asked me why couldn't we have a direct convert from an IElement to geometry.

image

As discussed with @IsakNaslundBh, this is a bit against the logic plus dangerous because of the risk of losing all properties etc. etc. But then, I believe this would be a nice bow towards the end user, possibly relatively easy to implement (we have Geometry methods in place for preview purposes). Additionally, conversion could record a warning and turn the component orange to inform the user about lost properties.

@rwemay @al-fisher FYI

@pawelbaran pawelbaran added type:feature New capability or enhancement type:question Ask for further details or start conversation labels Mar 13, 2020
@epignatelli
Copy link
Member

I guess you can turn anything into a Geometry with the IGeometry method.

But I'd turn the question the other way around, can methods that take IElement as input work on IGeometry(ies) instead?

@pawelbaran
Copy link
Member Author

Yep, agreed, but one needs to know about the existence of IGeometry method, which is not obvious for non-devs.

Concerning your question, the methods that take IElement inputs can work on IGeometry if IGeometry implements IElement interface.

@al-fisher
Copy link
Member

Yeah - I think this is not a bad idea!
You can see the GH Parameter components as effectively “casts” - so could be neat putting an IElement explicitly into a BHoMGeo Parameter.
As you say will just call IGeometry in the backend.

The warning about loss of data is also smart

@adecler
Copy link
Member

adecler commented Mar 14, 2020

I don't see a problem with that. In fact, using parameter components to cast objects is very much in the philosophy of GH (at least that's how I use them to cast string to int for example).

This is not specific to IElement however. Any BHoM object that can be casted to geometry should be outputted as such (with the warning of course).

@pawelbaran
Copy link
Member Author

Great! Is there a chance you could tackle that at some point @adecler? Defo not a priority, rather a nice to have.

@adecler
Copy link
Member

adecler commented Mar 20, 2020

Sure. at some point being the keywords here 😉
Moving back to a more innovation-focused plan next quarter mean that I will have less time for low priority issues. I am however keen to make sure that some momentum is kept on improving the user experience in the UI.

@rwemay
Copy link
Member

rwemay commented Mar 25, 2020

Sounds like a concensus on allowing casts to geometry if there is geometry calling the GetGeometry without necessitating another 'get geometry' method by the user. I agree with this.

As an aside (not to hold up/necessarily influence this), one slight challenge to this is that we've talked in the past a lot about multiple goemetries/representations on an object. This (I think) is a very separate subject, and good to have another conversation soon about it @al-fisher . My thinking has developed a bit on this. Given the entity/atomic nature of the objects, and us using reflection, interfaces and attributes more and more so, I think alternative geometry can be derived/separate/fragmented, and that we do have 'one geometry' associated with an object, by default. Thoughts?!

@pawelbaran
Copy link
Member Author

@rwemay you are right, an object can have a few geometrical representations. However, sticking to this issue, for each object we have only one representation at a time for GH/Dynamo preview purposes - this particular representation would be converted to geometry, which would also be intuitive for the end user.

@al-fisher
Copy link
Member

multiple geometries/representations on an object.

@rwemay

Yes! definately! - Will be good to add to next general Framework agenda @adecler ?
@alelom and I have recently been discussing too and would be good to start a plan in 3.2

@adecler
Copy link
Member

adecler commented Mar 26, 2020

@al-fisher , session created and invite sent. You can find the related issue in admin as last time.

@adecler
Copy link
Member

adecler commented Apr 9, 2020

Also see #366

@adecler adecler assigned IsakNaslundBh and unassigned adecler Jan 7, 2023
@IsakNaslundBh
Copy link
Contributor

Behaviour of this has improved by quite a bit since this issue was first raised I think:

image

  1. The BHoM Geometry parameter is now going red and throwing an error instead of failing silently
  2. You can achieve the behaviour by plugging the objects into a relevant Grasshopper geometry parameter

Dont think it would be to difficult to add the behaviour asked for here though as well, by adding another casting operation. Happy for a quick discussion if we should do that or not, hence leaving open for now.

@al-fisher
Copy link
Member

Yes - agreed @IsakNaslundBh.

We still have the ambiguity of multiple geometrical representations being possible from an object (i.e. 3D, Render etc.) - however this is clear in the framework that it is all handled by the relevant methods - so casting and IElement to BHoMGeometry through the component should simply call the Geometry() method obviously
We should implement this - and perhaps return a note/warning that this is the representation being called and that extraction of other geometrical representations are possible by other means, perhaps

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

6 participants