Update SurfaceMatchingDialog.rb

[Ski90Moo]

Resolved an issue with the intersect dialog (divided faces were not getting properly written the drawing_interface and were not getting added as OS model surface objects). Also improved the surface matching algorithm. New method uses centroid of the polygon for matching instead of trying to match every single vertex.

Testing has been successful thus far, but it could benefit from additional testing.

Fixes #75

[macumber]

Wow @Ski90Moo, thanks for wading in to the code here! I will take a look at this over the weekend.

[macumber]

@Ski90Moo I put together some reduced matching logic similar to yours but trying to cut down on false positives in this PR https://github.com/openstudiocoalition/openstudio-sketchup-plugin/pull/115

I think the biggest problem I see is that sometimes the intersect method results in self intersecting polygons like this:

It will be some work, but I think we can detect and fix these self-intersecting polygons after the intersection. The biggest lift will be implementing a self-intersecting polygon algorithm like the one discussed here. Ruby implementation here.

[Ski90Moo]

Ok, I'll have to spend some time reviewing this. I came across this same issue when researching the intersect solution and stumbled on this thread. It seems that SketchUp has improved their intersecting since then, so I am not sure why we are still getting this issue. I put in the .find_faces method to try to resolve some of these empty and self intersecting polygon issues

[Ski90Moo]

Can you send me this test model?

On Sun, Mar 19, 2023 at 2:00 PM Dan Macumber @.***> wrote:

@.**** commented on this pull request.

In plugin/openstudio/lib/dialogs/SurfaceMatchingDialog.rb https://github.com/openstudiocoalition/openstudio-sketchup-plugin/pull/113#discussion_r1141449859 :

@@ -136,13 +136,32 @@ def intersect(selection)

   # iterate through spaces to create intersecting geometry
   spaces.each do |space|

• entity = space.entity
• entity.entities.intersect_with(true, entity.transformation, entity.entities.parent, entity.transformation, false, selection.to_a)
• entity = space.entity
• before_faces = entity.entities.grep(Sketchup::Face) #create array of faces before the intersect
• entity.entities.intersect_with(true, entity.transformation, entity.entities.parent, entity.transformation, false, selection.to_a)
• edges = entity.entities.grep(Sketchup::Edge)
• edges.each {|edge| edge.find_faces} #Heal empty loops (no face) after intersection, this may not be necessary?

• Ski90Moo:This will find faces with AttributeDictionaries copied from the base face (this is causing the faces to be added to the drawing_interface but a new model surface object is not created because all the copies point to an existing surface object.) Rough work around solution is to delete the dictionary, delete the sketchup entity, and redraw it. This prompts the downstream observer to create a new model surface object and add the sketchup entities to the drawing_interface. There is probably a more direct solution without having to delete and redraw, but I do not fully understand how the observer works.

And another failed intersection

[image: image] https://user-images.githubusercontent.com/1327850/226200838-d611250c-8249-416c-b376-7ae40a12a25c.png

— Reply to this email directly, view it on GitHub https://github.com/openstudiocoalition/openstudio-sketchup-plugin/pull/113#discussion_r1141449859, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQUKBFF3XSLHVE4Y2LRKCG3W45JTRANCNFSM6AAAAAATSEIXRM . You are receiving this because you were mentioned.Message ID: <openstudiocoalition/openstudio-sketchup-plugin/pull/113/review/1347558894 @github.com>

-- Best Regards, Mike Lovejoy P.E. Helix Energy Partners LLC Helix, OR http://www.helix-engineers.net/ https://deref-mail.com/mail/client/aJLmMsp2k0Y/dereferrer/?redirectUrl=http%3A%2F%2Fwww.helix-engineers.net%2F P:541-379-0271

[macumber]

Sent them by email

[Ski90Moo]

@macumber I added an option for beta testing the intersect and surface matching and retained the original methods on the latest changes...sorry, I have no idea what I am doing with Github, can you see the changes I made?

[macumber]

Hey @Ski90Moo, yeah I can see your changes. I think adding a beta button is good balance. I closed https://github.com/openstudiocoalition/openstudio-sketchup-plugin/pull/115. I'll test this out and merge for the next release (probably sometime in June). We'll give you credit in the release notes, thanks!

[Ski90Moo]

Thank you Dan

[macumber]

@Ski90Moo I moved this to https://github.com/openstudiocoalition/openstudio-sketchup-plugin/pull/118 and reorganized the beta code all into one block. I also added a Contributor License Agreement (CLA) action. It requires you to "sign" the CLA before we can merge a PR with your changes on it. You can do this by adding a comment on my PR with the text "I have read the CLA Document and I hereby sign the CLA"