rs.ShrinkTrimmedSurface() broken for certain input cases

Was just testing something this morning and ran across this. The help for rs.ShrinkTrimmedSurface() implies that it should accept either surfaces or polysurfaces.

However if I feed it a polysurface it fails. If I feed it a single surface it works. I used this to test:

import rhinoscriptsyntax as rs

msg="Select surfaces or polysurfaces to shrink"
objs=rs.GetObjects(msg,8+16,preselect=True)
if objs:
    for obj in objs:
        result=rs.ShrinkTrimmedSurface(obj,False)
        print result

If you select the two closed polysurfaces in the file below, it fails and prints “None, None”
If you explode one or the other of the polysurfaces and select say the unshrunk horizontal surfaces of the result, it succeeds.

ShrinkTest.3dm (2.0 MB)

Upon further investigation and testing:
The underlying RC code for rs.ShrinkTrimmedSurface() is as follows:

    brep = rhutil.coercebrep(object_id, True)
    if not brep.Faces.ShrinkFaces(): return scriptcontext.errorhandler()  #fails here if input is a multiface brep
    rc = None
    object_id = rhutil.coerceguid(object_id)
    if create_copy:
        oldobj = scriptcontext.doc.Objects.Find(object_id)
        attr = oldobj.Attributes
        rc = scriptcontext.doc.Objects.AddBrep(brep, attr)
    else:
        rc = scriptcontext.doc.Objects.Replace(object_id, brep)
    scriptcontext.doc.Views.Redraw()
    return rc

If I step into that function I see it is failing because brep.Faces.ShrinkFaces() reports False if a polysurface is input that has at least one UNSHRUNK (i.e. natural) face. If all the faces are unshrunk, it succeeds and reports true.

I used this as a simplified test with the file below:

import rhinoscriptsyntax as rs
import scriptcontext as sc
import Rhino

msg="Select surfaces or polysurfaces to shrink"
objs=rs.GetObjects(msg,8+16,preselect=True)
if objs:
    for obj in objs:
        brep = rs.coercebrep(obj,True)
        shrink=brep.Faces.ShrinkFaces()
        if not shrink:
            print "Shrink failed!"
        else:
            print "Shrink succeeded."

ShrinkTest2.3dm (1.9 MB)

What I haven’t yet determined is if it’s just the reporting that is bad - which causes the rs function to fail immediately - or if the surfaces are actually shrunk despite the False success/failure return. Will attempt to find that out next.

Edit:
And the answer is: it is the reporting that is incorrect, not the actual shrink function - which is kinda what I expected or this would have been found much earlier…

To test:

import rhinoscriptsyntax as rs
import scriptcontext as sc
import Rhino

msg="Select surfaces or polysurfaces to shrink"
objs=rs.GetObjects(msg,8+16,preselect=True)
if objs:
    for obj in objs:
        brep = rs.coercebrep(obj,True)
        shrink=brep.Faces.ShrinkFaces()
        if not shrink:
            print "Shrink failed!"
        else:
            print "Shrink succeeded."
        if brep:
            sc.doc.Objects.Replace(obj,brep)
    sc.doc.Views.Redraw()

If you run this on the any of the polysurfaces in the files above with at least one unshrunk face, it reports “Failed” but in fact the shrink has succeeded if you look at the result in the Rhino window.

So it is actually the RhinoCommon method BrepFaceList.ShrinkFaces that is partially broken, it actually succeeds but reports failed in this particular case.

image

Does it report success when it actually fails? This would imply a relatively easy to find and fix inversion of a logic test.

No, it’s not a simple crossed wires inversion, but yes, there is a logic inversion. If you run it on a single surface it returns True if it could shrink the surface and False if it could not - i.e. if the surface is already shrunk or it failed for some other reason. That is the correct behavior.

However, with multi-face breps, my feeling is the method is programmed in such a way that it returns False if any one of the of the faces fails. It should instead be programmed to return True if any one of the faces succeeds.

@alain - this might be in your wheelhouse?

I created a youtrack issue for this just in case.
https://mcneel.myjetbrains.com/youtrack/issue/RH-64138

Well, as it doesn’t appear that this is going to be addressed any time soon, below is some code that ‘fixes’ the broken rhinoscriptsyntax ShrinkTrimmedSurface() method. The input arguments are the same as the original, but the difference is this will actually work to shrink multi-face breps that contain at least one face that will not shrink. The original fails in this case, the definition below succeeds and returns True (or a copy if that argument was supplied). You can test both the original method and the substitute on the file below, which is a cube with 5 trimmed unshrunk faces and one untrimmed shrunk face.

import rhinoscriptsyntax as rs
import scriptcontext as sc
import Rhino

def ShrinkTrimmedSurfaceEx(object_id, create_copy=False):
    """Shrinks the underlying untrimmed surfaces near to the trimming
    boundaries. See the ShrinkTrimmedSrf command in the Rhino help.
    
    ############################################################################
    Temporary remedy for bug/limitation in the native rhinoscriptsyntax method.
    This version also works on breps that contain  at least one unshrunk face,
    the native version will fail to do anyting in this case.
    Script by Mitch Heynick 13.05.21
    ############################################################################
    
    Parameters:
      object_id (guid): the surface's identifier
      create_copy (bool, optional): If True, the original surface is not deleted
    Returns:
      bool: If create_copy is False, True or False indicating success or failure
      bool: If create_copy is True, the identifier of the new surface
      None: on error
    Example:
      import rhinoscriptsyntax as rs
      filter = rs.filter.surface | rs.filter.polysurface
      surface = rs.GetObject("Select surface or polysurface to shrink", filter )
      if surface: rs.ShrinkTrimmedSurface( surface )
    See Also:
      IsSurfaceTrimmed
    """

    brep = rs.coercebrep(object_id, True)
    chk_brep=brep.Duplicate()
    brep.Faces.ShrinkFaces()
    #if result is not valid, return
    if not brep.IsValid: return sc.errorhandler()
    #if nothing was shrunk, brep and chk_brep are the same - return False
    if Rhino.Geometry.GeometryBase.GeometryEquals(brep,chk_brep): return False
    rc = None
    object_id = rs.coerceguid(object_id)
    if create_copy:
        oldobj = sc.doc.Objects.Find(object_id)
        attr = oldobj.Attributes
        rc = sc.doc.Objects.AddBrep(brep, attr)
    else:
        rc = sc.doc.Objects.Replace(object_id, brep)
    sc.doc.Views.Redraw()
    return rc

obj_id=rs.GetObject("Select a polysurface to shrink",16,preselect=True)
if obj_id:
    #test_result=rs.ShrinkTrimmedSurface(obj_id)
    test_result=ShrinkTrimmedSurfaceEx(obj_id)
    print test_result

AllButOne.3dm (1.9 MB)