IGH_Goo.CastTo<GeometryBase> bug

Grasshopper Developer
, ,
1

Ran into a weird set of behavior with trimmed a surface and passing it in as a parameter in a custom grasshopper node. I’ve got a trimmed surface. When I pass that in as a trimmed surface and then use IGH_Goo.CastTo to cast it to a GeometryBase, it comes out as a Nurbs without any loop information. When I pass it in as a Brep and do the same operation the edgeloops are preserved.

I’ve isolated the behavior down to this line of code:

geoGoo.CastTo(out Rhino.Geometry.GeometryBase geoValue);

In both cases the underlying value of geoGoo is a Rhino.Geometry.Brep.

1 Like
2

Hi @cullen.sarles,

How can we repeat what you’re seeing?

Thanks,

– Dale

3

There is a similar thing with TriRemesh component, though in this case it may be done on purpose.
If you plug a trimmed surface in the Geometry input, the trims are deleted and the underlying untrimmed surface is used.
If you first plug the trimmed surface in a Brep parameter then in Geometry input, the Brep is taken into account with its trim loops.

4

@dale

  1. Create a custom gasshopper
    a. It should have one generic input parameter and one geometry output parameter.
    b. Get the parameter as an IGH_Goo and then use the CastTo method to cast it to GeometryBase.
    c. Pass the cast surface back out of the node

  2. In Rhino Create a surface

  3. Trim it

  4. Use the surface node in grasshopper to select it

  5. Pass the geometry through the custom node we made

  6. If you visualize the surface now by passing it back out of the node and visualize the surface via the grasshopper preview, the surface is no longer trimmed.

  7. If you attach a debugger and inspect the node we made, you’ll see that the trimming information gets stripped off of the surface during the IGH_Goo.CastTo call.

As @magicteddy points out, if you first pass the surface through a brep node, or if you select the geometry with a just a geometry node instead of a surface node, the trimming information isn’t lost. It appears to be explicitly related to when geometry is coming in from a grasshopper Surface Node.

Hope this helps. I can provide a set of example files if that would be helpful.

-Cullen

5

@dale

I know you’re busy with Rhino 8 reaching Beta, but I just wanted to bump this to see if you’d had a chance to look further into the behavior.

Thinking about this behavior further, it may not be a bug per say: a pure untrimmed NURBS surface doesn’t have edge information so my guess here is that we’re getting a silent call to retrieve the underlying surface. However there’s nothing to warn users that this is happening. At the very least grasshopper should be showing a warning message on nodes when this conversion happens so that users know that they have lost data in the conversion process.

This behavior showed up in another issue today: "Baking" Trimmed surfaces using Rhinocommon, where the issue was that the type hint for a python script was casting breps to surfaces and throwing out the trimming information in the process.

Thanks,
Cullen

8

@TomTom

Moving this discussion ("Baking" Trimmed surfaces using Rhinocommon - #6 by TomTom) over here since the other thread was explicitly about baking trimmed surfaces in a python node. This a long, wordy reply, I don’t know your background knowledge so I’m going to attempt to build up to why I believe the behavior I’m seeing here is as least something that users should be provided with a visual warning about, if not a full on bug.

I’ve looked through the API documentation for both RhinoCommon and OpenNurbs just in case the RhinoCommon interface through to the C++ code had a different inheritance structure, they don’t.

Some things to note about objects in C++ and C#, it doesn’t matter which version of a reference or a pointer you have to an object (interface, abstract base class, void*, a concrete class, etc) the object is still the full version of that object. Object slicing bugs aside, casting up an inheritance tree shouldn’t be dangerous, you just have access to less of an object that you would have at more explicit version of that object. Casting back down a tree can be, and C# protects you from that by returning a null object if the cast isn’t valid. Both Brep and Surface inherit from the same base class (GeometryBase in C# and ON_Geometry in C++) and therefore casting either to that base class should be safe and result in no loss of data.

In the grasshopper API, IGH_Goo, is the interface for a data inside of Grasshopper. It provides a means to pass arbitrary data around from node to node, and serialize that data. GH_Goo<T> is the recommended abstract base class for implementing additional data types, most if not all data types in grasshopper implement GH_Goo<T>. Further more, IGH_Geometric_Goo and GH_GeometricGoo<T> serve as extensions of IGH_Goo and GH_Goo<T> respectively for geometry classes in grasshopper. If you implement GH_Goo<T> you’ll notice that it includes a property Value of the type you provide. This indicates that it’s meant as a wrapping class.

Okay so then what should the expected behavior of IGH_Goo.CastTo<T>(IGH_Goo.CastTo(T) Method) be? The documentation says that it attempts to cast to type T. My intuition here is that IGH_Goo.CastTo<T> is meant to attempt to cast the underlying data value to the presented type. This is supported by the way we interact with IGH_Goo values in grasshopper component implementations, how we retrieve values from the parameters in the API, how custom parameter types interact with base grasshopper components, and because by making it an explicit function, the interface is providing us a means to cast to objects that do not implement the interface.

This brings me to behavior that feels like a bug to me. With a debugger, I can see that Value propery on the GH_GeometricGoo object in the node has the trimming information attached whether it gets passed into the node as a brep (GH_Brep) or as a trimmed surface (GH_Surface). The fact that this information is present on the Value property of the before the call to IGH_Goo.CastTo<T> but not on the data passed out of the function is why I believe this to be a bug. If I was casting to a GH_Brep to a Rhino.Geometry.Surface, then yeah, losing the trimming information makes sense, but I’m not. I’m casting to the common abstract base class of their underlying values, Rhino.Geometry.GeometryBase, so I should not be losing data in the process. Interestingly enough, Both GH_Surface and GH_Brep implement GH_GeometricGoo<Brep>. This means that under the hood, both are storing their data as a brep, and are just providing different views of the same object. But CastTo<GeometryBase> removes the trimming information when called on a GH_Surface while it’s preserved when called on a GH_Brep.

To correct my comment in the other thread, it’s not that it’s a bug when grasshopper casts an object to a Surface, it’s a bug when grasshopper casts from a Surface.

I hope this is helpful. I’ll upload an example later today that isolates and demonstrates this behavior.

-Cullen

9

I’ve built a minimal example that reproduces the problem.
@curtisw, Looping you in incase Dale is swamped. Not a hair on fire type of thing, but it would be great if it got added to the list of smaller items to fix.

Here’s a link to the github repository: GitHub - Lightheart54/Grasshopper3dCastToDemo
The repository contains both the rhino file and grasshopper file for this screen shot:

CastToBugImage
CastToBugImage1513×757 156 KB

The grasshopper component’s Solve instance implementation is the following:

protected override void SolveInstance(IGH_DataAccess DA)
{
    GH_Brep baseData = new GH_Brep();
    if (!DA.GetData<GH_Brep>(0, ref baseData)) return;
            
    Brep brepData = baseData.Value.DuplicateBrep();
    if (brepData.Surfaces.Count > 1)
    {
        AddRuntimeMessage(GH_RuntimeMessageLevel.Error, "Must pass a single trimmed surface into the node");
        return;
    }

    //Create a new GH_Brep and GH_Surface from the underlying data
    /* This is some more weird behavior, the compiler won't let me cast to GeometryBase from
     * the concrete implementations, but is just fine if I do it from the interface
     * The error message is an unhelpful:
     * error CS1503: Argument 1: cannot convert from 'out Rhino.Geometry.GeometryBase' to 'out Rhino.Geometry.GeometryBase'
     */
    IGH_GeometricGoo newGHBrep = new GH_Brep(brepData);
    IGH_GeometricGoo newGHSurface = new GH_Surface(brepData);

    //preform castTo<GeometryBase>
    if(!newGHBrep.CastTo<GeometryBase>(out var brepBase))
    {
        AddRuntimeMessage(GH_RuntimeMessageLevel.Error, "Failed Brep Cast To");
        return;
    }
    if(!newGHSurface.CastTo<GeometryBase>(out var surfaceBase))
    {
        AddRuntimeMessage(GH_RuntimeMessageLevel.Error, "Failed Surface Cast To");
        return;
    }

    if(brepBase.ObjectType != surfaceBase.ObjectType)
    {
        var errMsg = $@"Casting to GeometryBase changed the underlying data type of the data,
Brep Cast Type Result: {brepBase.ObjectType}
Surface Cast Type Result: {surfaceBase.ObjectType}";
        AddRuntimeMessage(GH_RuntimeMessageLevel.Error,errMsg);
    }

    //Pass the data back out
    DA.SetData(0, brepBase);
    DA.SetData(1, surfaceBase);
}
10

Hi @cullen.sarles,

I believe I understand what the issue is. I guess it would help me if I understood what you are really trying to do and why?

You might know this. But a GH_Surface is not the same as a Rhino.Geometry.Surface and thus, not of type Rhino.DocObjects.ObjectType.Surface.

A GH_Surface is just a Brep with a singled face, not a surface.

This operation:

if (!newGHSurface.CastTo<GeometryBase>(out var surfaceBase))

Is going to cast, and in some cases convert, the GH object as something that inherits from Rhino.Geometry.GeometryBase. So the only real choices here are a Rhino.Geometry.Surface or a Rhino.Geometry.Brep.

GH_Surface.CastTo looks at the input type and tries to make a best guess at what you want. Since you said you wanted something of Type=GeometryBase, then the function returns a copy of the underlying Brep face’s surface - not what your test requires.

Knowing GH_Surface is really just a Brep. you should probably do this:

if (newGHSurface is GH_Surface ghSurface)
{
  var brepFace = ghSurface.Face;
  var brep = ghSurface.Value;
}

This gets around the automatic data converstion.

But again, I don’t know what bigger problem you are trying to solve.

– Dale

11

@dale

I don’t have a problem I’m trying to solve anymore. I’m already using pattern matching like you suggested to avoid the CastTo operation as necessary. The problem is that the work around is necessary and is likely the cause unintuitive behavior inside of grasshopper before we even get to talking about custom code.

In Grasshopper if you get a brep via a Surface parameter node, it tells you that it’s a trimmed surface. The fact that passing in through that node will cause some nodes to treat it as a surface and others to treat it as a brep is a problem. As @magicteddy pointed out, the TriRemesh component exhibits the behavior. The same piece of geometry coming as a GH_Surface versus a GH_Brep results in a different result without there being any indication prior that that will happen:

  • Both GH_Surface and GH_Brep will display Trimmed Surface in a panel
  • Both GH_Surface and GH_Brep will display the trimmed surface as their preview geometry in the Rhino Viewport
  • Both GH_Surface and GH_Brep will losslessly convert back and forth between each other, and if you pass the surface through the brep node first it will behave as expected in components like the TriRemesh component.

Here’s the behavior again, with just the TriRemesh component, as you can see, the center object is losing the trimmed information when passed into the TriRemesh node, even though it’s preview geometry before the node clearly shows it as a trimmed surface.

TriRemeshWeirdness
TriRemeshWeirdness1946×820 182 KB

CastToExample.3dm (27.9 KB)
CastToExample.gh (11.6 KB)

Take it or leave it, but I personally feel like this behavior is a bug. At the very least components, like TriRemesh, that quietly ignore trimming information on a brep when it is passed in as a GH_Surface should raise a runtime message to alert users as to what has happened, why it happened, and how to avoid it if they don’t want it to happen. Additionally, GH_Surface doing double duty as a single surface brep and as an untrimmed surface makes mistakes like what happened here more likely: "Baking" Trimmed surfaces using Rhinocommon. Grasshopper users aren’t trained to think of surfaces as surfaces and trimmed surfaces as breps and instead think of them as the same geometry type which makes it all the more jarring when all of a sudden either the API, or a node decides that a surface is a surface and not a trimmed surface.

Best,
Cullen

12

GH_Surface::CastTo has not changed since 2010. But I believe the behavior can be improved.

https://mcneel.myjetbrains.com/youtrack/issue/RH-77173

Note, changing/fixing the behavior may have unwanted effects on existing GH files. So don’t expect this to be addressed soon.

For now, I’d make sure to use the Brep component, not the Surface component, when passing trimmed surfaces to a component that wants generic geometry as input.

– Dale

1 Like
13

Dale already answered the way I would have. I still don‘t think its a good thing todo. Technically you don’t even „cast“ here. Whatever this strange English word is meaning. In my opinion it should rather throw in uncertain situations.

There could also be a reason to expect a Surface/NurbsSurface as a return object of a GH_Surface. I had use-cases were yielding Breps is occasionally a disadvantage. E.g. Trimming information can be a problem if you export to other CAD.

On the other hand I would never tried to cast to an abstract Basetype if I expect something concrete. You never know what problems you run into with too generic code during Runtime. That is also why I think that touching 13 year old code with huge amount of api users, is always a risk. And regarding the very low benefit of this „bug“-fix I personally would have spend the time to write a extension method or any other form of helper function, to simply do it the way I want. But in the end McNeel devs are in responsibility here, and I was just questioning the relevance of this bug in first place. But of course, its always good to bring attention to flaws in the api.

1 Like
14

@TomTom and @dale

Thanks for thinking through the behavior. I understand how it feels like a small thing, but developers propensity to build a work-around is probably the reason why this behavior has existed for 13 years and isn’t documented anywhere. I don’t expect it to be changed in GH1, but if I don’t point it out as strange it likely won’t be on the radar for GH2. I agree that changing it at this point would likely cause more problems than its worth, which is why I offered the idea of posting runtime messages when a node or type change is going to strip the trimming information off an object. It’s something that wouldn’t change any existing behavior and would help normal users of grasshopper understand why they to do something different.

Having built API’s, it always frustrated me when people consuming them worked around bugs and strange behaviors instead of telling me about them. On the other hand, I found it worse when they expected me to fix everything for me. So I’m doing my best to find the balance between finding my own solutions and posting about them. If there’s a better label for this type of thing than “bug” I’m happy to use it instead. Is there an API Feedback label beyond the “RhinoCommon, Grasshopper Developer, etc” general categories? I would prefer to not be crying wolf on things that aren’t blocking for me if that’s what the bug label amounts to.

-Cullen

15

Either category is fine - we’ll find it.

– Dale

1 Like