bugGNU Octave - Bugs: bug #42006, set (h, "facelighting",...

 
 

bug #42006: set (h, "facelighting", "flat") renders dark surfaces/patches

Submitter:  Pantxo Diribarne <pantxo>
Submitted:  Sun 30 Mar 2014 09:30:55 PM UTC
   
 
Category:  Plotting with OpenGL Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 17 Mar 2016 04:43:51 PM UTC, comment #10: 

I pushed your patch after adding you as contributor and editing the changelog a bit.

http://hg.savannah.gnu.org/hgweb/octave/rev/385b67d48dda

Thanks Markus, I am closing this report.

Pantxo Diribarne <pantxo>
Group Member
Tue 15 Mar 2016 12:50:16 PM UTC, comment #9: 

Attached please find the latest version of the patch with your comments addressed. I hope the wording is OK?

(file #36650)

Markus Mützel <mmuetzel>
Group administrator
Tue 15 Mar 2016 12:25:30 PM UTC, comment #8: 

A few minor remaining issues:

Pantxo Diribarne <pantxo>
Group Member
Tue 15 Mar 2016 09:34:51 AM UTC, comment #7: 

Hi Pantxo,

thank you for your feedback. The changes you proposed seem much simpler and cleaner than my prior approach.

Attached please find the updated version of the patch.

(file #36646)

Markus Mützel <mmuetzel>
Group administrator
Mon 14 Mar 2016 08:05:22 PM UTC, comment #6: 

Hi,

I like this patch and the interpretation of the black surface.

The answer to your question in comments is no, you can't assume the parent of a patch/surface is an axes, it may also be a hggroup (and actually Octave doesn't currently have any guards against incorrect parenting, but this is another bug).

Instead of testing the presence of a light while drawing patches/surfaces, I find it simpler to declare a private bool property "has_light" that is set once in "draw_axes_children" (right after the comment "// 1st pass: draw light objects"). This way you don't have to mess with finding the parent axes (which BTW you could find using graphics_object::get_ancestor("axes")) for each patch/surface.

Pantxo Diribarne <pantxo>
Group Member
Mon 14 Mar 2016 05:47:46 PM UTC, comment #5: 

While working on a patch to implement lighting in Octave (please see patch #8943), I encountered the same problem.

It looks like "glEnable (LIGHTING)" should only be called for patches and surfaces if a light object is present in the axes. Otherwise, the objects are "rendered in the dark", hence black.

The attached patch checks the "Parent" of the "patch" or "surface" objects for "Children" of "Type" "light". Is this the correct way to check for other objects of a certain type in the same axes?
Only if a light object is present, lighting is activated.

As I am neither an expert in C++ nor OpenGL programming, I would appreciate any help, tips, info and opinions.

This change reflects the behavior of Matlab when "FaceLighting" is set to a value other than "none" and no light object is present.

The link in comment #4 gives an error 404 for me. Is there a newer link?

(file #36635)

Markus Mützel <mmuetzel>
Group administrator
Mon 31 Mar 2014 11:57:50 AM UTC, comment #4: 
Michael Goffioul <goffioul>
Mon 31 Mar 2014 11:35:33 AM UTC, comment #3: 

I see that transparency/lighting importation belongs to GSOC project ideas so I assume this bug won't be fixed with a two-liner patch.

Michael, could you post a link to the most up-to-date JHandles sources here please (I can't access the wiki but AFAIR the link there didn't work)?

Pantxo Diribarne <pantxo>
Group Member
Mon 31 Mar 2014 11:03:33 AM UTC, comment #2: 

Lightning (like transparency) hasn't been imported from JHandles yet.

Michael Goffioul <goffioul>
Mon 31 Mar 2014 05:01:20 AM UTC, comment #1: 

The only lighting option that works is 'none'.  I don't think there has been a lot of work on light objects for Octave.

Rik <rik5>
Group administrator
Sun 30 Mar 2014 09:30:55 PM UTC, original submission:  

Try


close all
z = peaks ();
su = surface (z);
set (su, 'facelighting', 'flat')


Pantxo Diribarne <pantxo>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #36650:  bug42006v3.patch added by mmuetzel (4KiB - application/octet-stream)
file #36646:  bug42006v2.patch added by mmuetzel (3KiB - application/octet-stream)
file #36635:  bug42006.patch added by mmuetzel (4KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-03-17 pantxo StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2016-03-15 mmuetzel Attached File- Added bug42006v3.patch, #36650
    2016-03-15 pantxo StatusConfirmed In Progress
    2016-03-15 mmuetzel Attached File- Added bug42006v2.patch, #36646
    2016-03-14 mmuetzel Attached File- Added bug42006.patch, #36635
    2014-03-31 rik5 StatusNone Confirmed

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code