Skip to content

Commit

Permalink
Feat (Performance): receive performance improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
oguzhankoral authored Nov 28, 2023
2 parents 58ae858 + a5bb5c4 commit 120083b
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 22 deletions.
6 changes: 6 additions & 0 deletions speckle_connector/src/actions/receive_objects.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
require_relative 'action'
require_relative '../convertors/units'
require_relative '../convertors/to_native'
require_relative '../convertors/clean_up'

module SpeckleConnector
module Actions
Expand All @@ -26,7 +27,12 @@ def update_state(state)
converter = Converters::ToNative.new(state, @stream_id, @stream_name, @branch_name, @source_app)
# Have side effects on the sketchup model. It effects directly on the entities by adding new objects.
start_time = Time.now.to_f
state.sketchup_state.sketchup_model.start_operation('Receive Speckle Objects', true)
state = converter.receive_commit_object(@base)
if state.user_state.model_preferences[:merge_coplanar_faces]
Converters::CleanUp.merge_coplanar_faces(converter.converted_faces)
end
state.sketchup_state.sketchup_model.commit_operation
elapsed_time = (Time.now.to_f - start_time).round(3)
puts "==== Converting to Native executed in #{elapsed_time} sec ===="
puts "==== Source application is #{@source_app}. ===="
Expand Down
41 changes: 22 additions & 19 deletions speckle_connector/src/convertors/clean_up.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@ module CleanUp
# @param entities [Sketchup::Entities] entities to remove edges between that make entities coplanar.
# @note Merging coplanar faces idea originated from [CleanUp](https://github.com/thomthom/cleanup) plugin
# which is developed by [Thomas Thomassen](https://github.com/thomthom).
def self.merge_coplanar_faces(entities)
def self.merge_coplanar_entities(entities)
edges = []
faces = entities.collect { |entity| entity if entity.is_a? Sketchup::Face }.compact
faces = merged_faces(faces)
faces.each { |face| face.edges.each { |edge| edges << edge } }
edges.uniq!
edges.each { |edge| remove_edge_have_coplanar_faces(edge, faces, false) }
Expand All @@ -22,6 +23,22 @@ def self.merge_coplanar_faces(entities)
merged_faces(faces)
end

def self.merge_coplanar_faces(faces)
edges = []
faces = faces.reject(&:deleted?)

faces.each { |face| face.edges.each { |edge| edges << edge } }

edges.uniq!

edges.each { |edge| remove_edge_have_coplanar_faces(edge) }

# Remove remaining orphan edges
# edges.reject(&:deleted?).select { |edge| edge.faces.empty? }.each(&:erase!)

merged_faces(faces)
end

def self.merged_faces(faces)
faces.reject(&:deleted?)
end
Expand All @@ -35,43 +52,29 @@ def self.merged_faces(faces)
# - Whether UV texture map is aligned between faces or not.
# - Finally, if faces are coplanar by correcting these checks, then removes edge from Sketchup.active_model.
# @param edge [Sketchup::Edge] edge to check.
# @param faces [Array<Sketchup::Face>] scoped faces to check 'edge.faces' both (first and second)
# belongs to this faces or not. If any of this faces does not involve this scoped faces, then do not delete.
# @param ignore_materials [Boolean] whether ignore materials or not.
# Returns true if the given edge separating two coplanar faces.
# Return false otherwise.
# rubocop:disable Metrics/AbcSize
def self.remove_edge_have_coplanar_faces(edge, faces, ignore_materials)
def self.remove_edge_have_coplanar_faces(edge)
return false unless edge.valid? && edge.is_a?(Sketchup::Edge)
return false unless edge.faces.size == 2

# Check scoped faces have this edges
if edge.faces.size == 2
is_first = faces.include?(edge.faces[0])
is_second = faces.include?(edge.faces[1])
return false unless is_first && is_second
end

face_1, face_2 = edge.faces

return false unless face_1.normal.samedirection?(face_2.normal)

return false if face_duplicate?(face_1, face_2)
# Check for troublesome faces which might lead to missing geometry if merged.
return false unless edge_safe_to_merge?(edge)

# Check materials match.
unless ignore_materials
return false unless (face_1.material == face_2.material) && (face_1.back_material == face_2.back_material)
return false unless (face_1.material == face_2.material) && (face_1.back_material == face_2.back_material)

# Verify UV mapping match.
return false if !face_1.material.nil? && !continuous_uv?(face_1, face_2, edge) && face_1.material.texture.nil?
end
# Check faces are coplanar or not.
return false unless faces_coplanar?(face_1, face_2)

edge.erase!
true
end
# rubocop:enable Metrics/AbcSize

# Determines if two faces are overlapped.
def self.face_duplicate?(face_1, face_2, overlapping: false)
Expand Down
5 changes: 5 additions & 0 deletions speckle_connector/src/convertors/to_native.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,14 @@ class ToNative < Converter
# @return [String] source application of received object that will be converted to native
attr_reader :source_app

attr_reader :converted_faces

def initialize(state, stream_id, stream_name, branch_name, source_app)
super(state, stream_id)
@stream_name = stream_name
@branch_name = branch_name
@source_app = source_app.downcase
@converted_faces = []
end

# Module aliases
Expand Down Expand Up @@ -312,6 +315,8 @@ def convert_to_native(state, obj, layer, entities = sketchup_model.entities)
# Call 'to_native' method by passing this method itself to handle nested 'to_native' conversions.
# It returns updated state and converted entities.
state, converted_entities = to_native_method.call(state, obj, layer, entities, &convert_to_native)
faces = converted_entities.select { |e| e.is_a?(Sketchup::Face) }
@converted_faces += faces if faces.any?
if from_revit
# Create levels as section planes if they exists
create_levels(state, obj)
Expand Down
6 changes: 3 additions & 3 deletions speckle_connector/src/speckle_objects/geometry/mesh.rb
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,9 @@ def self.to_native(state, mesh, layer, entities, &convert_to_native)
mesh_layer_name = SketchupModel::Query::Layer.entity_layer_from_path(mesh['layer'])
mesh_layer = state.sketchup_state.sketchup_model.layers.to_a.find { |l| l.display_name == mesh_layer_name }
# Merge only added faces in this scope
if model_preferences[:merge_coplanar_faces]
added_faces = Converters::CleanUp.merge_coplanar_faces(added_faces)
end
# if model_preferences[:merge_coplanar_faces]
# added_faces = Converters::CleanUp.merge_coplanar_faces(added_faces)
# end
added_faces.each do |face|
face.layer = mesh_layer unless mesh_layer.nil?
# Smooth edges if they already soft
Expand Down

0 comments on commit 120083b

Please sign in to comment.