bugGNU Octave - Bugs: bug #48109, Trisurf fails to colour triangles...

 
 

bug #48109: Trisurf fails to colour triangles correctly.

Submitted by:  None
Submitted on:  Fri 03 Jun 2016 09:03:02 AM UTC  
 
Category: PlottingSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Matlab Compatibility
Status: FixedAssigned to: None
Originator Name: Tim DOriginator Email: -unavailable-
Open/Closed: ClosedRelease: 4.2.0
Operating System: Any

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)

Mon 27 Nov 2017 05:33:58 AM UTC, comment #10:

Turns out the same input validation was being used for trimesh as well. I used your template from trisurf to also update trimesh and checked in both changes under your name here http://hg.savannah.gnu.org/hgweb/octave/rev/678912855789.

Since you are now officially a contributor to Octave, would you like to have your name appear at the beginning of the manual? It's up to you. Some people like it, others prefer to remain more anonymous.

Marking bug as fixed and closing report.

Rik <rik5>
Project Administrator
Mon 20 Nov 2017 09:25:24 PM UTC, comment #9:

Many thanks Rik for all the precisions, now it's perfectly clear, and I have no more excuses for being useless. I'll look closely to the guideline as soon as possible.

Hugo Raguet <snake>
Mon 20 Nov 2017 07:31:06 PM UTC, comment #8:

>> 1) the function is creating triangles and coloring them. The most straightforward form should then provide one color per triangle in arguments. What should be the expected behavior when giving one color per vertex ? I guess coloring each triangle with the color of the first vertex defining it makes sense (and seems to be the implemented behavior), but this should appear in the documentation.


Matlab, and hence Octave, implements trisurf through the patch function. The coloring of patches is very complicated. See https://www.mathworks.com/help/matlab/creating_plots/how-patch-data-relates-to-a-colormap.html. From this documentation you can see that either one color per face, or one color per vertex are acceptable. The test results from Matlab show that it is using one color per vertex though, so Octave should do the same. In particular, the default "FaceColor" property is set "flat" which means the color of face is determined by the first vertex. However, if the "FaceColor" property is set to "interp" then the color varies smoothly across the face of the polygon as it moves from the color at vertex1 to the color at vertex2 and vertex3. In this case, it is quite important to be able to specify the colors of all vertices, not just one color per face.

>> 2) I agree that "extending the input validation to accept either scenario is not hard", but it has currently not been done (Octave 4.2.1 in Linux) and this is what caused this thread in the first place.


It hasn't been done, because no one confirmed the correct behavior with Matlab until 1 day ago.

>> Is any developer planning to do it? I'd be glad to do it myself but I do not know yet where I should start to contribute, and if it is even worth for such small changes.


It is always very helpful, and very useful, if an initial patch is supplied for a bug report. Polishing of a patch by developers is easier than writing a patch from scratch. And it is very likely that a 4.2.2 bug fix release will be put out at the end/start of the year so there is still time to get any fix posted here into that release.

For some general guidelines see http://wiki.octave.org/Contribution_guidelines.

Rik <rik5>
Project Administrator
Mon 20 Nov 2017 05:08:43 PM UTC, comment #7:

I am a bit confused.

1) the function is creating triangles and coloring them. The most straightforward form should then provide one color per triangle in arguments. What should be the expected behavior when giving one color per vertex ? I guess coloring each triangle with the color of the first vertex defining it makes sense (and seems to be the implemented behavior), but this should appear in the documentation.

2) I agree that "extending the input validation to accept either scenario is not hard", but it has currently not been done (Octave 4.2.1 in Linux) and this is what caused this thread in the first place.

Is any developer planning to do it? I'd be glad to do it myself but I do not know yet where I should start to contribute, and if it is even worth for such small changes.

Hugo Raguet <snake>
Mon 20 Nov 2017 03:47:59 PM UTC, comment #6:

Thanks for testing. The error message is nice and clear that trisurf can either have one color per face or one color per vertex (as currently coded in Octave), but no other combinations. Extending the input validation to accept either scenario is not hard.

Rik <rik5>
Project Administrator
Mon 20 Nov 2017 06:59:56 AM UTC, comment #5:

And empty 3d figure is shown

Anonymous
Sun 19 Nov 2017 07:42:59 PM UTC, comment #4:

Hi Rik

Sorry I do not have access to Matlab right now so I cannot directly help you, but don't you agree with me that the only problem comes from the fact the Octave source code for trisurf currently expects one color per vertex, while it should rather expect one color per triangle ?

Hugo Raguet <snake>
Sun 19 Nov 2017 02:55:46 AM UTC, comment #3:

Can someone with access to Matlab run the test code from comment #1? I'd like to see the error message.

Also, as an addendum, try

Rik <rik5>
Project Administrator
Tue 31 Oct 2017 04:29:14 PM UTC, comment #2:

Hello

similar problem here, Octave 4.2.1 on Linux.
Obviously, `trisurf` is supposed to specify one color per triangles. However, the source code expects one color per vertex, which is wrong:
However; when calling directly the primitive `patch` function (which is eventually called in trisurf.m, but with wrong arguments), all is well.
This might explain other `trisurf`related bugs, and seems to be easily fixed.

Hugo Raguet <snake>
Wed 23 Nov 2016 01:31:54 AM UTC, comment #1:

What is the size of SSDisp?

Also, if you have access to Matlab, does the following work?

Maybe there is no input validation on the color input C, but that seems odd.

Rik <rik5>
Project Administrator
Fri 03 Jun 2016 09:03:02 AM UTC, original submission:

Hi all,

I am running the trisurf function.

I am calling it like so:

trisurf(Triangles,Points(:,2),Points(:,3),Points(:,4),SSDisp);

'Triangles' is a 3xn vector of the triangluation
This points to rows in the 'Points' which is a vector where each row defines the corner points of each triangle.
'SSDisp' is a value at the centre of each triangle I have calculated.
If I try and run this in the current octave version (4.0) this gives an error 'error: trisurf: C must have 'numel (Z)' elements'

This is not in the octave documentation or explicitly in the MATLAB online docs either but MATLAB actually allows users to colour triangles in trisurf with a colour matrix that is the size of the number of triangles, This means the user does not need the colour vector to be the same size as the 'z' values. Picture attached

Currently the Trisurf code looks like this:

It will colour the triangles without an error if changed to:

I.e this is not reshaping c (the colour vector) but just saying its a column vector.

The function works fine for me if I change this to the code I have added above. I have attached a picture of this working.

(Ps I am English so spell colour differently to Americans)

Cheers,

Tim

Anonymous

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #42484:  trisurf.patch added by snake (3KiB - text/x-patch)
file #37369:  SSDisp.PNG added by None (95KiB - image/png - Coloured correctly with the new code)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by snake (Posted a comment)
  • -unavailable- added by rik5 (Posted a comment)
  • -unavailable- added by None (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 8 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon 27 Nov 2017 05:33:58 AM UTCrik5StatusIn Progress=>Fixed
      Open/ClosedOpen=>Closed
    Sat 25 Nov 2017 07:56:45 PM UTCsnakeAttached File-=>Added trisurf.patch, #42484
    Mon 20 Nov 2017 03:47:59 PM UTCrik5StatusNeed Info=>In Progress
      Operating SystemMicrosoft Windows=>Any
    Wed 23 Nov 2016 01:31:54 AM UTCrik5StatusNone=>Need Info
      Release4.0.0=>4.2.0
    Fri 03 Jun 2016 09:03:02 AM UTCNoneAttached File-=>Added SSDisp.PNG, #37369

    Back to the top


    Powered by Savane 3.1-cleanup1