Improve performance for drawing arcs and faces in bb corners

Developers Ruby API
1

Hi everyone,

I’m creating a tool that when a component is selected, a hot spot are drawn in each corner of element. Like image:

But this is consuming a lot of CPU (about 10% ~ 14% more) each time that have to draw those hot spots.
I know my code is not performatic at all, I would like to get some advices to improve it and I think I’m working wrong with the Tool, maybe need to start_transaction and stuffs like that.

Here is what I’m doing:

module My_module
class My_class
    def initialize
    	@state = 0
    	@group_entities = []
    end

def deactivate(view)
	reset
end

def reset
	ents = Sketchup.active_model.entities
	@group_entities.each {|e| ents.erase_entities e}
	@group_entities = []
	@state = 0
end
def onMouseMove(flags, x, y, view)
	ph = view.pick_helper
	ph.do_pick x,y
	@element = ph.best_picked

	if @element.is_a? Sketchup::ComponentInstance
		ip = view.inputpoint x, y
		selected_face = ip.face
		trans = ip.transformation
		#puts element.definition.get_attribute('dynamic_attributes', 'puxador_medida').to_f.to_mm
	elsif @element.is_a? NilClass
		if @state == 1
			reset
			@state = 0
		end
	end

	view.refresh
end
def draw(view)
	if @element.is_a? Sketchup::ComponentInstance
		if @state == 0
			@state = 1
			draw_selectable_corners
		end
	end
end
def draw_selectable_corners
	draw_left_front_bottom
	draw_left_front_top
	draw_right_front_bottom
	draw_right_front_top

	draw_left_back_bottom
	draw_left_back_top
	draw_right_back_bottom
	draw_right_back_top
end

def draw_right_back_top
	pt = @element.definition.bounds.corner BB_RIGHT_BACK_TOP
	point = pt.transform(@element.transformation)  
	ents = Sketchup.active_model.entities

    #X_VEC = [1,0,0] and X_VEC_R = [-1,0,0], RADIUS = 3

	arc1 = ents.add_arc point, X_VEC_R, Z_VEC, RADIUS, 0, 90.degrees
	arc2 = ents.add_arc point, X_VEC_R, Y_VEC, RADIUS, 0, -90.degrees
	arc3 = ents.add_arc point, Y_VEC_R, X_VEC, RADIUS, 0, 90.degrees

	line1 = ents.add_line point, [point[0] - RADIUS, point[1], point[2]]
	line2 = ents.add_line point, [point[0], point[1] - RADIUS, point[2]]
	line3 = ents.add_line point, [point[0],point[1], point[2] - RADIUS]

	create_faces arc1, arc2, arc3
	group = ents.add_group arc1, arc2, arc3, line1, line2, line3, line1.faces, line2.faces
	@group_entities << group.entities.map {|e| e}
end 
#and the others "draw_each_corner" like above.
def create_faces arc1, arc2, arc3
	arc1.map {|a| a.find_faces}
	arc2.map {|a| a.find_faces}
	arc3.map {|a| a.find_faces}
end

I know that need to refactor a lot of thing, but the point is what could be improved to draw 8 corners and what could be improved in Tool too, like the states, transactions, observers, etc…

Thanks a lot.

2

(1) use 2 spaces for Ruby code indents (you have 4 and 8 space idents.)

(2) You keep re-creating objects many times. Move creation to the tool’s activate() method if possible:

def activate()
  @view = Sketchup.active_model.active_view
  @ph = @view.pick_helper
end

(3) There is a lot of code in the onMouseMove callback that is not being used. For example, you are creating local variables, and doing nothing with them.
Keep code within onMouseMove to the very minimum. Whenever the user moves the mouse, the SketchUp engine redraws the UI. This means it redraws all the toolbar buttons and runs their validation procs (at least on MS Windows.) Not just your tool’s toolbars.

(4) Try making the tool do the picking inside onLButtonDown() when state==0 then increment state unless @element.nil? :

def onLButtonDown(flags, x, y, view)
  if @state==0
    @ph.do_pick( x,y )
    @element = ph.best_picked
    if @element &&
    @element.is_a?(Sketchup::ComponentInstance)
      @state = 2
     view.refresh # was: draw_selectable_corners()
    end
  elsif @state==2
    # user picked a hotspot
  end
  # was: view.refresh
end

EDIT: I made an error, the view.refresh call goes up where the draw_selectable_corners() call is.

1 Like
3

This seems a like a situation where drawing temporary graphics would out-perform real geometry. This is done in the draw method[1] of a Tool using the View.draw methods.[2] Temporary graphics are fast, do not interact with real geometry, and do not need to be explicitly deleted or hidden when exiting the Tool.

[1] Homepage | SketchUp Developer

[2] Homepage | SketchUp Developer

1 Like
4

I don’t fully get what these corner markers are for…

BUT… If the corners are always the same why not make a single SKP component for a corner, kept in your plugin’s subfolder, load it if it’s not already in the model from an earlier operation, then add 8 instances onto the bb corners [inside a new @group to keep it tidy], each corner-component-instance can be placed by its origin on the bb’s corner point; the 0 one will only need a point transformation to locate it, the others need to be located at the other points and suitably oriented in a 3d rotation transformation about that in 90/180/270.degree.angle-steps, in X/Y/Z, to suit

If they are temporary markers then delete the @group as the tool exits.

Using a single component reduces the model’s geometry, but keep the number of segments in the arcs low, you currently do not set it so they are 90.degrees with the default number - 12, a default circle only has 24 so you are using twice as many segments as a default circle - try using 6 - in the existing code approach OR when manually making the SKP marker… Currently the marker has (1+12)*3 edges [39, *8==312] - with 6 segments it reduces to (1+6)*3 edges [21. *8==168]… as raw geometry - or just 21 as instances.
You can also set the instances NOT to cast/receive shadows etc to further reduce impact on performance…

1 Like
5

Yes, I’m using 2 :grin: , but when copy and paste code here, it changes.

Omg, redraw all toolbar? That’s new for me, I’ll be more careful from now on. And less code within onMouseMove.

This will change the tool approach. I need every time the mouse is over the component, those hotspot appears and when the mouse is out the hotspot go away. But I appreciate the tip.

Sorry, I haven’t mentioned that the tool is for align components based on those hotspot. Mouse over component, hotspot appears, choose one, then mouse over another componente, hotspot appears on this second component, choose another one, then the components are aligned by this hotspots, side by side.

Thanks for the help Dan.

6

Well, that’s very interesting, thanks for sharing.

7

Sorry, I haven’t mentioned that. It’s for align based on hotspot, just like I answered to Dan.

Never thought in reload a skp component, but, as it’s a temporary thing, maybe I’ll not use this now (my bad not mentioned the tool objective), thanks anyway for this information.
The ideia of just rotate the corners is so simple that I was ashamed. lol.

Understood.

Alright, very valuable information, 312 to 168 it’s a lot of difference.

Thanks for tips TIG.

8

What about put in onMouseMove method, an case statement to control the tool flow? Too heavy?
Ex:

def onMouseMove(flags, x, y, view)
  @ph.do_pick x, y
  @element = @ph.best_picked

  case @state
  when 0
    if @element.is_a? Sketchup::ComponentInstance
      @state = 1
      @view.refresh
    end
   when 1
     if @element.is_a? NilClass
        reset
     end
	
   when 2
     [....]
   end
end

Is that ok?
What is the best pratical to code in onMouseMove method?

9

Probably because they are tab characters. My editor (Notepad++) can dynamically replace tabs with spaces.

if @element.nil?

better than:

if @element.is_a? NilClass

Reason, all Object subclasses (which are all classes,) inherit a .nil? method.
NilClass is a singleton class, with nil as the singleton instance, so it doesn’t make much sense to test references with is_a?/kind_of?,…
and lastly it reads better.

Also you can use if and unless in modifier position (after a statement.)

reset if @element.nil?

better than:

if @element.nil?
  reset
end
10

:grin: my bad. (I’m using Sublime)

"tab_size": 2,
"translate_tabs_to_spaces": false

Great.

Many thanks Dan.

You guys are awesome, hope someday could contribute to community too.
I owe you a beer.