bugGNU Octave - Bugs: bug #65421, patch fails to render if cdata is...

 
 

bug #65421: patch fails to render if cdata is specified before geometry in property/value calling form

Submitter:  Liang Tang <lt1234>
Submitted:  Wed 06 Mar 2024 01:02:26 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Unexpected Error or Warning
Status:  In Progress Assigned to:  None
Originator Name:  lt1234 Open/Closed:  * Open
Release:  * 8.2.0 Operating System:  * Any
Fixed Release:  10.1.0 (current default) Planned Release:  10.1.0 (current default)
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 18 Mar 2024 04:35:03 PM UTC, comment #41: 

I pushed the patch from comment #39 to the stable branch:
https://hg.savannah.gnu.org/hgweb/octave/rev/32002886fc7c

Markus Mützel <mmuetzel>
Group administrator
Fri 15 Mar 2024 05:32:07 PM UTC, comment #40: 

ignore my comment #37, apparently I had accidentally transposed it.

The 'geometry ahead of other properties' advice was mainly to avoid this input ordering bug with the intent to remove that advice once this bug was fixed. With the changes on default, i assumed it would be best to at least provide a warning/suggestion over on stable.  I did remove that advice on default after the earlier patch, but forgot to back the docstring change out when i backed out the patch. I just backed that out now so the text currently matches between stable and default.    If you feel the advice shouldn't be given, we can remove it.

yes, facecenterdcata that was a typo. 

 'cdata' it was in the original suggested list, my only change was to add vertices since faces was in there.  removing it is fine as it is an arbitrary suggestion list.

Nicholas Jankowski <nrjank>
Group Member
Fri 15 Mar 2024 05:00:10 PM UTC, comment #39: 

The attached patch changes the recently added "programming notes" for the `patch` function:

  • We probably shouldn't point users to the "cdata" property in a list that also mentions "faces", "vertices", and "facevertexcdata". "cdata" should be described in the section about the input "C" (or a reference to the "cdata" property could be added there).
  • I don't know where the advice to set all of the patch geometry properties before setting any other properties comes from. I suspect that was inspired by the last point in that list. Anyway, that point is confusing (and the truth that is to it is covered in a later item).
  • I assume that `facecentercdata` is a typo and it should say `facevertexcdata` instead.


Does that change look good to you?


(file #55840)

Markus Mützel <mmuetzel>
Group administrator
Fri 15 Mar 2024 04:29:53 PM UTC, comment #38: 

Sorry. I didn't get your point.

In the example, `faces` is a 4x3 matrix (i.e., 4 faces with 3 vertices each) and `cdata` is a 3x4 matrix (i.e., 3 color values for the vertices of the 4 faces). (If you check the dimensions the "xdata" and "ydata" properties of the resulting patch, they are also 3x4.)

Markus Mützel <mmuetzel>
Group administrator
Fri 15 Mar 2024 04:23:20 PM UTC, comment #37: 

(minor detail)
should cdata be set to the transpose of


cdata = vert_color(faces).';
cdata =

   5   3   1
   1   4   2
   1   3   4
   4   6   2


the shape later is 3x4 not 4x3

Nicholas Jankowski <nrjank>
Group Member
Fri 15 Mar 2024 04:12:21 PM UTC, comment #36: 

A reproducer with only `set` function calls:

vertices = [2.5, 7.5; 6.5, 7.0; 2.5, 5.5; 6.5, 6.0; 1.0, 6.5; 8.0, 6.5];

faces = [5, 3, 1; 1, 4, 2; 1, 3, 4; 4, 6, 2];

vert_color = (1:6).';  # index to colormap

cdata = vert_color(faces).';

hf = figure ();
clf (hf);
hax = axes (hf);
hp = patch (hax);

## step 1
set (hp, 'vertices', vertices);
v1 = get (hp, 'vertices')
f1 = get (hp, 'faces')
fvc1 = get (hp, 'facevertexcdata')
cd1 = get (hp, 'cdata')

## step 2
# set (hp, 'facevertexcdata', vert_color);
set (hp, 'cdata', cdata);
v2 = get (hp, 'vertices')
f2 = get (hp, 'faces')
fvc2 = get (hp, 'facevertexcdata')
cd2 = get (hp, 'cdata')


## step 3
set (hp, 'faces', faces);
v3 = get (hp, 'vertices')
f3 = get (hp, 'faces')
fvc3 = get (hp, 'facevertexcdata')
cd3 = get (hp, 'cdata')


When setting 'cdata' (which in general might require duplicating vertices) before setting 'faces', the color data is lost.
If you invert steps 2 and 3 or set 'facevertexcdata' instead of 'cdata', the color data isn't lost.

To be honest, it is quite unusual that a patch would be created with 'vertices' and 'faces' but the color data would be provided by 'cdata'.
More commonly, the color data comes from the "same sources" where the geometry comes from. So, it would be more usual to set a combination of either 'vertices', 'faces', and 'facevertexcdata', or 'xdata', 'ydata', ('zdata',) and 'cdata'.

The entire use-case looks pretty much constructed. So, fixing this has probably a low priority.

Markus Mützel <mmuetzel>
Group administrator
Fri 15 Mar 2024 02:46:26 PM UTC, comment #35: 

ok, I think i see what you mean now.

there are a number of other cases where you can use set() to change properties in a way that breaks plotting/rendering ability. but yes, it seems possible we'd hit the same problem by doing a multiple-property set(). sidestepping the bug at patch creation is likely insufficient.  i'll run a few tests and see if I can come up with such a combination of patch creation and set changes to trigger this same issue.  absent anything else, an xfail BIST set would be useful.

Assuming that's the case, then we could opt to keep a bandaid solution in _patch_ as a temporary / partial fix. as it's on default there's less need for that.

The question is which is more likely: we fall into waiting for a complete fix in graphics.cc, and it never happening due to priorities, or the partial/temp fix being implemented, and the full fix never happens because that even further lowers any priority. :)

Nicholas Jankowski <nrjank>
Group Member
Fri 15 Mar 2024 02:11:04 PM UTC, comment #34: 

Sorry if I haven't been clear enough before. Trying again:
It looks like the issue is caused by some update function not doing the right thing when properties are changed in a certain order.
If that is an issue when calling the `patch` function, it is likely also an issue when changing properties with the `set` function. Any change to the `patch` function will likely not fix the issue. It would only complicate the code to work around an issue that should be fixed elsewhere entirely.

Markus Mützel <mmuetzel>
Group administrator
Fri 15 Mar 2024 01:43:52 PM UTC, comment #33: 

The ordering issue is definitely a problem buried in graphics.cc. With some minimal testing on Matlab, property/value ordering seems to not matter at all except for repeated properties (I think it retains the first instance of a property, but haven't checked that in some time).

As `__patch__.m` was already doing quite a bit of input parsing and reformatting before sending the argument list to `__go_patch__`, the initial fix was definitely just an easy bandaid to try to sidestep the issue.

It would be fairly simple to turn the struct into name-value pairs, insert them at the same position into the argument list, then perform the same hack to avoid the bug. It would essentially avoid ever actually sending a struct to `__go_patch__`. I don't know of anything user-visible that would be impacted by that change, and it would sidestep the bug at the m-code level.

Nicholas Jankowski <nrjank>
Group Member
Fri 15 Mar 2024 07:15:21 AM UTC, comment #32: 

I'm not sure if `__patch__.m` is the right place to address the use-case reported here. A change would likely need to happen somewhere in `graphics.cc` or `graphics.in.h`.

Markus Mützel <mmuetzel>
Group administrator
Thu 14 Mar 2024 09:25:28 PM UTC, comment #31: 

ok, that does definitely cause an error when structs are included.  i backed out the changeset and will look at modifying the approach.  I can probably easily get it to avoid erroring when a struct is included.  getting it to avoid this bug with a struct included (i did verify fields in a struct trigger the same bug) may require extracting all of the fields from the struct in `__patch__.m` and passing them in as regular arguments.

Nicholas Jankowski <nrjank>
Group Member
Thu 14 Mar 2024 08:15:09 PM UTC, comment #30: 

i didn't realize that was an acceptable calling form.  will do some tests with/without the change and see if it's a concern.

Nicholas Jankowski <nrjank>
Group Member
Thu 14 Mar 2024 06:12:16 PM UTC, comment #29: 

IIUC, the change from here might fail if the additional input arguments of the "patch" function are a mix of property-value pairs and structures with graphics properties.

Markus Mützel <mmuetzel>
Group administrator
Tue 12 Mar 2024 07:08:50 PM UTC, comment #28: 

@liang:  there is no difference in this case for a command being process like:


patch ('xdata', x, 'ydata', y, 'cdata', c)

or


patch ('xdata', x, ...
       'ydata', y, ...
       'cdata', c);


this specific issue only deals with color data appearing before geometry data when using the 'property' 'value' calling form.

my goal was for the fix to make minimal changes, so it only slides the geometry items (xdata, ydata, zdata, faces, vertices) to the front without changing the relative order of those inputs or the ones that follow.  That satisfied this specific compatibility issue.

but yes, that's fine keeping this on default. Since this change relaxed the (unanticipated) ordering requirement I think the docs on default don't need any changing.  It may warrant adding a line to the help on stable.  i added a few property related notes to the end of the patch docstring that I believe adequately describes the current state of what will go out soon in octave 9 and pushed as https://hg.savannah.gnu.org/hgweb/octave/rev/773c6bd19630.

closing this report as fixed.

Nicholas Jankowski <nrjank>
Group Member
Tue 12 Mar 2024 05:47:53 PM UTC, comment #27: 

Ok. As long as the user is notified the expected sequence, it should be good.  Maybe the software documentation is also updated to help with this.   Thanks.

Liang Tang <lt1234>
Tue 12 Mar 2024 05:26:37 PM UTC, comment #26: 

Graphics properties in Octave are generally processed from left to right. And sometimes re-ordering leads to different results.
E.g., `set(h, "Units", "normalized", "Position", [0 0 1 1], "Units", "pixels")` leads to a different result than `set(h, "Units", "normalized", "Units", "pixels", "Position", [0 0 1 1])` obviously.

The interaction between the different properties of patch objects is sometimes not obvious from the start. I'm not sure if there are properties which must not be interchanged with the x/y/zdata, the vertices, or the faces properties. (There might be none and everything is fine. But I can't tell immediately.)

If you don't mind, it might be best to keep this on the default branch.

Markus Mützel <mmuetzel>
Group administrator
Tue 12 Mar 2024 05:23:46 PM UTC, comment #25: 

Thanks. 

Is this issue related to how a command is parsed, processing line-by-line vs. processing a complete command?

Liang Tang <lt1234>
Tue 12 Mar 2024 04:50:09 PM UTC, comment #24: 

this part (input ordering) of the recent work on patch appears to be complete and could be closed.  I had pushed it to default out of concerns of unanticipated outcomes from the change, but it's a relatively simple fix that could be pushed to stable if we think it's worthwhile. the effect would be to prevent odd errors if name/property input format doesn't have geometry first, since that's never stated anywhere in octave or matlab docs as a requirement.

Nicholas Jankowski <nrjank>
Group Member
Mon 11 Mar 2024 01:19:08 AM UTC, comment #23: 

yes, so the converse of that comment #15 is 'when you define your geometry with faces/vertices, FaceVertexCData is meant to be used."  if your data source is providing face/vertex data, that would probably be the easiest approach.

Nicholas Jankowski <nrjank>
Group Member
Sun 10 Mar 2024 11:55:28 AM UTC, comment #22: 

In comment #15.  "CData is meant to be used when you define your geometry with xdata,ydata,zdata".

So I had converted face/vertex to xyz for use with CData.  The outcome is the figure on the left.  You can see that many displaced face colors are not on the colorbar.  

And because comment #15, I place this info here.  I have no preference to keep this info here or move it away.

Liang Tang <lt1234>
Sun 10 Mar 2024 01:05:32 AM UTC, comment #21: 

Additionally, why go to the trouble of converting to xyzdata instead of just inputting your facevertexcdata shaped color info as facevertexcdata?

Nicholas Jankowski <nrjank>
Group Member
Sun 10 Mar 2024 12:55:26 AM UTC, comment #20: 

This doesn’t seem to be related to the input ordering. Should this have been posted on the other report?

Nicholas Jankowski <nrjank>
Group Member
Sat 09 Mar 2024 12:04:50 PM UTC, comment #19: 

I will wait until the crash issue resolved and try again.  As it is right now, I experience crashes with 8.2 and 8.4 without a consistent pattern. 

I converted the the original github geometry from face/vertex to xyz.  The plots are in the attached jpeg file.  The face/vertex is on the RHS.  Thanks.    

patch(...
'Xdata', xdata,...
'Ydata', ydata,...
'Zdata', zdata,... 
'FaceColor', 'interp',...
'EdgeColor', 'interp',...
'FaceAlpha', 0.5,...    % 'FaceAlpha', plt.face_alpha,...
'LineWidth', 2,...      % 'LineWidth', plt.edge_width,
'CData', plt.data  ...  % 'FaceVertexCData',(plt.data(:,1)) ... %  'CData', plt.data  ...      %this crashes  'FaceVertexCData', plt.data ...  % uint8(plt.data(:,1))   round(plt.data(:,1))
); axis equal;colorbar; % lighting none;


Liang Tang <lt1234>
Fri 08 Mar 2024 02:37:54 AM UTC, comment #18: 

comment #12:

> i think it is worth making the crashing issue a separate report, as it seems unrelated to this report's main issue regarding the cdata ordering. 


I submitted :

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

Crash-related discussion should continue there. 

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Fri 08 Mar 2024 02:30:57 AM UTC, comment #17: 

@lt1234, yes, please create another report that fully describes the crashing issue.

default built fine and didn't cause any test failures, although it's worth noting that we have very few plot tests to catch issues.  pushed that patch to default as:
https://hg.savannah.gnu.org/hgweb/octave/rev/4ec97d8770ff

So absent any other changes in Octave 10 patch will render even if the user puts CData before geometric properties. Marking ready for test.


Also pushed a corrected doc description of what the cdata and facevertexcdata shapes are -supposed- to do, at least according to matlab compatibility and generally in agreement with what was seen below.  it now reads:


cdata: scalar | matrix | array, def. [](0x0)
Data defining the patch object color relative to its x/y/z-coordinate data. Patch color can be defined using indices into the current colormap or as RGB triplets, where the RGB colors are defined along the third dimension. These colors can be separately defined for the entire patch object, for individual faces, or for individual vertices, and is determined by the shape of "cdata" as follows:

If "cdata" is a scalar index into the current colormap or a 1-by-1-by-3 RGB triplet, it defines the color of all faces and edges.

If the patch object has N faces, and "cdata" is a 1-by-N vector of colormap indices or a 1-by-N-by-3 RGB array, it defines the color of each face.

If the patch object has N faces and M vertices per face, and cdata is a M-by-N matrix of colormap indices or a M-by-N-by-3 RGB array, it defines the color at each vertex. (The shape of "cdata" should match that of "xdata", "ydata", and "zdata".)

...

facevertexcdata: scalar | matrix, def. [](0x0)
Data defining the patch object color relative to its face-vertex data. Patch color can be defined using indices into the current colormap or as RGB triplets, where the RGB colors are defined in the rows of "facevertexcdata". These colors can be separately defined for the entire patch object, for individual faces, or for individual vertices, and is determined by the shape of "facevertexcdata" as follows:

If facevertexcdata is a scalar index into the current colormap or a 1-by-3 RGB triplet, it defines the color of all faces and edges.

If the patch object has N faces, and facevertexcdata is a N-by-1 column vector of indices or a N-by-3 RGB matrix, it defines the color of each one of the N faces.

If the patch object has M vertices, and facevertexcdata is a M-by-1 column vector of indices or a M-by-3 RGB matrix, it defines the color at each vertex.


pushed to stable as https://hg.savannah.gnu.org/hgweb/octave/rev/0e2fbef78702

Nicholas Jankowski <nrjank>
Group Member
Thu 07 Mar 2024 11:06:36 PM UTC, comment #16: 

Nik,

Originally, I think Octave supports different color specifications than matlab.  So I accepted that the two codes may differ on this specific subject. 

I went through your list.  I think the list is consistent with
https://www.mathworks.com/help/matlab/ref/matlab.graphics.primitive.patch-properties.html

One observation is that mathworks has the following on FaceVertexCData :

Face and vertex colors, specified as a single color for the entire patch, one color per face, or one color per vertex for interpolated face color.

Your list appears to reject one color per face for FaceVertexCData.  For many applications, the physical quantities, like displacement, velocity, etc are readily available at the vertices.  To get the face values, additional domain integration is required.  I am sure that I will not spend the extra.  But I have no saying what others would do. 

I think the crash issue should move to a different bug report.

Liang Tang <lt1234>
Thu 07 Mar 2024 09:33:53 PM UTC, comment #15: 

after looking at the amount of input checking in _patch_.m I figured a little bit more would at least put a bandaid on the main report issue.  attached is a patch that slides any geometry related properties to the front of the argument list before jumping into the compiled code.  also added a test to patch.m that checks for the same output with cdata either before or after faces.  seems to pass all simple tests with patch.  i haven't rebuild default in a while, so rebuilding and running tests on there before pushing the patch. 

note that in general, CData is meant to be used when you define your geometry with xdata,ydata,zdata,  and FaceVertexCData is meant to be used when you define your geometry with Faces / Vertices.  looking at how it changes some of the 'other' properties when you mix them, it's not a great surprise that there can be odd results.  could be that's causing the crashes you posted about.

(file #55798)

Nicholas Jankowski <nrjank>
Group Member
Thu 07 Mar 2024 09:13:34 PM UTC, comment #14: 

After demo_colorbar is completed, I can rotate w/o any issue.  I had rotate issue only with the no crash ofig in the memory. For a large size geometry displayed using patch, rotate might rotate whichever direction it would rotate, but not in the direction that I would like the plot to. I have not tried if interruptible=off setting would help.   

The only other time I had issue with rotate was logged earlier, i.e., when campos is off, rotate seems to be good only for Rx, Ry and Rz.  This limitation also appeared to be a matlab bug in 2017-2020 time frame.  But, not any more.

Liang Tang <lt1234>
Thu 07 Mar 2024 09:06:42 PM UTC, comment #13: 

and i didn't see it below so please when you create the separate report, include the actual commands you used to created the problematic figure.

Nicholas Jankowski <nrjank>
Group Member
Thu 07 Mar 2024 08:59:34 PM UTC, comment #12: 

i think it is worth making the crashing issue a separate report, as it seems unrelated to this report's main issue regarding the cdata ordering.

Nicholas Jankowski <nrjank>
Group Member
Thu 07 Mar 2024 08:35:22 PM UTC, comment #11: 

Works as expected on my computer (I've also tried 8.4 from flatpak).

If you do "demo colorbar" and try rotating there -- does it work as expected?

Dmitri.
--


Dmitri A. Sergatskov <dasergatskov>
Thu 07 Mar 2024 07:45:34 PM UTC, comment #10: 

When I loaded the no_crash in 8.4, the rotate automatically applied to the colorbar only.

Liang Tang <lt1234>
Thu 07 Mar 2024 07:26:15 PM UTC, comment #9: 

With no sphere_no_crash I see attached sphere_no_crash.png
I can zoom, rotate and autoscale w/o problem.
(again this is on linux (hgid b4f8f14dc1fb (stable))

Dmitri.
--



Dmitri A. Sergatskov <dasergatskov>
Thu 07 Mar 2024 07:18:24 PM UTC, comment #8: 

And now it really attached.
May  be crash should  be a separate bug.

Dmitri.
--



(file #55796)

Dmitri A. Sergatskov <dasergatskov>
Thu 07 Mar 2024 07:06:52 PM UTC, comment #7: 

I verify it on linux with 9.0.91 (hgid b4f8f14dc1fb (stable))
using nvidia proprietary driver.


octave:1> hgload sphere_crash.ofig
Missing separate debuginfos, use: ...
...
Thread 9 "QThread" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7ffed6204640 (LWP 3191251)]
0x00007ffff74a8e13 in octave::patch::properties::update_data (this=this@entry=0x7ffebc84bab0) at ../libinterp/corefcn/graphics.cc:9741
9741                      cd(ii, jj, kk) = fvc(row, kk);
(gdb) thread apply all bt


Full trace is attached.

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Thu 07 Mar 2024 06:16:55 PM UTC, comment #6: 

I just need a short time to find out if the crash is data specific or patch related.  It appears to be, somehow, data size related.  Two ofig files attached.  The geometric data is a modified version of a public domain mesh.  https://github.com/otvam/fem_mesh_matlab. A plot with one sphere works great.   I duplicate the last sphere in the link arbitrarily to produce the crash.  Both ofigs are generated with 8.2.0 win10 64x.  No crash is from 'FaceVertexCData', no cdata.  Other than the patch specification, I am not aware of any differences between the two. 

I have two Octave versions, 8.2 and 8.4 win10 64x. 

(1) no_crash means no crash.  It does not mean figure gui plot toolbox is acting normally. 

(2) Crash means the ofig causes Octave gui 8.2.0 with software rendering and 8.4.0 hardware rendering to crash.  For 8.2, load the file and then click any plot toolbox.  For 8.4, it did not complete load-and-display. 

(3) Originally, after the plot is generated, 8.2.0 gui would crash when the plot toolbox is being used.  It took just few rotations, for me, to cause the issue. For a larger data, even a mouse click on any point on the figure would cause the issue.

(file #55795)

Liang Tang <lt1234>
Thu 07 Mar 2024 03:08:26 PM UTC, comment #5: 

I will be slower in responding.  After move CData to the end of patch specification, I started to experience a complete Octave session crash issue after the plot is generated.

Liang Tang <lt1234>
Thu 07 Mar 2024 03:01:21 PM UTC, comment #4: 

all of the input handling for patch (anything more complex than patch(x,y,z,c)) is done inside graphics.cc, and a real fix for the ordering issue will need to be addressed there. We could add some top level hack to check and reorder the varagin inputs to make sure faces and vertices appear first if they appear, but it would probably be best to handle than internally, where the rest of the internal functions take place. 

regarding the different cdata/facevertexcdata forms below, i'm going to suggest those details be moved over to bug #63523 which can be slightly broadened to address both facevertexcdata and cdata input validation.

Nicholas Jankowski <nrjank>
Group Member
Wed 06 Mar 2024 10:39:46 PM UTC, comment #3: 

regarding the different forms of D (it would help if you could indicate where you got the forms from that you tried, as some you said you can't tell what they should do): whether it's supplied to CData, or FaceVertexCData the interpretation is slightly different. In particular, color ordering  for vertices might be different because CData supposedly goes off the x/y/zdata properties while FaceVertexCData uses the supplied Vertex property data. Also, there seems to be insufficient input form checking on those properties, and some strange results occur:

Using Colormap indices:

ONE COLOR:
D = 20
both CData and FaceVertexCData - color all faces the 20th color in the current colormap


ONE COLOR PER FACE:
D = [10 20 30 40]'  # numel(D) = N = number of faces
Both CData and FaceVertexCData:  color Nth face the Nth index # specified in the vector

D = [10 20 30 40]  # numel(D) = N = number of faces
CData:  color Nth face the Nth index # specified in the vector
FaceVertexCData:  error - invalid value for property


ONE COLOR PER VERTEX:
D = [10 20 30 40 50 60]'  # numel(D) = M = number of Vertices
Both CData and FaceVertexCData:  color Nth face the Nth index # specified in the vector

D = [10 20 30 40 50 60]  # numel(D) = M = number of vertices
CData:  color Nth face the Nth index # specified in the vector
FaceVertexCData:  error - invalid value for property


ONE COLOR PER VERTEX, DIFFERENT ON EACH FACE:
D=round(64*rand(size(P,1), size(T,1))) # N x M matrix
CData: specifies one color per vertex per face. if you do facecolor = interp, you'll see that faces sharing vertices can have diff colors on either side of the shared edge.
FaceVertexCData: error. invalid value for array property

D=round(64*rand(size(T,1), size(P,1))) # M x N matrix
CData: specifies one color per vertex per face like above. different color/vertex ordering. Not sure how Octave maps it since the matlab docs say should be NxM.
FaceVertexCData: error. invalid value for array property


Using RGB color specs:

ONE COLOR:
D = [0.1 0.5 0.7]
CData: odd color results. supposedly expects dim 3 vector. facevertexcdata stored as column vector.
FaceVertexCData:  single rgb value applied to all faces (checking CData, it contains the same values changed to a dim3 vector)

D = [0.1 0.5 0.7]'
CData: odd color results. supposedly expects dim 3 vector. facevertexcdata stored as column vector.
FaceVertexCdata: odd color results. Cdata contains a seemingly random 3x4 numeric array.

D = cat(3,0.1 ,0.5, 0.7)
CData: single rgb value applied to all faces. (facevertexdata, same values stored as a row vector)
FaceVertexCData: error. invalid value for array property.


ONE COLOR PER FACE:
D = [0 0.1 0.2; 0.3 0.4 0.5; 0.6 0.7 0.8; 0.9 1.0 0.0] #(Nx3 matrix)
CData: odd color results. Cdata stored as 4x3, FaceVertexCData stored as D(:) (12x1)
FaceVertexCData: One color per face. Cdata stored as 1x4x3 array.

D = D'  #(3xN matrix)
CData: odd color results. Cdata stored as 3x4, FaceVertexCData stored as D(:) (12x1)
FaceVertexCData: Error - invalid value for property.

D = permute([0 0.1 0.2; 0.3 0.4 0.5; 0.6 0.7 0.8; 0.9 1.0 0.0], [1 3 2])  # Nx1x3 array
CData: one color per face.  FaceVertexCData stored as 4x3.
FaceVertexCData:  Error - invalid value for property.


ONE COLOR PER VERTEX:
D = [0 0.1 0.2; 0.3 0.4 0.5; 0.6 0.7 0.8; 0.9 1.0 0.0; 0.1 0.2 0.3; 0.4 0.5 0.6] # M x 3 matrix
CData:  odd color values. Cdata is 6x3. FaceVertexCData is 18x1.
FaceVertexCData: one color per vertex. CData stored as 3x4x3

D = D'  #(3xM matrix)
CData: odd color results. Cdata stored as 3x6, FaceVertexCData stored as D(:) (18x1)
FaceVertexCData: Error - invalid value for property.


D = rand(3x4x3) # dim1&2 size to match x/y/zdata, RGB in dim3
CData: one color per vertex per face. (color can jump across edges). FaceVertexCData stored as 12x3.
FaceVertexCData: Error - invalid value for property.


I think those are all of the likely options to be tried for D.
some of the behavior needs to be checked for compatibility, then maybe some more refined bug reports generated for input validation, especially with cdata. Also, see bug #63523 where some input validation checks were noticed to be lacking.

Nicholas Jankowski <nrjank>
Group Member
Wed 06 Mar 2024 09:55:51 PM UTC, comment #2: 

I did a quick check with the patch format below.  All variances of D in the original report render. 

I have some added info.

(1) In the original report, I had FaceVertexCData worked for some D.  The geom data is ahead of Face.  FaceVertexCData simply replaces Cdata.  So the order of the patch specification is affecting CData more. 

(2) According 8.2, some of the D are out of scope for Octave. I did those as if I followed (at least I tried to) Mathworks' documentation, which is quite loose and difficult for me. Now all of D render in Octave. So the 8.2 and the current documentation do not cover all the variances that the software supports. 

(3) I don't understand the color scheme specified by any N-by-M. For example, a patch of 500 vertices and 800 faces  with RGB would have a 800x500x3 as its color spec.  But each vertex does not connect to 800 faces and each face does not have 500 vertices.     



patch(...
'Vertices', P, ...   
'Faces', T,...  
'FaceColor', 'interp',...
'EdgeColor', 'interp',...
'FaceAlpha', 0.5,...
'LineWidth', 2, ...
'CData', D);

Liang Tang <lt1234>
Wed 06 Mar 2024 05:29:52 PM UTC, comment #1: 

we recently push an update to document all of the graphics property items.  see bug #65167.  but I didn't go through and verify all properties as functional. if you can grab a recent version of the stable build of octave 9.0.91 you should be able to preview the current documentation of what things are supposed to do.  the current version of patch cdata and facevertexdata are:


cdata: scalar | matrix | array, def. [](0x0)
Data defining patch object color relative to it’s x/y/z-coordinate data. Patch color can be defined for faces or for vertices.
If cdata is a scalar index into the current colormap or a RGB triplet, it defines the color of all faces.
If cdata is an N-by-1 vector of indices or an N-by-3 (RGB) matrix, it defines the color of each one of the N faces.
If cdata is an N-by-M matrix or an N-by-M-by-3 (RGB) array, it defines the color at each vertex.

facevertexcdata: scalar | matrix, def. [](0x0)
Data defining the patch object color relative to the patch’s face-vertex data. Patch color can be defined for faces or for vertices.
If facevertexcdata is a scalar index into the current colormap or a RGB triplet, it defines the color of all faces.
If facevertexcdata is an N-by-1 vector of indices or an N-by-3 (RGB) matrix, it defines the color of each one of the N faces.
If facevertexcdata is an M-by-1 vector of indices or an M-by-3 (RGB) matrix, it defines the color at each one of the M vertices.


it appears that your calling forms would work correctly, except you put your CData property before the Faces property.  checking with Matlab 2023, this shouldn't be a requirement, but apparently it is in octave.  if you put vertices and Faces first, i think all of you options for D render properly.

retitling report to reflect the input restriction

Nicholas Jankowski <nrjank>
Group Member
Wed 06 Mar 2024 01:02:26 PM UTC, original submission:  


patch/cdata does not work.  It leads to an warning below, which should have been an error. Thanks.

warning: opengl_renderer: some vertices in "faces" property are undefined.  Not rendering.

facevertexcdata is  undocumented. But it works except RGB input.


P = [ 2.5    7.5
      6.5    7.0
      2.5    5.5
      6.5    6.0
      1.0    6.5
      8.0    6.5]
     
T = [5  3  1;
     1  4  2;
     1  3  4;
     4  6  2]; 

D=20;

D=round( 64*rand(size(P,1), 1));            % index to colormap
D=round(255*rand(size(P,1), 3));            % RGB

D=round(125*rand(size(P,1), size(T,1)));    % don't make sense what this is for 
D=round(125*rand(size(T,1), size(P,1)));    % Not an option in Octave documentation; don't make sense what this is for 

D=round( 64*rand(size(T,1), 1));            % Not an option in Octave documentation
D=round(255*rand(size(T,1), 3));            % Not an option in Octave documentation

% none would work
    
patch(...
'Vertices', P, ...   
'CData', D,    % 'FaceVertexCData', D,      % Octave facevertexcdata is  undocumented.
'Faces', T,...  
'FaceColor', 'interp',...
'EdgeColor', 'interp',...
'FaceAlpha', 0.5,...
'LineWidth', 2);



Documentation: 

cdata: scalar | matrix, def. [](0x0)
Data defining the patch object color. Patch color can be defined for faces or for vertices.
If cdata is a scalar index into the current colormap or a RGB triplet, it defines the color of all faces.    
If cdata is an N-by-1 vector of indices or an N-by-3 (RGB) matrix, it defines the color of each one of the N faces. 
If cdata is an N-by-M or an N-by-M-by-3 (RGB) matrix, it defines the color at each vertex.  

facevertexcdata is  undocumented.

N is size of Vertices matrix. (P here)
colormap is 64x3.
RGB is 0-255.

Liang Tang <lt1234>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #55798:  bug65421_patchorderfix.diff added by nrjank (2KiB - application/octet-stream - patch for cdata ordering, pushes all geometry properties to front of input stack)
file #55796:  sphere_crash_gdb.txt.gz added by dasergatskov (5KiB - application/gzip)
file #55795:  sphere.zip added by lt1234 (349KiB - application/x-zip-compressed)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by dasergatskov (Posted a comment)
  • -email is unavailable- added by nrjank (Posted a comment)
  • -email is unavailable- added by lt1234 (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 17 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2024-03-15 mmuetzel Attached File- Added bug65421-patch-doc-programming-notes.patch, #55840
    2024-03-14 nrjank StatusFixed In Progress
        Open/ClosedClosed Open
    2024-03-12 nrjank Fixed ReleaseNone 10.1.0 (current default)
    2024-03-12 nrjank StatusReady For Test Fixed
        Open/ClosedOpen Closed
        Summarypatch fails to render if cdata is specified before geometry patch fails to render if cdata is specified before geometry in property/value calling form
    2024-03-09 lt1234 Attached File- Added one_sphere_different_color_face_vertex_vs_xyz_data.JPG, #55813
    2024-03-08 nrjank StatusConfirmed Ready For Test
        Operating SystemMicrosoft Windows Any
        Planned ReleaseNone 10.1.0 (current default)
    2024-03-07 nrjank Attached File- Added bug65421_patchorderfix.diff, #55798
    2024-03-07 dasergatskov Attached File- Added sphere_no_crash.png, #55797
    2024-03-07 dasergatskov Attached File- Added sphere_crash_gdb.txt.gz, #55796
    2024-03-07 lt1234 Attached File- Added sphere.zip, #55795
    2024-03-06 nrjank StatusNone Confirmed
    2024-03-06 nrjank Summarypatch/cdata does not work. patch fails to render if cdata is specified before geometry

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code