bugGNU Octave - Bugs: bug #34417, patch() does not process 'cdata'...

 
 

bug #34417: patch() does not process 'cdata' correctly

Submitted by:  Rik <rik5>
Submitted on:  Wed 28 Sep 2011 04:24:16 PM UTC  
 
Category: PlottingSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Incorrect Result
Status: FixedAssigned to: Ben Abbott <bpabbott>
Originator Name: Open/Closed: Closed
Release: devOperating System: GNU/Linux

Add a New Comment (Rich MarkupRich 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)

Thu 29 Aug 2013 11:49:12 PM UTC, comment #20:

Re-visiting this bug report, I found that we got nearly everything right except for a typo in the input processing of 'agrs' instead of 'args'. With that fix (http://hg.savannah.gnu.org/hgweb/octave/rev/1391dc953ae2) I'm closing this report.

Rik <rik5>
Project Administrator
Fri 04 May 2012 05:19:15 PM UTC, comment #19:

Arnaud, for help please use email.

help-octave Octave <help-octave@octave.org>

Ben Abbott <bpabbott>
Project MemberIn charge of this item.
Fri 04 May 2012 05:15:17 PM UTC, comment #18:

hello,

I get the same problem trying to convert some files using the patch function from matlab to octave :

"warning: "interp" not supported, using 1st entry of cdata"

Could you explain how to fix it ?

thanks

arnaud <arnaudnaud>
Fri 14 Oct 2011 10:44:22 PM UTC, comment #17:

I found the problem (local to me). Now I get the same result as jwe.

I have a new example that works differently from ML.

Matlab and Octave return the same facevertexcdata ...

But gnuplot complains

The FLTK backend produces the correct result.

Ben Abbott <bpabbott>
Project MemberIn charge of this item.
Fri 14 Oct 2011 10:31:51 PM UTC, comment #16:

John,

Before, I start digging deeper, those results were produced from the code below, correct?

Ben Abbott <bpabbott>
Project MemberIn charge of this item.
Fri 14 Oct 2011 02:04:57 PM UTC, comment #15:

With the current version of Octave on my system, and with fltk+opengl graphics, I see

With gnuplot, I see

John W. Eaton <jwe>
Project Administrator
Thu 13 Oct 2011 11:56:06 PM UTC, comment #14:

For the two patches in comment #12, the facevertexcdata is empty, for ML.

Same for Octave.

I ran the test below.

Matlab returned,

Octave gives an error for me.

Ben Abbott <bpabbott>
Project MemberIn charge of this item.
Thu 13 Oct 2011 11:15:10 PM UTC, comment #13:

What is the orientation of facevertexcdata in Matlab?

Does my change get the order of the elements right? Maybe the transpose is not needed, just the (:)?

I was trying to fix a problem where x, y, and c are all matrices. There was some size mismatch if I did not force facevertexcdata to be a column vector. I thought that was how Matlab stores this property, but maybe I got the order of the elements wrong.

John W. Eaton <jwe>
Project Administrator
Thu 13 Oct 2011 11:08:24 PM UTC, comment #12:

Rik,

John just pushed a patch that looks to conflict with your original changeeset.

Yours ...

http://hg.savannah.gnu.org/hgweb/octave/rev/41c2f4633a62

John's ..

http://hg.savannah.gnu.org/hgweb/octave/rev/276bb0dd9d24

There have been other changes as well.

Presently, the code below does not produce the intended result.

Ben Abbott <bpabbott>
Project MemberIn charge of this item.
Mon 10 Oct 2011 03:16:01 AM UTC, comment #11:

Second patch works for me.

Rik <rik5>
Project Administrator
Mon 10 Oct 2011 12:28:29 AM UTC, comment #10:

I've attached my 2nd attempt.

Qualifier: Interpolating lines between vertices is not supported by the gnuplot bacekd (I didn't check the fltk backend).

(file #24123)

Ben Abbott <bpabbott>
Project MemberIn charge of this item.
Sun 09 Oct 2011 03:00:23 PM UTC, comment #9:

I got to this faster than I'd expected. Before pushing another reversion, does this look ok?

I'll do some more testting before I push.

(file #24122)

Ben Abbott <bpabbott>
Project MemberIn charge of this item.
Sun 09 Oct 2011 02:35:17 PM UTC, comment #8:

Thanks for catching this. I'll take a look later today.

Ben Abbott <bpabbott>
Project MemberIn charge of this item.
Sun 09 Oct 2011 03:01:59 AM UTC, comment #7:

Ben,
With your new patch in place (e9f6a6edec42) I went to try the list of test cases at http://www.mathworks.com/help/techdoc/ref/patch.html.

The first 'cdata' case no longer works. It appears that zdata is being interpreted as cdata.

Rik <rik5>
Project Administrator
Mon 03 Oct 2011 03:04:41 AM UTC, comment #6:

Yes. The stable branch is essentially frozen. Release Candidate 0 was put on the FTP site and I only know of one bug which may be incorporated into RC1. Even though bug fixes would normally go on to stable I am putting them on to default as 3.6 will be the next release.

Rik <rik5>
Project Administrator
Sun 02 Oct 2011 04:41:00 PM UTC, comment #5:

Rik,

I've prepared a changeset for the remaining problems I found. While I was finishing it, I noticed you pushed this change to default (rather than to stable).

Was that done on purpose?

Ben Abbott <bpabbott>
Project MemberIn charge of this item.
Fri 30 Sep 2011 04:00:53 PM UTC, comment #4:

Ben,

No. I needed to resolve the orientation of cdata issue so that I could close another bug. Matlab documentation has extensive references for what should happen in every situation ((http://www.mathworks.com/help/techdoc/ref/patch.html). I went through the examples last night and there are some failures with Octave when trying to do the most complex colorings such as having the edges one color and the facecolor something else or having gradients across the faces. Results were better when using the 'faces','vertices' style of patch creation rather than x,y data but Octave is still imperfect in this regard.

If you like, you can hijack this bug and re-title it to something more specific. Otherwise, you could close it and open up another bug report for your issue.

Rik <rik5>
Project Administrator
Fri 30 Sep 2011 11:24:55 AM UTC, comment #3:

Rik,

Do you plan to continue working on the patch() command?

Ben

Ben Abbott <bpabbott>
Project MemberIn charge of this item.
Fri 30 Sep 2011 05:08:03 AM UTC, comment #2:

I've gone ahead and applied the patch in this changeset (http://hg.savannah.gnu.org/hgweb/octave/rev/41c2f4633a62).

Rik <rik5>
Project Administrator
Fri 30 Sep 2011 01:03:52 AM UTC, comment #1:

I ran the plot demos, and did not see any regressions. However, I tried using scalar colors in the way I've done with ML, and discovered they fail with and without your patch.

Perhaps some new tests should be included when this is fixed?

Ben Abbott <bpabbott>
Project MemberIn charge of this item.
Wed 28 Sep 2011 04:24:16 PM UTC, original submission:

There are two ways of setting the color for patches. The first method is to create the patch with 'faces' and 'vertices' and use 'facevertexcdata'. This approach works. The second method is to create the patch with x and y vertices and use 'cdata' to set the color. This approach seems to fail. Extensive reference on how to set the color of patch faces is given here (http://www.mathworks.com/help/techdoc/ref/patch.html).

Sample code for method 1 (Working)

Sample code for method 2 (Not Working)

My hunch is that cdata and facevertexcdata are supposed to be transposes of each other. If one is a row vector the other needs to be a column vector.

The following change to _patch_.m at line 292 eliminates the error for me.

I have attached a changeset which does just that. If someone could verify that this doesn't mess up anything else I think it should be adopted.

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 #24123:  changeset.patch added by bpabbott (2kB - application/octet-stream - 2nd attempt)
file #24122:  changeset.patch added by bpabbott (947B - application/octet-stream)
file #24033:  patch.cdata added by rik5 (763B - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by arnaudnaud (Posted a comment)
  • -unavailable- added by jwe (Posted a comment)
  • -unavailable- added by bpabbott (Posted a comment)
  • -unavailable- added by rik5 (Submitted the item)
  • -unavailable- added by rik5
  •  

    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 7 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Thu 29 Aug 2013 11:49:12 PM UTCrik5StatusPatch Submitted=>Fixed
      Open/ClosedOpen=>Closed
    Mon 10 Oct 2011 12:28:29 AM UTCbpabbottAttached File-=>Added changeset.patch, #24123
      Assigned toNone=>bpabbott
    Sun 09 Oct 2011 03:00:23 PM UTCbpabbottAttached File-=>Added changeset.patch, #24122
    Wed 28 Sep 2011 04:24:16 PM UTCrik5Attached File-=>Added patch.cdata, #24033
      Carbon-Copy-=>Added kahacjde

    Back to the top


    Powered by Savane 3.1-cleanup