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

 
 

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

Submitted by:  Rik <rik5>
Submitted on:  Thu 12 May 2016 04:33:13 PM UTC  
 
Category: Plotting with gnuplotSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Regression
Status: FixedAssigned to: None
Originator Name: Open/Closed: Closed
Release: devOperating System: GNU/Linux

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 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>
Project Administrator
Tue 17 May 2016 12:13:23 AM UTC, comment #35:

I've pushed both changesets. We're ready for others to test.

http://hg.savannah.gnu.org/hgweb/octave/rev/68522e5b74ef

http://hg.savannah.gnu.org/hgweb/octave/rev/db71f8a7d896

Ben Abbott <bpabbott>
Project 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>
Project 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.

Ben Abbott <bpabbott>
Project 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>
Project 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>
Project 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:

(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>
Project 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 ...
to

Ben Abbott <bpabbott>
Project 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

Ben Abbott <bpabbott>
Project 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>
Project 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,

does make the lines red.

Rik <rik5>
Project 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.

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

Ben Abbott <bpabbott>
Project 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>
Project 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

or

Rik <rik5>
Project 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

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>
Project Member
Sat 14 May 2016 02:41:42 AM UTC, comment #12:

@Dan,

I suspect the problem is with with _gnuplot_draw_axes_
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>
Project Member
Sat 14 May 2016 12:43:08 AM UTC, comment #11:

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

@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>
Project 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:

Works for me.

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

I tried ...

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>
Project 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>
Project 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>
Project 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:

In 4.0.2 it works fine.

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

Using bisection, the cset that introduced the problem was

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>
Project 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>
Project Administrator
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.

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

Rik <rik5>
Project Administrator

 

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

Attach File(s):
   
   
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
  • -unavailable- added by bpabbott (Posted a comment)
  • -unavailable- added by sebald (Posted a comment)
  • -unavailable- added by avinoam (Posted a comment)
  • -unavailable- added by rik5
  • -unavailable- added by rik5
  • -unavailable- added by mtmiller (Posted a comment)
  • -unavailable- added by rik5 (Submitted the item)
  •  

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

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 15 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Fri 20 May 2016 05:44:06 PM UTCrik5StatusReady For Test=>Fixed
      Open/ClosedOpen=>Closed
    Tue 17 May 2016 12:13:23 AM UTCbpabbottStatusConfirmed=>Ready For Test
    Mon 16 May 2016 12:36:36 AM UTCbpabbottAttached File-=>Added octave-gnuplot_surf_edgecolor-djs2016may17-v2.patch, #37172
      Attached File-=>Added changeset.patch, #37173
    Sun 15 May 2016 09:51:19 PM UTCsebaldAttached File-=>Added octave-gnuplot_surf_edgecolor-djs2016may17.patch, #37168
    Sun 15 May 2016 09:37:28 PM UTCsebaldAttached File-=>Added octave-gnuplot_surf_edgecolor-djs2016may16.patch, #37167
    Sun 15 May 2016 09:17:32 PM UTCsebaldAttached File-=>Added octave-gnuplot_surf_edgecolor-djs2016may15.patch, #37166
    Sun 15 May 2016 07:09:58 PM UTCbpabbottAttached File-=>Added changeset.patch, #37162
    Sat 14 May 2016 04:09:38 PM UTCrik5Summarysombrero in gnuplot ignores colormap=>edges of mesh visible with flat shading and gnuplot toolkit
    Sat 14 May 2016 06:54:20 AM UTCsebaldAttached File-=>Added octave-gnuplot_facecolor-djs2016may14.patch, #37154
    Fri 13 May 2016 04:17:16 AM UTCrik5Carbon-Copy-=>Added sebald
      Carbon-Copy-=>Added bpabbott
    Thu 12 May 2016 11:10:59 PM UTCmtmillerStatusNone=>Confirmed
    Thu 12 May 2016 04:33:13 PM UTCrik5Attached File-=>Added sombrero.png, #37144

    Back to the top


    Powered by Savane 3.1-cleanup1