Face Creation Attempt Aborts Operation

Developers Ruby API
1

I’ve just spent over an hour trying to track down a bug. It turns out to be what I would consider a bug in the api. The creation of the face fails, and there is no error. The operation is also aborted at the same time.

I’m not sure how to correct the problem, other than modifying my points in such a way that the problem is circumvented.

Here is the model.

Face Test Bug.skp (117.2 KB)

Here is the code to reproduce the error.

shed = Sketchup.active_model.entities.grep(Sketchup::ComponentInstance)[0]
shed.set_attribute('bc', 'test', 'value1')
puts "expected text value: value1, got value: #{shed.get_attribute('bc', 'test')}"

Sketchup.active_model.start_operation('Face Test')
points = [[192.0, -96.0, 120.0],
          [96.0, -96.0, 152.0],
          [5.0, -96.0, 121.66666666666667],
          [5.0, -96.0, 39.0],
          [67.0, -96.0, 39.0],
          [67.0, -96.0, 120.0],
          [192.0, -96.0, 120.0],
          [192.0, -96.0, 116.0]]
shed.set_attribute('bc', 'test', 'value2')
shed.definition.entities.add_face(points)
#this demonstrates that the operation was aborted and the previous value is in the dictionary.
puts "expected text value: value2, got value: #{shed.get_attribute('bc', 'test')}"
Sketchup.active_model.commit_operation

PS: I want to get some other input before posting an issue in the api issue tracker.

2

Ok this seems to work for the time being.

def self.remove_duplicate_points(points, digits = 2)
  pts = []
  last_point = nil
  points.each do |point|      
    x_same = !last_point.nil? && (point[0] - last_point[0]).abs.round(digits) == 0
    y_same = !last_point.nil? && (point[1] - last_point[1]).abs.round(digits) == 0
    z_same = !last_point.nil? && (point[2] - last_point[2]).abs.round(digits) == 0
    unless x_same && y_same && z_same
      if pts.count { |e| e == point } > 0
        vector = Geom::Vector3d.new(last_point, point)
        vector.length = 0.001
        point = (Geom::Point3d.new(point) + vector).to_a
      end
      pts.push point 
    end

    last_point = point
  end
  pts
end
3

You should always test the return result from any of the entities factory methods.
Some may also raise exceptions so trapping those and putsing their message to the console can help in testing.

EDIT: I see you said that no error is raised.

4

I tried to show a simplified example. In my production code the returned face is further processed if it is not nil.

Also using a model observer the onTransactionAbort event is triggered when trying to create the face, even if not in a transaction.

onTransactionAbort: #<Sketchup::Model:0x0000023c49d70938>

The real problem is that there is no way to ‘catch’ the error. Even if the abort is caught in the event, the executing method continues happily after the abort, as opposed to an error where the method aborts if it isn’t handled.

5

So you are implying that your set of points (which are really arrays of Numerics) have duplicate(s).

A simple way is …

points.uniq!

But to use SketchUp’s internal tolerance it’s probably better to create an array of actual Geom::Point3d objects.

points.map! {|pt| Geom::Point3d.new(pt) }
uniq = []
points.each do |pt| 
  uniq << pt unless uniq.any?{|mem| mem == pt }
end
6

The next to last point in your Array already closes the loop. The final one is a stray that is causing add_face to fail silently!

7

Silent fails are bugs. This should either raise an ArgumentError exception, or ignore stray and coincident points (but output a warning.)

Yea, I’d say file an issue.

1 Like
8

No it’s more complicated than that. It only fails when there is other geometry at the same location.

In an empty model this works fine.

Sketchup.active_model.start_operation('Face Test')
points = [[192.0, -96.0, 120.0],
          [96.0, -96.0, 152.0],
          [5.0, -96.0, 121.66666666666667],
          [5.0, -96.0, 39.0],
          [67.0, -96.0, 39.0],
          [67.0, -96.0, 120.0],
          [192.0, -96.0, 120.0],
          [192.0, -96.0, 116.0]]
Sketchup.active_model.entities.add_face(points)
Sketchup.active_model.commit_operation
9

100% agreed. Silent fails can be very difficult to track down.

1 Like
10

But if you remove that extra point it also works fine in your original code snippet. So it seems that something is triggered by the interaction between that extra bit and the pre-existing geometry. Nonetheless, I agree the silent fail is a bug.

11

Thanks for filing that issue. But IMO … the issue thread itself needs to have the reproducible code and issue details. (What if for some unknown reason the SketchUp forum ceases to exist, as has happened 3 times in the past and a new forum is put in place ?)

1 Like
12

Big plus to adding complete instructions in the github ticket itself. :+1::+1:

2 Likes
13

Yeah thanks for adding that info to the ticket.