[Talk] Qpik::SimpleRays Overhaul

Dan Rathbun

Qpik::SimpleRays

Jakub Kupikowski (qpik) is not a programmer, so I overhauled
his plugin so it can be removed from the Quarantine List.
(See original posting in Plugins forum for info.)

v 1.0.1 : 2013-01-05

Corrected @@lang assignment in language file rescue block.

simple_rays.rb: lines 109 and 111
@@lang == 'en' %(#008000)[# just use English]
should be making an assignment:
@@lang **=** 'en' %(#008000)[# just use English]

v 1.0.0 : 2013-01-04

First release.

Notes:

All code within the Qpik::SimpleRays namespace
No longer modifies Ruby nor API classes.
Now cleanups the temporary image files and dirs after itself.
Remembers inputbox entries during the session
commented out the purge all materials
multi-language

What I did not do:

Re-write methods for optimization

It is likely the code could benefit from some re-write, for instance I think the whole Bitmap#lbo_dword() method could be replaced with inline use of [n].pack("V") ?

Dan Rathbun

Overhaul of qpik's SimpleRays plugin by Dan Rathbun, 2013-01-03

MISC:

a) Formalized versioning and added version entries to doc header.

(The previous 7 versions are numnered 0.1.0 thru 0.7.0)

b) This overhaul will be version %(#004040)[**1.0.0**]

GENERAL FIXES & CHANGES TO AVOID BAD CODING STYLES

1) Use of Ruby keywords or common method identifiers:

~ Use of "method" as local var replaced with "meth"

2) Use of local varname same as the method name it is within.

(We try not to make the Ruby interpreter do more work by having to determine whether we are making a recursive call, or making an assignment to a local variable. Also bad style.)
~ local var names replaced with shorter nicknames

3) Use of do keyword with for .. in statements.

(Although most Ruby books say this is allowed, it is uneeded because the " for" keyword is the block opener. The Ruby interpreter often in older versions, raises SyntaxError when " do" is used this way. The interpreter expects an " end" to match the " for" and another " end" to match the " do".)
~ removed all unneeded " do" keywords from for.. in statements.

4) ALL code wrapped within Qpik::SimpleRays namespace !!!

**~** defining global methods in the ` ObjectSpace` is a NO-NO !
  (Everyone else's classes and modules inherit them!)

5) Eliminated all use of $ global variables !

**~** There is NEVER a good reason to clutter the global ` ObjectSpace` with variables that a only specific plugin uses!

(Instead, wrap code in a module namespace hierarchy and use class / module @@ vars, or constants within your namespaces.)

6) Where possible, each() iterators replaced with faster for .. in.

7) Indentation:

**~** Weird odd indents (ie, 1 space, 3 space, etc.,) replaced with conventional Ruby 2 space indents.
**~** Removed ALL outdenting. (Bad Style, makes blocks hard to read.)

Plugin is within it's own sub-directory of "Qpik" directory.

FIXED RUBY VERSION COMPATIBILITY ISSUES

a) Parenthesis ()

**~** Added parentheses around argument lists where appropriate, and
  definately when argument list contains more than one parameter.
  (Readability and future Ruby version Compatibility.)

b) id() vs object_id()

**~** ` id()` is deprecated and outputs a warning to ` $stdout` on each call. Replaced all uses with ` object_id()`

FIXED ILLEGAL ADDITION OF CUSTOM METHODS TO API & BASE CLASSES

[ruby:jzpz62dh]class Array[/ruby:jzpz62dh] methods:

[ruby:jzpz62dh]exposure()[/ruby:jzpz62dh]

moved into plugin module
[ruby:jzpz62dh]xyz_array[/ruby:jzpz62dh] is now passed in as 1st argument
[ruby:jzpz62dh]frontside()[/ruby:jzpz62dh]
renamed to [ruby:jzpz62dh]frontside_vector_array()[/ruby:jzpz62dh]
moved into plugin module
[ruby:jzpz62dh]vector_array[/ruby:jzpz62dh] is passed in as 2nd argument
[ruby:jzpz62dh]get_bounds_2d()[/ruby:jzpz62dh]
moved into plugin module
[ruby:jzpz62dh]array_of_xy_arrays[/ruby:jzpz62dh] is now passed in
internal min & max calculations have been replaced with [ruby:jzpz62dh]min()[/ruby:jzpz62dh] & [ruby:jzpz62dh]max()[/ruby:jzpz62dh] from [ruby:jzpz62dh]Enumerable[/ruby:jzpz62dh]
[ruby:jzpz62dh]get_point2d()[/ruby:jzpz62dh]
moved into plugin module
[ruby:jzpz62dh]point[/ruby:jzpz62dh] is passed a 1st argument
[ruby:jzpz62dh]get_point3d()[/ruby:jzpz62dh]
moved into plugin module
[ruby:jzpz62dh]point[/ruby:jzpz62dh] is passed a 1st argument
[ruby:jzpz62dh]move()[/ruby:jzpz62dh]
removed (was not used, but if needed later can be replaced with API bulit-in [ruby:jzpz62dh]Array.offset[/ruby:jzpz62dh] method)
[ruby:jzpz62dh]move!()[/ruby:jzpz62dh]
replaced with API bulit-in [ruby:jzpz62dh]Array.offset![/ruby:jzpz62dh] method
[ruby:jzpz62dh]round_to()[/ruby:jzpz62dh]
replaced with: [ruby:jzpz62dh]Array.map {|n| round_f(n,x) }[/ruby:jzpz62dh]
it was only called twice from [ruby:jzpz62dh]lightmap()[/ruby:jzpz62dh]
see [ruby:jzpz62dh]Float#round_to()[/ruby:jzpz62dh] which was renamed plugin method [ruby:jzpz62dh]round_f()[/ruby:jzpz62dh]
[ruby:jzpz62dh]scale()[/ruby:jzpz62dh]
replaced with an Array scaling transformation
used in only in [ruby:jzpz62dh]Bitmap#blur()[/ruby:jzpz62dh]
[ruby:jzpz62dh]to_i()[/ruby:jzpz62dh]
renamed [ruby:jzpz62dh]intized_array()[/ruby:jzpz62dh]
moved into [ruby:jzpz62dh]Qpik::SimpleRays::Bitmap[/ruby:jzpz62dh] class
[ruby:jzpz62dh]xyz_array[/ruby:jzpz62dh] is passed in as argument
called only by [ruby:jzpz62dh]Bitmap#blur()[/ruby:jzpz62dh]

[ruby:jzpz62dh]class Bitmap[/ruby:jzpz62dh]

moved within the Qpik::SimpleRays namespace
(The global ObjectSpace is where Ruby defines it's classes, that EVERYONE will use. Custom classes should ALWAYS be defined within your namespace.)

[ruby:jzpz62dh]blur()[/ruby:jzpz62dh]

!! Bad style: making assignments within boolean expressions !!

(These assignments have NOT been changed.)
[ruby:jzpz62dh]write()[/ruby:jzpz62dh]

wrapped file handling in [ruby:jzpz62dh]begin[/ruby:jzpz62dh] .. [ruby:jzpz62dh]rescue[/ruby:jzpz62dh] .. [ruby:jzpz62dh]ensure[/ruby:jzpz62dh] block.
[ruby:jzpz62dh]to_byte()[/ruby:jzpz62dh]
renamed --> [ruby:jzpz62dh]lbo_dword()[/ruby:jzpz62dh]

(It does not output a byte.
Outputs a 4-byte (32bit) DWORD String, in little-endian byte order.)

!!!! this method might be replaced with: [ruby:jzpz62dh][n].pack("V")[/ruby:jzpz62dh]
[ruby:jzpz62dh]from_byte()[/ruby:jzpz62dh]
NOT USED (commented out)

[ruby:jzpz62dh]class Float[/ruby:jzpz62dh] methods:

[ruby:jzpz62dh]round_to()[/ruby:jzpz62dh]

moved into plugin module
renamed [ruby:jzpz62dh]round_f()[/ruby:jzpz62dh]
number [ruby:jzpz62dh]n[/ruby:jzpz62dh] is passed in as 1st argument

[ruby:jzpz62dh]class Numeric[/ruby:jzpz62dh] methods:

[ruby:jzpz62dh]rgb()[/ruby:jzpz62dh]

moved into plugin module
number is passed in as 1st argument

[ruby:jzpz62dh]class Sketchup::Face[/ruby:jzpz62dh] methods:

[ruby:jzpz62dh]lightmap()[/ruby:jzpz62dh]

moved into plugin module
[ruby:jzpz62dh]face[/ruby:jzpz62dh] is passed in as 1st argument

[ruby:jzpz62dh]class Sketchup::Model[/ruby:jzpz62dh] methods:

[ruby:jzpz62dh]raytest2()[/ruby:jzpz62dh] : removed (was not used)
[ruby:jzpz62dh]to_a()[/ruby:jzpz62dh] : removed (was empty, and not used.)

FIXED PLUGIN METHODS:

+ Menu command is now a [ruby:jzpz62dh]UI::Command[/ruby:jzpz62dh] object with a validation proc that grays the menu item if the selection is empty.

~ Creation of command and context menu handler wrapped within a unless [ruby:jzpz62dh]file_loaded?()[/ruby:jzpz62dh] block.

~ These methods were global, now defined within plugin module:

[ruby:jzpz62dh]raytest()[/ruby:jzpz62dh]

+ short-circuit return [ruby:jzpz62dh]nil[/ruby:jzpz62dh] if [ruby:jzpz62dh]params[/ruby:jzpz62dh] from inputbox is false (ie, the user cancelled the inputbox.)
~ The user's previous working Dir is remembered in [ruby:jzpz62dh]@@prevDir[/ruby:jzpz62dh], and restored within an ensure clause.
+ Temporary image files and directories are cleaned up (This cleanup is disabled when debug mode is on.)
~ Untitled (unsaved) models now use "[ruby:jzpz62dh]~/_temp_simple_rays[/ruby:jzpz62dh]" as temp image directory. (Previously they used a temporary dir beneath the application "plugins" directory.)
[ruby:jzpz62dh]raytest_dialog()[/ruby:jzpz62dh]

**+** remembers the last used options in [ruby:jzpz62dh]@@def_opts[/ruby:jzpz62dh]

ADDITIONS:

+ MULTI-LANGUAGE SUPPORT

+ [ruby:jzpz62dh]SketchupExtension[/ruby:jzpz62dh] loader file "[ruby:jzpz62dh]load_qpik_simple_rays.rb[/ruby:jzpz62dh]"

(The above list is supplied with the code as "v1.0.0_overhaul.txt")

Jim

@anton_s said:

n existing function Kernel.sprintf can round a number

Formatting (sprintf) is not the same as rounding. sprintf is for formatting strings and is not a substitute for rounding.

Also, you know by now not to add methods to the base classes!

Aerilius

I think string manipulation would also probably be a lot slower. (otherwise very creative thought outside the box)

Dan Rathbun

@aerilius said:

.... (otherwise very creative thought outside the box)

Yes .. I do not wish to discourage you (Anton, or anyone,) from making suggestions!

All optimization suggestions are welcome.

The code could REALLY benefit from more overhaul for speed.

Maybe I should put this up on GitHub ... with Jakub's permission, of course. (He lst logged into SCF a week ago, and has not yet gotten my PM about this overhaul.)
I am wanting him to weight in on this ...

Dan Rathbun

@anton_s said:

qpik's round_f function isn't proper. He could avoid math operations just by ...

(1) The basic mathematical operations are built-in to every CPU and programming language, and are FAST

(2) String operations will always be MUCH slower than math operations (Agreeing with Aerilius.)

(3) qpik got the statement (n * 10**x).round.to_f / 10**x from one of our (Jim, TIG or myself's) postings here on the subject of rounding Float objects.

**%(#BF4000)[*ADD ref posts:*]*** (by TIG) Re: Is there a ceiling function like in excel?

(by me) Re: Rounding

(4) Agreeing with Jim, why did you post code that modifys a Ruby base class ( Numeric,)

... when the whole point of overhauling the SimpleRays code, was NOT to modify Ruby base classes and SketchUp API classes ??

Incidentally, Ruby 1.9.x has the decplaces argument added to the Numeric#round() method. Hopefully the next SketchUp version will use one of the Ruby releases in the 1.9 trunk.

In the meantime, I think it would be better to use the Backport gem, if you must have 1.9.x features NOW.

Dan Rathbun

@jim said:

@anton_s said:

... function Kernel.sprintf can round a number

Formatting ( sprintf) is not the same as rounding. sprintf is for formatting strings and is not a substitute for rounding.

To be fair ... Anton is correct.

Kernel.sprintf(), as well as *format_string*.%() both WILL INDEED round when a precision "dot" is used in the format string.

Anton_S

Nice Work Dan!!!

A suggestion:

Qpic's round_f function isn't proper. He could avoid math operations just by using an existing function.
An existing function Kernel.sprintf can round a number to any positive precision and amazing it's availiable in ruby 1.8.x.

After a bit of experience, I written two round_to methods wrapping the Kernel.sprintf.

<span class="syntaxdefault">module AMS</span><span class="syntaxkeyword">;</span><span class="syntaxdefault"> end </span><span class="syntaxcomment"># Top namespace<br /><br /></span><span class="syntaxdefault">module AMS</span><span class="syntaxkeyword">;;</span><span class="syntaxdefault">Numeric<br /><br />  </span><span class="syntaxcomment"># Rounds num to a given precision in decimal digits (default 0 digits).<br /></span><span class="syntaxdefault">  </span><span class="syntaxcomment"># Retrieves value in numeric (int or float)<br /></span><span class="syntaxdefault">  def self</span><span class="syntaxkeyword">.</span><span class="syntaxdefault">round_to</span><span class="syntaxkeyword">(</span><span class="syntaxdefault">num</span><span class="syntaxkeyword">,</span><span class="syntaxdefault"> ndigits </span><span class="syntaxkeyword">=</span><span class="syntaxdefault"> 0</span><span class="syntaxkeyword">)<br /></span><span class="syntaxdefault">    n </span><span class="syntaxkeyword">=</span><span class="syntaxdefault"> ndigits</span><span class="syntaxkeyword">.</span><span class="syntaxdefault">to_i</span><span class="syntaxkeyword">.</span><span class="syntaxdefault">abs<br />    x </span><span class="syntaxkeyword">=</span><span class="syntaxdefault"> Kernel</span><span class="syntaxkeyword">.</span><span class="syntaxdefault">format</span><span class="syntaxkeyword">(</span><span class="syntaxstring">"%.#{n}f"</span><span class="syntaxkeyword">,</span><span class="syntaxdefault"> num</span><span class="syntaxkeyword">)</span><span class="syntaxdefault"> </span><span class="syntaxcomment"># Kernel sprintf - format string<br /></span><span class="syntaxdefault">    x </span><span class="syntaxkeyword">=</span><span class="syntaxdefault"> </span><span class="syntaxkeyword">((</span><span class="syntaxdefault">ndigits </span><span class="syntaxkeyword">==</span><span class="syntaxdefault"> 0</span><span class="syntaxkeyword">)</span><span class="syntaxdefault"> </span><span class="syntaxkeyword">?</span><span class="syntaxdefault"> x</span><span class="syntaxkeyword">.</span><span class="syntaxdefault">to_i </span><span class="syntaxkeyword">;</span><span class="syntaxdefault"> x</span><span class="syntaxkeyword">.</span><span class="syntaxdefault">to_f</span><span class="syntaxkeyword">)<br /></span><span class="syntaxdefault">    x </span><span class="syntaxkeyword">=</span><span class="syntaxdefault"> x</span><span class="syntaxkeyword">.</span><span class="syntaxdefault">abs if x</span><span class="syntaxkeyword">.</span><span class="syntaxdefault">zero</span><span class="syntaxkeyword">?</span><span class="syntaxdefault"> </span><span class="syntaxcomment"># remove (-) sign if x is (-0.0 or -0)<br /></span><span class="syntaxdefault">    return x<br />  end </span><span class="syntaxcomment"># def round_to<br /><br /></span><span class="syntaxdefault">  </span><span class="syntaxcomment"># Retrieves value in string form<br /></span><span class="syntaxdefault">  def self</span><span class="syntaxkeyword">.</span><span class="syntaxdefault">round_to2</span><span class="syntaxkeyword">(</span><span class="syntaxdefault">num</span><span class="syntaxkeyword">,</span><span class="syntaxdefault"> ndigits </span><span class="syntaxkeyword">=</span><span class="syntaxdefault"> 0</span><span class="syntaxkeyword">)<br /></span><span class="syntaxdefault">    n </span><span class="syntaxkeyword">=</span><span class="syntaxdefault"> ndigits</span><span class="syntaxkeyword">.</span><span class="syntaxdefault">to_i</span><span class="syntaxkeyword">.</span><span class="syntaxdefault">abs<br />    x </span><span class="syntaxkeyword">=</span><span class="syntaxdefault"> Kernel</span><span class="syntaxkeyword">.</span><span class="syntaxdefault">format</span><span class="syntaxkeyword">(</span><span class="syntaxstring">"%.#{n}f"</span><span class="syntaxkeyword">,</span><span class="syntaxdefault"> num</span><span class="syntaxkeyword">)</span><span class="syntaxdefault"> </span><span class="syntaxcomment"># Kernel sprintf - format string<br /></span><span class="syntaxdefault">    x </span><span class="syntaxkeyword">=</span><span class="syntaxdefault"> x</span><span class="syntaxkeyword">.</span><span class="syntaxdefault">delete</span><span class="syntaxkeyword">(</span><span class="syntaxstring">"-"</span><span class="syntaxkeyword">)</span><span class="syntaxdefault"> if x</span><span class="syntaxkeyword">.</span><span class="syntaxdefault">to_f</span><span class="syntaxkeyword">.</span><span class="syntaxdefault">zero</span><span class="syntaxkeyword">?</span><span class="syntaxdefault"> </span><span class="syntaxcomment"># remove (-) sign if x is (-0.0 or -0)<br /></span><span class="syntaxdefault">    return x<br />  end </span><span class="syntaxcomment"># def round_to2<br /><br /></span><span class="syntaxdefault">end </span><span class="syntaxcomment"># AMS;;Numeric &nbsp;</span><span class="syntaxdefault"></span>

Anton_S

@jim said:

Also, you know by now not to add methods to the base classes!

Changed it!

@aerilius said:

I think string manipulation would also probably be a lot slower

I compared the timing using ruby console+, but got random and similiar results.

In any way, how do you know that Kernel.format string is slower than few statements of math operations?
Perhaps, format string is the ruby native function that was written in c++ and compiled?

Dan, I'm sorry for flooding the topic with questions
You can move the posts to new developers topic or remove them

Dan Rathbun

@anton_s said:

In any way, how do you know that string functions is slower than few statements of math operations?

Experience. 36 years engineering, 30 years using, programming, repairing and building computers.
And every time we test Ruby string methods against other means (logical or math, etc.,) the string methods are slower. Maybe things will be faster in Ruby 1.9.x, as I believe the core team has done alot of optimization.

@anton_s said:

Perhaps, format string is the ruby native function that was written in c++ and compiled?

Anton, ALL MRI Ruby functions are written in C (not C++,) and they are ALL compiled into the interpreter DLL.

Dan Rathbun

v1.0.1 download Moved to Plugins forum.

Old v1.0.0 deleted.

THIS thread retitled as "[Talk] Qpik::SimpleRays Overhaul"