Merge pull request #786 from NREL/59994516_PluginDoors

I think this fixes the last issue, but now seeing issues that occur with. Thanks, I must have missed that.

Author: DavidGoldwasser

openstudiocore/ruby/openstudio/sketchup_plugin/lib/interfaces/DrawingUtils.rb

@@ -24,47 +24,53 @@
 # Everything in this module should be strictly based on entities and not drawing interfaces.
 module DrawingUtils
 
-
+  # returns true if entity is the base face of face
+  def DrawingUtils.is_base_face(face, face_normal, face_points, entity)
+    if (entity.class == Sketchup::Face and not entity.equal?(face))
+      # Eliminate faces that are not parallel.
+      # Another test would be to check if both are in the same plane.
+      # There are some precision issues with 'face.plane' however.
+      if (entity.normal.parallel?(face_normal))
+        # Detect if the vertices of the entity are a subset of this face.
+        if (face_points.is_subset_of?(entity.full_polygon.reduce.points))
+          return true
+        end
+      end
+    end  
+    return false
+  end
+  
   # Strictly determined using Faces, not drawing interfaces.
   # Tries to match a face to a base face.
-  def DrawingUtils.detect_base_face(face, force)
+  def DrawingUtils.detect_base_face(face)
     base_face = nil
+    first_guess = nil
 
-    if not force
-      drawing_interface = face.drawing_interface
-      if drawing_interface
-        if drawing_interface.class == OpenStudio::SubSurface
-          parent = drawing_interface.parent
-          if parent
-            base_face = parent.entity
-            if base_face and base_face = Sketchup::Face
-              return base_face
-            else
-              base_face = nil
-            end
+    # try the current parent as a first guess
+    if drawing_interface = face.drawing_interface
+      if drawing_interface.class == OpenStudio::SubSurface
+        if parent = drawing_interface.parent
+          if temp = parent.entity and temp.class == Sketchup::Face
+            first_guess = temp
           end
-        else
-          # other classes don't have parent surfaces
-          return nil
         end
       end
     end
 
     face_normal = face.normal
     face_points = face.full_polygon.reduce.points
-
-    for child_entity in face.all_connected
-      if (child_entity.class == Sketchup::Face and not child_entity.equal?(face))
-        # Eliminate faces that are not parallel.
-        # Another test would be to check if both are in the same plane.
-        # There are some precision issues with 'face.plane' however.
-        if (child_entity.normal.parallel?(face_normal))
-          # Detect if the vertices of the entity are a subset of this face.
-          if (face_points.is_subset_of?(child_entity.full_polygon.reduce.points))
-            base_face = child_entity
-            break
-          end
-        end
+    
+    all_connected = face.all_connected
+    if first_guess
+      if all_connected.reject!{|e| e == first_guess}
+        all_connected = [first_guess].concat(all_connected)
+      end
+    end
+    
+    for entity in all_connected
+      if is_base_face(face, face_normal, face_points, entity)
+        base_face = entity
+        break
       end
     end
     return(base_face)

openstudiocore/ruby/openstudio/sketchup_plugin/lib/interfaces/SubSurface.rb

@@ -353,7 +353,7 @@ def parent_from_entity
       Plugin.log(OpenStudio::Trace, "#{current_method_name}")
 
       if (valid_entity?)
-        if (base_face = DrawingUtils.detect_base_face(@entity, true))
+        if (base_face = DrawingUtils.detect_base_face(@entity))
           return(base_face.drawing_interface)
         else
           return(super)  # Return the space interface

openstudiocore/ruby/openstudio/sketchup_plugin/lib/interfaces/Surface.rb

@@ -370,13 +370,21 @@ def face_polygon
         # This is more dynamic than looking at @children which may not be up-to-date yet.
         child_faces = []
 
+        #puts "face_polygon = #{self}, #{@entity}"
+        
         # DLM: detect_base_face can be expensive, do we have to search all_connected?  is there a way to cache the result of detect_base_face?
         for face in @entity.all_connected 
-          if (face.class == Sketchup::Face and @entity == DrawingUtils.detect_base_face(face, true))
-            #puts "found child face->" + face.to_s
-            child_faces << face
+          if face.class == Sketchup::Face
+            face_normal = face.normal
+            face_points = face.full_polygon.reduce.points
+            if DrawingUtils.is_base_face(face, face_normal, face_points, @entity)
+              #puts "found child face->" + face.to_s
+              child_faces << face
+            end
           end
         end
+        
+        #puts "child_faces = #{child_faces}"
 
         reduced_polygon = Geom::Polygon.new(@entity.full_polygon.outer_loop.reduce)  # Removes colinear points
         new_points = []

openstudiocore/ruby/openstudio/sketchup_plugin/lib/observers/SurfaceGroupEntitiesObserver.rb

@@ -82,7 +82,7 @@ def onElementAdded(entities, entity)
             #  This is a brand new surface or a copy that had to be cleaned.
             if (@drawing_interface.class == Space)
 
-              base_face = DrawingUtils.detect_base_face(entity, true)
+              base_face = DrawingUtils.detect_base_face(entity)
 
               if (base_face.nil?)
                 Plugin.log(OpenStudio::Info, "New Surface in Space")  
@@ -172,9 +172,9 @@ def onElementAdded(entities, entity)
             base_face = nil
             if (@drawing_interface.class == Space)
               if (swapped)
-                base_face = DrawingUtils.detect_base_face(entity.drawing_interface.entity, true)
+                base_face = DrawingUtils.detect_base_face(entity.drawing_interface.entity)
               else
-                base_face = DrawingUtils.detect_base_face(entity, true)
+                base_face = DrawingUtils.detect_base_face(entity)
               end
             end
 
@@ -235,6 +235,8 @@ def onElementAdded(entities, entity)
                 original_surface.add_observers
 
                 new_surface = SubSurface.new_from_entity(original_entity)
+                #puts "new_surface = #{new_surface}, #{new_surface.model_object.name}, #{new_surface.entity}"
+                #puts "new_surface.parent = #{new_surface.parent}, #{new_surface.parent.model_object.name}, #{new_surface.parent.entity}"
 
                 # Must trigger the base surface to recalculate vertices to account for the new sub surface.
                 original_surface.on_change_entity
@@ -244,8 +246,10 @@ def onElementAdded(entities, entity)
                 Plugin.log(OpenStudio::Info, "Copy-paste/divide surface:  new sub surface no swap") 
                 original_surface = entity.drawing_interface
 
-                SubSurface.new_from_entity(entity)
-
+                new_surface = SubSurface.new_from_entity(entity)
+                #puts "new_surface = #{new_surface}, #{new_surface.model_object.name}, #{new_surface.entity}"
+                #puts "new_surface.parent = #{new_surface.parent}, #{new_surface.parent.model_object.name}, #{new_surface.parent.entity}"
+                
                 # Must trigger the base surface to recalculate vertices to account for the new sub surface.
                 original_surface.on_change_entity