bugGNU Octave - Bugs: bug #47911, edges of mesh visible with flat...

 
 

bug #47911: edges of mesh visible with flat shading and gnuplot toolkit

Submitter:  Rik <rik5>
Submitted:  Thu 12 May 2016 04:33:13 PM UTC
   
 
Category:  Plotting with gnuplot Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Regression
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 20 May 2016 05:44:06 PM UTC, comment #36: 

I've tested a bit and it seems like a good addition.  I'm marking this as fixed and closing the report.

I did run through a number of the demos both in gnuplot and Qt and I see some differences.  I'll file another bug report about those since they are really a separate issue.  I'll add both Dan and Ben to the CC list for that report since you two have the most experience with that backend.  But you can remove yourself later if it isn't right.

Rik <rik5>
Group administrator
Tue 17 May 2016 12:13:23 AM UTC, comment #35: 
Ben Abbott <bpabbott>
Group Member
Mon 16 May 2016 10:54:02 AM UTC, comment #34: 

Ok. If there are no regressions, then I think the changesets can be pushed (file #37172, file #37173).

I expect this is a safe change (pushing is safer than not). I'll give Rik some time to comment. If no one beats me to it, I'll push late today.

Ben Abbott <bpabbott>
Group Member
Mon 16 May 2016 02:41:35 AM UTC, comment #33: 

Wow, that's quite a lot of demos.  It's difficult to go through them comparing one against another for the gnuplot and qt toolkits, so just some observations from memory.

1) There is a lot of flashing going on for the animation demos and some of the examples probably used 'hold' or something because it takes like three of four fast redraws for the eventual plot configuration to appear.  This applies to qt as well as gnuplot.

2) Generally, as far as color of lines, edges, faces, etc., the gnuplot demos look to match the qt demos pretty well.  I can't see anything that stands out that might be an issue with what is entailed here.

3) The biggest problems with gnuplot demos are mainly the layout of plots, boxes and text.

4) There are about a half dozen gnuplot plots that do have problems, and I think most are related to improper removal of hidden surfaces.  Sometimes gnuplot doesn't get that quite right, and it's because it doesn't yet have a true hidden surface algorithm, just hidden mesh is implemented.

Dan Sebald <sebald>
Mon 16 May 2016 01:33:37 AM UTC, comment #32: 

You can use the rundemos function.


cd scripts/plot
rundemos appearance
rundemos draw
rundemos util


Ben Abbott <bpabbott>
Group Member
Mon 16 May 2016 01:28:14 AM UTC, comment #31: 

Thanks Ben.  I applied your shading mods and all looks fine.  Is there a list of plot demos?

Dan Sebald <sebald>
Mon 16 May 2016 12:39:50 AM UTC, comment #30: 

I forgot to mention that Dan's change looks good to me. Perhaps we should run through all the plot demos just to make sure.

Ben Abbott <bpabbott>
Group Member
Mon 16 May 2016 12:36:36 AM UTC, comment #29: 

I found another missing semicolon (scmd = "palette"). I'm also attached a changeset that add demos to shading.m. The new demos reverse the values for edgecolor and facecolor.

(file #37172, file #37173)

Ben Abbott <bpabbott>
Group Member
Sun 15 May 2016 09:51:19 PM UTC, comment #28: 

Sorry.  I forgot to replace a couple semicolons so there was some variables printed out.  A new patch is attached.

(file #37168)

Dan Sebald <sebald>
Sun 15 May 2016 09:37:28 PM UTC, comment #27: 

Well, that was easy.  The facecolor_none_or_white variable ends up not even being used anymore.  Attached is an update to the previous patch but with the above variable removed.

(file #37167)

Dan Sebald <sebald>
Sun 15 May 2016 09:17:32 PM UTC, comment #26: 

I'm attaching a first draft of a patch.  It's not as bad as it looks.  Mainly I regrouped the code so that the color-based edge was not intertwined with the face (pm3d) code.  That fact meant the two couldn't coexist (e.g., a 'flat' edge with a fixed face color...btw, there is no edge color interpolation as far as I know.  Only the pm3d/face color has interpolation).

This patch is a first draft.  I want to go back and look at this "none_or_white" stuff and determine if that is even relevant anymore now that (as Ben suggested) the 'none' case is handled simply by non-inclusion.

To test this, use the 'shading' command, and then alter shading.m something like the following:


diff --git a/scripts/plot/appearance/shading.m b/scripts/plot/appearance/shading
--- a/scripts/plot/appearance/shading.m
+++ b/scripts/plot/appearance/shading.m
@@ -82,15 +82,24 @@ function shading (varargin)
   ##hlist = [hp(:); hs(:)];

   switch (lower (mode))
+%    case "flat"
+%      set (hlist, "edgecolor", "none");
+%      set (hlist, "facecolor", "flat");
+%    case "interp"
+%      set (hlist, "edgecolor", "none");
+%      set (hlist, "facecolor", "interp");
+%    case "faceted"
+%      set (hlist, "edgecolor", [0 0 0]);
+%      set (hlist, "facecolor", "flat");
     case "flat"
-      set (hlist, "facecolor", "flat");
-      set (hlist, "edgecolor", "none");
+      set (hlist, "edgecolor", "flat");
+      set (hlist, "facecolor", "none");
     case "interp"
-      set (hlist, "facecolor", "interp");
-      set (hlist, "edgecolor", "none");
+      set (hlist, "edgecolor", "interp");
+      set (hlist, "facecolor", "none");
     case "faceted"
-      set (hlist, "facecolor", "flat");
-      set (hlist, "edgecolor", [0 0 0]);
+      set (hlist, "edgecolor", "flat");
+      set (hlist, "facecolor", [0 0 0]);
     otherwise
       error ('shading: Invalid MODE "%s"', mode);
   endswitch


(file #37166)

Dan Sebald <sebald>
Sun 15 May 2016 07:38:55 PM UTC, comment #25: 

Doesn't quite work Ben.  What I'm using for a test is the normal shading.m and then I also reverse the attributes of the edge and face.  The lines remain hidden even though there is no face, and the line color suffers the same bug that was solved in the previous patch.

I'm working on a patch that reorganizes somewhat.  I found that some stuff was redundant or non-reachable.  Just a few more mods and it should work...

Dan Sebald <sebald>
Sun 15 May 2016 07:09:58 PM UTC, comment #24: 

I'm attaching a simple patch. I don't have time to verify it doesn't break anything else until later. Rik/Dan, please take a look and comment.

(file #37162)

Ben Abbott <bpabbott>
Group Member
Sun 15 May 2016 06:39:33 PM UTC, comment #23: 

opps! Repeating comment #22 with corrected spelling.

My suggestion to not specify the linetype won't work. However, since the faces and the lines are drawn independently, the proper fix is simpler. Just don't draw the lines.
For edgecolor == "none", the fix for the gnuplot stream is to change ...

splot "-" binary format='%float64' record=49x49 using ($1):($2):($3):($4) title "" with pm3d \
, "-" binary format='%float64' record=49x49 using ($1):($2):($3) title "" with lines linetype 9 \
;

to

splot "-" binary format='%float64' record=49x49 using ($1):($2):($3):($4) title "" with pm3d \
;


Ben Abbott <bpabbott>
Group Member
Sun 15 May 2016 06:36:35 PM UTC, comment #22: 

My suggestion to not specify the linetype won't work. However, since the faces and the lines are drawn independently, the proper fix is simpler. Just don't draw the lines.

For edgecolor == "none", the fix for the gnuplot stream is to change ...
+verbation+
splot "-" binary format='%float64' record=49x49 using ($1):($2):($3):($4) title "" with pm3d \
, "-" binary format='%float64' record=49x49 using ($1):($2):($3) title "" with lines linetype 9 \
;
-verbatim-
to

splot "-" binary format='%float64' record=49x49 using ($1):($2):($3):($4) title "" with pm3d \
;




Ben Abbott <bpabbott>
Group Member
Sun 15 May 2016 02:53:07 PM UTC, comment #21: 

I'm away from my computer for most of the day. If someone doesn't get to it first, I'll try removing the linetype designation from the splot command, when the edgecolor is none.

I did run a few tests yesterday that indicated the edgecolor was consistent with the default color for the linetype used (which may vary with "term" specified for gnuplot)

Ben Abbott <bpabbott>
Group Member
Sun 15 May 2016 02:39:06 PM UTC, comment #20: 

I'm hoping that, as Dan says, there is simply a conditional do disable drawing of all lines when edgecolor is 'none'.  On the positive side, the edgecolor attribute is working correctly when the lines are supposed to be drawn.  For example,


set (h, 'edgecolor', 'r')


does make the lines red.


Rik <rik5>
Group administrator
Sun 15 May 2016 12:39:03 AM UTC, comment #19: 

In addition to Dan's changes, we also introduced gnuplot's new linetype syntax. For the demo, we're not specifying the linecolor for the linetype used to draw the surfac plot.


set linetype 9 default;
set linetype 9 dashtype solid linewidth 0.500000;


In any event, this may complicate a solution because more than one changeset is likely involved in creating this bug.

Ben Abbott <bpabbott>
Group Member
Sat 14 May 2016 08:03:27 PM UTC, comment #18: 

I have a fix for the shading demo (just add a conditional).  But for the demo I interchanged the "edgecolor" and "facecolor" settings and see some inconsistencies.  So, I'm going to work on this a bit more before attaching a patch, but I'll be out for most of the day.

Dan Sebald <sebald>
Sat 14 May 2016 06:18:45 PM UTC, comment #17: 

I see the same problem as Rik running gnuplot 5. 'faceted' looks correct, but 'flat' and 'interp' both have grey edge colors.

@Rik, should Dan's patch be pushed?

Ben Abbott <bpabbott>
Group Member
Sat 14 May 2016 04:09:38 PM UTC, comment #16: 

I checked in Dan's cset here (http://hg.savannah.gnu.org/hgweb/octave/rev/e6fcaea7d433).  It fixes the face color shading issue for me.

But before we're done here, I notice that there is a difference between the toolkits when running 'demo shading'.  For the 'flat' shading the edge colors of the mesh should be invisible.  Instead, for gnuplot they are a light grey.  I think this is probably intimately related to the fix for facecolor so I'm going to leave this bug report open and re-title it.

Sample code


graphics_toolkit gnuplot
demo ('shading')


or


graphics_toolkit gnuplot
demo ('pcolor', 1)
shading flat




Rik <rik5>
Group administrator
Sat 14 May 2016 06:54:20 AM UTC, comment #15: 

OK, I'm getting familiar with this again.  If I recall correctly, that original bug that motivated the color change was about face color (and black surface plots).  Hence the hunk of code Ben pointed out


1181                 ## Convert color to 24-bit RGB
1182                 zz(:,kk+3) = hex2dec (sprintf ("%02x%02x%02x",
1183                                                round (255*obj.facecolor)));


Apparently what is happening is the characters of the string "flat" are being treated as numerical RGB values and it somehow gets interpreted as bronze.

I'm attaching a patch that solves the immediate issue of the sombrero.  There's a lot going on here with so many figure properties.  There are bound to be more small tweaks like this one still needed for the code.


(file #37154)

Dan Sebald <sebald>
Sat 14 May 2016 02:55:13 AM UTC, comment #14: 

I'm compiling at the moment, so just running the gnuplot code from a file through gnuplot in the mean time.  When Octave compilation is complete I'll follow through the code to see what the issue is.

Anyway, yeah, this was a big changeset, and it was a new approach to the way "withpm3d" is addressed.  It wouldn't surprise me that I overlooked something in the original patch.

Dan Sebald <sebald>
Sat 14 May 2016 02:49:59 AM UTC, comment #13: 

Seems to be related to the bug report below.

https://savannah.gnu.org/bugs/?34523

see file #36502

Ben Abbott <bpabbott>
Group Member
Sat 14 May 2016 02:41:42 AM UTC, comment #12: 

@Dan,

I suspect the problem is with with _gnuplot_draw_axes_

1178               if (facecolor_none_or_white)
1179                 zz(:,kk+3) = cdat(:,k);
1180               else
1181                 ## Convert color to 24-bit RGB
1182                 zz(:,kk+3) = hex2dec (sprintf ("%02x%02x%02x",
1183                                                round (255*obj.facecolor)));
1184                endif

The obj.facecolor = "flat".

This code is part of the changeset below

http://hg.savannah.gnu.org/hgweb/octave/rev/21c3641422e0

If I recall correctly this is part of a large changeset that I split up. I apparently got something wrong. Do you have any insight here?

Ben Abbott <bpabbott>
Group Member
Sat 14 May 2016 12:43:08 AM UTC, comment #11: 

@Dan, I'm using the default branch with tip

$ hg tip
changeset:   21687:4418e579cca6
bookmark:    @
tag:         tip
user:        Mike Miller <mtmiller@octave.org>
date:        Wed May 11 13:12:22 2016 -0700
summary:     scanf: return maximum/minimum value on integer overflow (bug #47759)


@Avinoam, I tried your change in comment #9. The sombrero plot is still brown (bronze?), as it is for Dan. As Dan suggests, it appears that you are using a different release or repository version. However, I see in your diff that _gnuplot_drawnow_.m is dated May 11.

If not

Ben Abbott <bpabbott>
Group Member
Sat 14 May 2016 12:14:12 AM UTC, comment #10: 

After cleanup of the 'mono' variable code lines:

https://savannah.gnu.org/bugs/index.php?47912#comment2

I too am still seeing the bronze sombrero surface.  I'll debug.

A.K., what Octave version or repository version are you using?

Dan Sebald <sebald>
Fri 13 May 2016 10:08:19 PM UTC, comment #9: 



$ hg -v diff
diff -r 4418e579cca6 scripts/plot/util/__gnuplot_drawnow__.m
--- a/scripts/plot/util/__gnuplot_drawnow__.m        Wed May 11 13:12:22 2016 -0700
+++ b/scripts/plot/util/__gnuplot_drawnow__.m        Sat May 14 01:07:02 2016 +0300
@@ -23,7 +23,8 @@

 ## Author: jwe

-function __gnuplot_drawnow__ (h, term, file, debug_file)
+function __gnuplot_drawnow__ (h, term, file, mono, debug_file)
+

   if (nargin < 1 || nargin > 5 || nargin == 2)
     print_usage ();


Works for me.

Avinoam Kalma <avinoam>
Group Member
Fri 13 May 2016 07:03:27 PM UTC, comment #8: 

I tried ...


$ hg diff
diff --git a/libinterp/dldfcn/__init_gnuplot__.cc b/libinterp/dldfcn/__init_gnuplot__.cc
--- a/libinterp/dldfcn/__init_gnuplot__.cc
+++ b/libinterp/dldfcn/__init_gnuplot__.cc
@@ -110,8 +110,7 @@ public:
   {
     octave_value_list args;
     if (! debug_file.empty ())
-      args(4) = debug_file;
-    args(3) = mono;
+      args(3) = debug_file;
     args(2) = file;
     args(1) = term;
     args(0) = go.get_handle ().as_octave_value ();


This did not fix the color problem.

I'm running gnuplot 5. The displayed sombrero and the sombrero.png are both brown.


Ben Abbott <bpabbott>
Group Member
Fri 13 May 2016 06:04:35 PM UTC, comment #7: 

It does?  I hadn't noticed.  Maybe it is the "mono" misalignment causing problems.  I'm working on a patch for that right now.  I'll confirm.

Dan Sebald <sebald>
Fri 13 May 2016 05:51:25 PM UTC, comment #6: 

Dan's fix for drawnow (bug #47912) fixes also somebero problem.

Avinoam Kalma <avinoam>
Group Member
Fri 13 May 2016 11:06:26 AM UTC, comment #5: 

This may also depend upon the change below.

http://hg.savannah.gnu.org/hgweb/octave/rev/5b9618f4f841

If no one gets to it earlier, I'll take a look over the weekend.

Ben Abbott <bpabbott>
Group Member
Fri 13 May 2016 08:46:52 AM UTC, comment #4: 

A.K., I've come across the same issue with drawnow().  See

https://savannah.gnu.org/bugs/index.php?47912

for an easy mod to make it work for you.  I don't know what the intension of the change was, but someone should fix that bug soon in the repository.

Dan Sebald <sebald>
Fri 13 May 2016 08:25:29 AM UTC, comment #3: 

Moreover, trying to debug it with "drawnow", I get in the development branch:


>> drawnow ('postscript', 'd1.ps', 0, 'd1.gnu');
error: wrong type argument 'bool'
error: fopen: filename must be a string


In 4.0.2 it works fine.

Avinoam Kalma <avinoam>
Group Member
Fri 13 May 2016 04:17:16 AM UTC, comment #2: 

Using bisection, the cset that introduced the problem was


21373:21c3641422e0
 Fix gnuplot toolkit face/edgecolor and linestyle/width for surf() (bug #34523)


This was originally written by Dan Sebald, but modified and checked in by Ben Abbott.  I'm adding both to the CC list for this report in case one of them knows instantly what the problem is.

Rik <rik5>
Group administrator
Thu 12 May 2016 11:10:59 PM UTC, comment #1: 

Confirmed here also. With gnuplot 5 it is the monochrome ugly brown you attached. With gnuplot 4 the surface is all black. Both work correctly in Octave 4.0.1.

Mike Miller <mtmiller>
Group Member
Thu 12 May 2016 04:33:13 PM UTC, original submission:  

The following simple code on the development branch now produces a monochrome sombrero, rather than a 3-D surface colored according to the existing colormap.


./run-octave -f --no-gui
graphics_toolkit gnuplot
sombrero
print sombrero.png


This code works fine in version 4.0.2.  I've attached the file sombrero.png.

Rik <rik5>
Group administrator

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #37173:  changeset.patch added by bpabbott (3KiB - application/octet-stream)
file #37162:  changeset.patch added by bpabbott (1KiB - application/octet-stream)
file #37144:  sombrero.png added by rik5 (175KiB - image/png)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by bpabbott (Posted a comment)
  • -email is unavailable- added by sebald (Posted a comment)
  • -email is unavailable- added by avinoam (Posted a comment)
  • -email is unavailable- added by rik5
  • -email is unavailable- added by rik5
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by rik5 (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 15 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-05-20 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2016-05-17 bpabbott StatusConfirmed Ready For Test
    2016-05-16 bpabbott Attached File- Added octave-gnuplot_surf_edgecolor-djs2016may17-v2.patch, #37172
        Attached File- Added changeset.patch, #37173
    2016-05-15 sebald Attached File- Added octave-gnuplot_surf_edgecolor-djs2016may17.patch, #37168
    2016-05-15 sebald Attached File- Added octave-gnuplot_surf_edgecolor-djs2016may16.patch, #37167
    2016-05-15 sebald Attached File- Added octave-gnuplot_surf_edgecolor-djs2016may15.patch, #37166
    2016-05-15 bpabbott Attached File- Added changeset.patch, #37162
    2016-05-14 rik5 Summarysombrero in gnuplot ignores colormap edges of mesh visible with flat shading and gnuplot toolkit
    2016-05-14 sebald Attached File- Added octave-gnuplot_facecolor-djs2016may14.patch, #37154
    2016-05-13 rik5 Carbon-Copy- Added sebald
        Carbon-Copy- Added bpabbott
    2016-05-12 mtmiller StatusNone Confirmed
    2016-05-12 rik5 Attached File- Added sombrero.png, #37144

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code