patchGNU Octave - Patches: patch #9040, Add function "lighting"

 
 

patch #9040: Add function "lighting"

Submitted by:  Markus Mützel <mmuetzel>
Submitted on:  Tue 05 Jul 2016 07:15:13 PM UTC  
 
Category:  Core : new function Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  None Open/Closed:  Closed

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

Mon 18 Jul 2016 07:22:28 PM UTC, comment #8: 

Thank you, Rik, for pushing, for including the necessary changes and for the feedback.

I changed my local hg setup a few days ago to replace the ü with ue. So newer patches should hopefully apply with less trouble.

Markus Mützel <mmuetzel>
Project Member
Mon 18 Jul 2016 06:24:14 PM UTC, comment #7: 

@Markus: I pushed your patch here (http://hg.savannah.gnu.org/hgweb/octave/rev/18eae417ffec).

I had to remove the u umlaut in your name and replace with 'ue' because I was getting some crazy encoding error that prevented Mercurial from applying the patch

applying lighting
transaction abort!
rollback completed
cleaning up working directory...done
abort: decoding near 'Markus M�tzel <mar': 'utf8' codec can't decode byte 0xfc in position 8: invalid start byte!

Basically, I think you understand the Octave coding conventions, but here is what I changed:

1) "!CONDITION" => "! CONDITION".  In English, one can read this as two words "NOT" "CONDITION" so it makes it easier to understand.

2) "function_name (...)".  I found one instance of "cellfun(...)" which I changed to "cellfun (...)".

3) strcmp instead of strcmpi for all comparisons to graphics properties retrieved from a graphic object.  Internally, Octave normalizes all graphics properties to lower case so it is shorter code, and slightly less work for the CPU work, touse strcmp.  For properties that a user might enter, such as inputs to a function, there is no guarantee and there strcmpi is useful.

Finally, I added a BIST test for

figure
lighting flat

This failed in the original code because there were no mesh or patch objects, but strcmp ([], 'none') returns 0, not an empty matrix.  This then caused trouble later on.  I fixed the code by adding an early return if there are no objects to process.

if (isempty (hlist))
   return;
endif

Rik <rik5>
Project Administrator
Mon 11 Jul 2016 05:03:51 PM UTC, comment #6: 

Oops. Thank you, Colin, for spotting that one. I must have missed it after copying over most part of "shading.m".

Attached is a changeset with a fix for this.

(file #37808)

Markus Mützel <mmuetzel>
Project Member
Mon 11 Jul 2016 06:40:50 AM UTC, comment #5: 

texinfo function name is "shading" but should be lighting.

(I haven't tested, just read through the doc)

Colin Macdonald <cbm>
Thu 07 Jul 2016 02:40:14 AM UTC, comment #4: 

Oops.  I was misreading as "nargin != 0 || narg*in* != 1"

Lachlan Andrew <lachlan>
Wed 06 Jul 2016 05:32:39 PM UTC, comment #3: 

Lachlan, thank you for taking a look at that patch.

"lighting" must be called with one input variable (apart from the axes handle already stripped by _plt_get_axis_arg_) and no output variable. Everything else should get caught with the if statement you mentioned.

It seems to work as expected for me...

Markus Mützel <mmuetzel>
Project Member
Wed 06 Jul 2016 11:03:21 AM UTC, comment #2: 

Thanks for the patch, Markus.

Have you tried it?  I think "if ( nargin != 1 || nargout != 0 )" should always be true, and so the patch should always exit through print_usage.

Lachlan Andrew <lachlan>
Tue 05 Jul 2016 07:17:02 PM UTC, comment #1: 

Attached please find the patch.

(file #37724)

Markus Mützel <mmuetzel>
Project Member
Tue 05 Jul 2016 07:15:13 PM UTC, original submission:  

The function "lighting" can be used to change the "facelighting" and "edgelighting" properties of all patch and surface objects in one axes.

Most of the function is very similar to "shading" including the changes from bug #48392.

I will attach the patch as soon as I have the patch number.

Markus Mützel <mmuetzel>
Project Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #37808:  lighting_v2.patch added by mmuetzel (8KiB - text/x-diff)
file #37724:  lighting.patch added by mmuetzel (8KiB - text/x-diff)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by cbm (Posted a comment)
  • -email is unavailable- added by lachlan (Posted a comment)
  • -email is unavailable- added by mmuetzel (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-07-18 rik5 StatusNone => Done
        Open/ClosedOpen => Closed
    2016-07-11 mmuetzel Attached File- => Added lighting_v2.patch, #37808
    2016-07-07 lachlan CategoryNone => Core : new function
    2016-07-05 mmuetzel Attached File- => Added lighting.patch, #37724

    Back to the top


    Powered by Savane 3.5