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.
|
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.
|
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.
|
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.
|
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.
|
Mon 20 Nov 2017 06:59:56 AM UTC, comment #5:
And empty 3d figure is shown
|
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 ?
|
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
|
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.
|
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.
|
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
|