Detect & Fix "broken" faces

Is there any way to fix “broken” faces like this usign Ruby API:


example1 (1).skp (123.9 KB)

  • it’s one face instead of two

I detect such faces using a simple snippet, but I’m not sure is it always correct:

def valid?(face)
  !face.loops.any? do |loop|
    loop.edges.flat_map(&:vertices).uniq.any? do |vrtx|
      (vrtx.edges & face.edges).count > 2
    end
  end
end

Thank you!

1 Like

I these are known as “bowtie” faces.


I tried the following without success.

Sketchup.send_action(21124) # Run validity check tool
#=> true

Sketchup.send_action('fixNonPlanarFaces:')
#=> true

Both report no problems found. No change in the “face”.


REF: https://github.com/SketchUp/api-issue-tracker/issues/291

this code is a bit inelegant, but it works.

model = Sketchup.active_model
ents = model.active_entities
ents.select{|e| e.is_a?(Sketchup::Face)}.each{|e| e.erase!}
ents.select{|e| e.is_a?(Sketchup::Edge)}.each{|e| e.find_faces}

I haven’t tried this - I’m away from a suitable PC ATM…
It’s a clunky idea, but it might work ?
Once you have the problem_vertex and its position, collect the vertices around the loop [you have the face’s loop and its vertices are always ordered].
Break your collection when the next vertex.position == problem_vertex.position
Next you need to use those collected vertex.positions - try this - form a group [in the problem face’s parent’s entities] which contains a face using those points and explode it, it’s arrival might split the problem face.
Alternatively, delete the bow-tie face, use the collected loop of points AND the loop of the remaining points to add two faces separately.
Note that in any fix their orientation might be somewhat messed up, as the loops will get reversed in one face.
In that case remember the original problem face.normal and reverse the wayward one of new faces’ if its normal is reversed !

Please try and report back…

You solution can generate extra faces in some cases. Example:

Yes, I agree that the brute force method has side effects. Since I don’t know the larger context of what you are attempting, I must leave it up to you to decide of this approach is usable or if TIG’s approach is a better fit.

Please let us know the solution when you find it!

There is a draft version of fix method:

def fix(face)
  raise "don't support" if face.loops.count > 1

  @start_eu = nil
  bad_vrtx = nil
  face.loops.first.edgeuses.each do |eu|
    vrtx = (eu.reversed? ? eu.edge.vertices.reverse : eu.edge.vertices).last
    next unless (vrtx.edges & face.edges).count > 2

    bad_vrtx = vrtx
    @start_eu = eu
    break
  end

  current_eu = @start_eu
  faces = [[]]

  loop do
    current_eu = current_eu.next
    vrtxs = (current_eu.reversed? ? current_eu.edge.vertices.reverse : current_eu.edge.vertices)
    faces[-1] << vrtxs.first.position
    break if @start_eu == current_eu

    faces << [] if vrtxs.last == bad_vrtx
  end

  original_normal = face.normal
  face.erase!

  faces.each do |pts|
    f = Sketchup.active_model.entities.add_face(*pts)
    f.reverse! unless f.normal.samedirection?(original_normal)
  end
end

But it fails in this case:


example2.skp (122.0 KB)

Have you tried with entities.intersect_with?

If that fails, create a temp group with edges perpendicular from each vertex of the face. Then transform the end vertex back to the start so you have a set of zero length vertices. Then explode the temp group.

Have you tried with entities.intersect_with?

It does not work

Then transform the end vertex back to the start so you have a set of zero length vertices.

Sorry, but how to do it?

@alexey

Not sure why SU is not seeing these as two faces but how about trying this code. I have tried it on your sample models and it seems to fix the problem

model =  Sketchup.active_model
ents = model.entities
faces = ents.grep(Sketchup::Face)
faces.each {|f|
	verts = f.vertices
	verts.each{|v|
		tr = Geom::Transformation.new(Geom::Point3d.new(0,0,0))
		ents.transform_entities(tr, v)
	}
}
3 Likes

Thank you. Looks like it works, but I need time for testing

If you get a #<TypeError: reference to deleted Face> error, try changing the ents.grep(Sketchup::Face) to ents.grep(Sketchup::Edge)

This will get the vertices from edges instead of the faces.
Cheers

1 Like

I don’t see why @ChrisDizon 's snippet works !
But it does !
At first sight it looks like it’ll move all of the face’s vertices to the origin !
The following is a simple one line version, this fixes a specified ‘face’ - which you have after checking for mangled vertices…

face.parent.entities.transform_entities( ORIGIN, face.vertices )
2 Likes

A Transformation created by Transformation.new(ORIGIN) will create an identity because it moves the ORIGIN to itself (your snippet implicitly invokes new, as compared to @ChrisDizon’s version which did so explicitly, but the result is the same). It would seem when done this way transform_entities forces the SketchUp engine to reconsider how the edges are assembled into loops, and that heals the bow-tie.

1 Like

That initialize he uses is “Translates the origin to point.” so it’s a translation that moved zero distance.
The transformation would be the same as Geom::Transformation.new or IDENTITY.

I’m guessing that this is triggering some cleanup/merging just by the act of applying a transformation to the entities (vertices). But I’d have to step through the debugger to learn more.

1 Like

Getting the vertices from the edges is probably more efficient as well, as there’s less vertex duplication.
I think I’d first collect all vertices from the edges in a Set to deduplicate them, then apply the transformation once per vertex.

And if the fix is so simple and consistent, couldn’t SketchUp’s .add_face() code run its own vertex check, and if there are any mangled vertices do the same - and apply an empty transformation to the vertices to jolt them into seeing sense, and make separate faces, rather than the bow-tie - which I imagine is never wanted [unless line splitting is set off?] !

1 Like

I am now hoping this logic can help with Can't intersect faces

:grin:

Awesome, it works! In result I got such code:

class NonManifold
  def self.auto_fix
    non_manifold = Sketchup.active_model.entities.grep(Sketchup::Face).reject { |face| valid?(face) }
    fix(non_manifold)
  end

  def self.valid?(face)
    face.vertices.none? do |vrtx|
      (vrtx.edges & face.edges).count > 2
    end
  end

  def self.fix(faces)
    faces.map(&:vertices).uniq.each do |v|
      Sketchup.active_model.entities.transform_entities(IDENTITY, v)
    end
  end
end

:frowning: No, it does not

Sketchup.active_model.entities.grep(Sketchup::Edge).flat_map(&:vertices).uniq.each do |v|
  Sketchup.active_model.entities.transform_entities(IDENTITY, v)
end

does not fix the “intersection” issue