bugGNU Octave - Bugs: bug #55436, Add support for more types for...

 
 

bug #55436: Add support for more types for image's cdata

Submitter:  Guillaume <gyom>
Submitted:  Thu 10 Jan 2019 11:05:43 AM UTC
   
 
Category:  Plotting with OpenGL Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Feature Request
Status:  Fixed Assigned to:  None
Originator Name:  Guillaume Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Tue 05 Feb 2019 06:20:52 PM UTC, comment #4: 

Seems to work.  Marking as fixed and closing report.

Rik <rik5>
Group administrator
Sun 27 Jan 2019 06:08:04 PM UTC, comment #3: 

I pushed a changeset that adds support for more types for "cdata" here:
http://hg.savannah.gnu.org/hgweb/octave/rev/edb11e68b571

Marking as ready for test.

Markus Mützel <mmuetzel>
Group administrator
Sat 12 Jan 2019 12:15:14 PM UTC, comment #2: 

Following the code, it looks like the cdata is implicitly cast to double when calling "convert_cdata_2". I don't think this is possible without loosing precision at least for int64 and uint64.
Since we pass the colors as float values to glColor that might not be a problem after all.

You might also want to add the same full list of type constraints to the patch and surface properties as well.

Can you create a full fledged patch out of your changes?

Markus Mützel <mmuetzel>
Group administrator
Thu 10 Jan 2019 11:20:45 AM UTC, comment #1: 

FWIW, Matlab seems to support all these types:
Data Types: single | double | int8 | int16 | int32 | int64 | uint8 | uint16 | uint32 | uint64 | logical

https://de.mathworks.com/help/matlab/ref/matlab.graphics.primitive.image-properties.html#budqsu__sep_shared-CData

Markus Mützel <mmuetzel>
Group administrator
Thu 10 Jan 2019 11:05:43 AM UTC, original submission:  

Currently, only a few data types are supported with image() and imagesc():


octave> figure,imagesc(int16(rand(32)))
warning: unsupported type for cdata (= int16 matrix).  Valid types are uint8, uint16, double, single, and bool.


Following comment #3 in bug #46933, I made the following change in libinterp/corefcn/graphics.cc:


--- a/libinterp/corefcn/graphics.cc     Wed Jan 09 19:10:38 2019 -0800
+++ b/libinterp/corefcn/graphics.cc     Thu Jan 10 10:53:01 2019 +0000
@@ -1015,8 +1015,20 @@

   if (cdata.is_uint8_type ())
     CONVERT_CDATA_1 (uint8NDArray, uint8_, false);
+  if (cdata.is_int8_type ())
+    CONVERT_CDATA_1 (int8NDArray, int8_, false);
   else if (cdata.is_uint16_type ())
     CONVERT_CDATA_1 (uint16NDArray, uint16_, false);
+  else if (cdata.is_int16_type ())
+    CONVERT_CDATA_1 (int16NDArray, int16_, false);
+  else if (cdata.is_uint32_type ())
+    CONVERT_CDATA_1 (uint32NDArray, uint32_, false);
+  else if (cdata.is_int32_type ())
+    CONVERT_CDATA_1 (int32NDArray, int32_, false);
+  else if (cdata.is_uint64_type ())
+    CONVERT_CDATA_1 (uint64NDArray, uint64_, false);
+  else if (cdata.is_int64_type ())
+    CONVERT_CDATA_1 (int64NDArray, int64_, false);
   else if (cdata.is_double_type ())
     CONVERT_CDATA_1 (NDArray, , true);
   else if (cdata.is_single_type ())
@@ -1027,7 +1039,7 @@
     {
       // Don't throw an error; leads to an incomplete FLTK object (bug #46933).
       warning ("unsupported type for cdata (= %s).  "
-               "Valid types are uint8, uint16, double, single, and bool.",
+               "Valid types are uint8, int8, uint16, int16, uint32, int32, uint64, int64, double, single, and bool.",
                cdata.type_name ().c_str ());
       a = NDArray (dv, 0);  // return 0 instead
     }


and while it then works with, eg, int16, there is still an error for other types:


octave> figure,imagesc(int32(rand(32)))
error: invalid value for array property "cdata"
error: __go_image__: unable to create graphics handle
error: called from
    image>__img__ at line 205 column 5
    image at line 125 column 10
    imagesc at line 101 column 12


so I also modified libinterp/corefcn/graphics.in.h:


--- a/libinterp/corefcn/graphics.in.h   Wed Jan 09 19:10:38 2019 -0800
+++ b/libinterp/corefcn/graphics.in.h   Thu Jan 10 11:03:57 2019 +0000
@@ -4556,8 +4556,13 @@
       cdata.add_constraint ("single");
       cdata.add_constraint ("logical");
       cdata.add_constraint ("uint8");
+      cdata.add_constraint ("int8");
       cdata.add_constraint ("uint16");
       cdata.add_constraint ("int16");
+      cdata.add_constraint ("uint32");
+      cdata.add_constraint ("int32");
+      cdata.add_constraint ("uint64");
+      cdata.add_constraint ("int64");
       cdata.add_constraint ("real");
       cdata.add_constraint (dim_vector (-1, -1));
       cdata.add_constraint (dim_vector (-1, -1, 3));


It does seem to work for me but I wonder if it doesn't create problems elsewhere as it was not implemented in the first place.

Guillaume <gyom>

 

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

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by gyom (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 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-02-05 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2019-01-27 mmuetzel StatusNone Ready For Test

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code