EntitiesBuilder#add_face does not de-duplicate vertices, Pushpull on defective face causes SU to lockup.

[jimvjohn]

Note from Thom: consider improvement to validity checked.

---

SU 2022 - Win 10

The title is Dan Rathbun's and mine, see: '[SketchUp Forum] [Developers/Ruby API] Losing face' for details.

The issue is builder.add_face([p1, p2, p3, p1]) creates a 3 edged face that exists until it fails the SU 'Validity Check'. Invoking Pushpull on that face while it exists causes SU to lock up.

Dan thinks this is a 'short edge' problem. He's probably right, p1 to p1 is an impossibly short edge. However builder.add_face([p1, p1, p2, p3]) creates a valid 3 edged face.

It seems this was inherited from PolygonMesh as add_polygon has the same issues.

The workaround is no duplicate points in the build array.

module Face_erase_test
  def Face_erase_test.fe_test
    p1 = Geom::Point3d.new( 0.0, 0.0, 200.0)    
    p2 = Geom::Point3d.new(-100.0, 0.0, 100.0)
    p3 = Geom::Point3d.new( 0.0, 0.0, 0.0)

    puts
    puts "To generate a 'Validity Check' on demand goto: SU Menu> Window> Model Info> Statistics"
    puts "  and click on 'Fix Problems'"
    puts
    puts  

    mesh1 = Geom::PolygonMesh.new(3,1) 
    ret = mesh1.add_polygon(p1, p2, p3, p1)  
    puts "Test output:"
    puts
    puts "PolygonMesh"
    puts("   mesh1.add_polygon(p1,p2,p3,p1) returns: " + ret.inspect + ", Face will be erased")  
    puts "         mesh1.count_points= " + mesh1.count_points.to_s
    (mG1 = Sketchup.active_model.entities.add_group).entities.add_faces_from_mesh(mesh1)  
    mG1.material = 'red'

    (bG1 = Sketchup.active_model.entities.add_group).entities.build { |builder|
      face1 = builder.add_face([p1, p2, p3, p1])
      bG1.material = 'red'
      puts  
      puts"Build"  
      puts("  builer.add_face([p1,p2,p3,p1])   returns: " + face1.class.to_s + ", Face will be erased" )  
      puts "         face1.outer_loop.vertices.count= " + face1.outer_loop.vertices.count.to_s

      #face1.pushpull(50) # removing comment causes a 'deadly embrace' condition
      puts "         #face1.pushpull(50) # causes SU to lock up <--- Note  "
      puts
    }
    (bG2 = Sketchup.active_model.entities.add_group).entities.build { |builder|
      face2 = builder.add_face([p1, p2, p3])
      bG2.material = 'skyblue'
      puts("  builer.add_face([p1,p2,p3])   returns: " + face2.class.to_s + ", Face will last" )  
      puts "         face2.outer_loop.vertices.count= " + face2.outer_loop.vertices.count.to_s

      face2.pushpull(50) 
      puts "         face2.pushpull(50) <--- works  "
      puts
    }    
    (bG3 = Sketchup.active_model.entities.add_group).entities.build { |builder|
      face3 = builder.add_face([p1, p1, p2, p3])
      bG3.material = 'magenta'
      puts("  builder.add_face([p1,p1,p2,p3])  returns: " + face3.class.to_s + ", Face will remain" )
      puts "         face3.outer_loop.vertices.count= " + face3.outer_loop.vertices.count.to_s
      face3.pushpull(50)  
      puts "         face3.pushpull(50) <--- works  "      
    }    
    t = Geom::Transformation.new(Geom::Point3d.new(0, 0, 250.0))
    mG1.transform!(t*t*t)
    bG1.transform!(t*t)    
    bG2.transform!(t)    
  end    
end

[DanRathbun]

Dan thinks this is a 'short edge' problem. He's probably right, p1 to p1 is an impossibly short edge.

I was not sure really. I floated this idea, and you tried using larger face edges. and there was no change in behavior.

It might also be a "bow-tie" issue. Ie, the face is generated in a twisted construct.

I find it weird that it is difficult to select the faces manually in the GUI created with [p1, p2, p3, p1]. These faces I can only see two of the edges, the lower and the vertical. The upper edge is not showing in the view. If I click in the middle of the face it is not selected. But if I click where the invisible edge is supposed to be, the face is selected (instead of the invisible edge.)

[DanRathbun]

Invoking Pushpull on that face while it exists causes SU to lock up.

I also did this and experienced the lock up which needed to be killed via Task Manager.

---

However I am surprised that the other two scenarios had no issues.

Doing a pushpull on a lone face will always add vertices to the model. This is warned against explicitly at the top of the Sketchup::EntitiesBuilder class Overview.

_Note: While using Sketchup::Entities#build it is important to not add or remove vertices by other means than the builder. Also don't modify the position of the vertices in the Entities container geometry is added to. Doing so can break the vertex-cache that de-duplicates the vertices_.

ie, ... face.pushpull() is not one of the builder instance methods.

Really when using the builder, only use builder methods and defer using any other API geometry modification methods until after the #build block returns.

However, I tested using a face1 declaration outside the block, and doing a pushpull on it after the block, but SketchUp's face.pushpull() still locks up when trying to yank on this "funky face".

[jimvjohn]

Good point about the vertex warning, in my zeal to be brief, I totally forgot about that. This test case is my first use of build, so I plead newbie.

[DanRathbun]

Not a problem. We do have three problems here:

• The avoidable bad face that is created
• Possible problems with Validity Check not fixing such faces
• face.pushpull choking on the face instead of raising an exception of some kind

[thomthom]

Both PolygonMesh and EntitiesBuild puts more responsibility to the API user to feed it valid data. This is an compromise for the speed these interface offers. Compare to entities.add_face that will perform more validation etc, but is significantly slower.

Don't think I'm convinced to consider this a bug, the documentation warns that these interfaces perform fewer checks. And adding checks would come at an performance cost. You feed it bad data you get bad data back.

[DanRathbun]

Don't think I'm convinced to consider this a bug, the documentation warns that these interfaces perform fewer checks.

As I said, I agree that the situation with the builder can be avoided by uniquifying the set of point arguments.

However, can the Validity Checker be made to fix such weird faces when a model you get from somewhere else is loaded ? This would side step most issues when trying to pushpull these faces in most cases.

[thomthom]

Hm, yea, maybe this could be an improvement to the validity checker.