bugGNU Octave - Bugs: bug #49130, uint8 images are not shown...

 
 

bug #49130: uint8 images are not shown correctly by imshow with gnuplot

Submitter:  None
Submitted:  Mon 19 Sep 2016 05:53:48 PM UTC
   
 
Category:  Plotting with gnuplot Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  R.S.Carmenes Originator Email:  -email is unavailable-
Open/Closed:  * Closed Release:  * 4.2.0-rc1
Operating System:  * GNU/Linux Fixed Release:  None
Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 02 Oct 2016 01:31:59 PM UTC, comment #20: 

Okay, it must be just my machine.  I cast clim to double as part of a different cset so I don't think anything else remains on this report.

Rik <rik5>
Group administrator
Sat 01 Oct 2016 04:19:36 PM UTC, comment #19: 

If my comments can be of any help (as the originator of this bug report), your patch solve the observed problems for me too, and the example that Rik said it hang his octave (cww comment #16), worked well for me (I am using gnuplot 5.0.4).

Thanks for all your work!

Anonymous
Fri 30 Sep 2016 05:47:18 PM UTC, comment #18: 

I agree for consistency we should cast clim to double in the True Color branch of the if statement.

I'm using gnuplot 5.0.1 which I think does have a few problems.  If the examples from comment #2 work for you now then I'll just ignore my machine.

Rik <rik5>
Group administrator
Fri 30 Sep 2016 05:36:34 PM UTC, comment #17: 

I'm not seeing anything hang here.  What version of gnuplot are you using?

There are some other recent changes that might have influence:

  • scripts/general/inputParser.m(parse) : reset Unmatched and UsingDefaults befor

  a new parse.

etc.

"double->uint8->double to achieve clamping of RGB values.  Keep cdata as double"

Just noting that in one case there is:


    cdata = double (cdata);
    cdata = 255 * (cdata - clim(1)) / (clim(2)-clim(1));


and in another:


      cdata = double (cdata);
      clim = double (clim);
      cdata = 1 + fix (cmap_sz * (cdata - clim(1)) / (clim(2) - clim(1)));


for which the difference is "clim" cast to double.  It's probably a low likelihood of someone setting the clim to uint8


>> cdata = [1 10];
>> clim = uint8([0 10]);
>> (cdata - clim(1)) / (clim(2) - clim(1))
>> class ( (cdata - clim(1)) / (clim(2) - clim(1)) )
ans = uint8
>> class (cdata)
ans = double


More importantly, is to make the two consistent.

I've tried experimenting to see if casting clim to double is necessary, but I can seem to set(gca,'clim') to make it stick.

Dan Sebald <sebald>
Fri 30 Sep 2016 04:07:58 PM UTC, comment #16: 

I should note that things are much better now when using image(), but the motivating examples from comment #2 which use imshow() still fail.

In fact, the following code hangs until I kill Octave


close all
graphics_toolkit gnuplot
I = (rand (64,64) >= 0.5);
imshow (I)
colormap (gray (2));   # hangs here



Rik <rik5>
Group administrator
Fri 30 Sep 2016 04:00:39 PM UTC, comment #15: 

I pushed your cset here (http://hg.savannah.gnu.org/hgweb/octave/rev/202c6871f07d).

I followed it with some changes to make it more Matlab compatible.


changeset:   22564:3fd1b248bb27
branch:      stable
tag:         tip
user:        Rik <rik@octave.org>
date:        Fri Sep 30 08:51:18 2016 -0700
files:       scripts/plot/util/private/__gnuplot_draw_axes__.m
description:
Better Matlab compatibility for images created with gnuplot (bug #49130).

* __gnuplot_draw_axes__.m (mapcdata): Don't do conversion to
double->uint8->double to achieve clamping of RGB values.  Keep cdata as double
and use indexing to find and clamp values outside range [0, 255].
For scaled data, use scaling factor of colormap_size rather than
colormap_size-1 for compatibility.
For "direct" integer data, convert range from zero-based indices to ones-based.
For "direct" float data, truncate using "fix" rather than "round" for
compatibility.


diff -r b828361c8949 -r 3fd1b248bb27 scripts/plot/util/private/__gnuplot_draw_axes__.m
--- a/scripts/plot/util/private/__gnuplot_draw_axes__.m        Wed Sep 28 15:57:05 2016 -0500
+++ b/scripts/plot/util/private/__gnuplot_draw_axes__.m        Fri Sep 30 08:51:18 2016 -0700
@@ -2849,21 +2849,27 @@ endfunction

 function retval = mapcdata (cdata, mode, clim, cmap_sz)
   if (ndims (cdata) == 3)
-    # True color, clamp data to 8-bit
+    ## True Color, clamp data to 8-bit
     cdata = double (cdata);
-    cdata = uint8 (255*(cdata-clim(1))/(clim(2)-clim(1)));
-    # Scale using inverse of gnuplot's cbrange mapping
-    retval = 1 + double (cdata)*(cmap_sz-1)/255;
+    cdata = 255 * (cdata - clim(1)) / (clim(2)-clim(1));
+    cdata(cdata < 0) = 0;  cdata(cdata > 255) = 255;
+    ## Scale using inverse of gnuplot's cbrange mapping
+    retval = 1 + cdata * (cmap_sz-1)/255;
   else
     if (islogical (cdata))
       cdata += 1;
     elseif (strcmp (mode, "scaled"))
       cdata = double (cdata);
       clim = double (clim);
-      cdata = 1 + fix ((cmap_sz-1)*(cdata-clim(1))/(clim(2)-clim(1)));
+      cdata = 1 + fix (cmap_sz * (cdata - clim(1)) / (clim(2) - clim(1)));
     else
-      cdata = round (cdata);
+      if (isinteger (cdata))
+        cdata += 1;
+      else
+        cdata = fix (cdata);
+      endif
     endif
     retval = max (1, min (cdata, cmap_sz));
   endif
 endfunction
+



Rik <rik5>
Group administrator
Wed 28 Sep 2016 09:15:22 PM UTC, comment #14: 

I'm attaching a new changeset which puts all the color transforming code into one subroutine.  It actually reduces a lot of conditional code in the main routine.

There are a couple bug fixes hadn't been acknowledged beyond the general bug fix.  One is that I removed the code that sets some parent/graphics-object properties when the image data is logical.  That caused a problem with whatever follows the logical example:


I = uint8 (255*rand (64,64));
imshow (I)
[looks fine]

I = (rand (64,64) >= 0.5);
imshow (I)

I = uint8 (255*rand (64,64));
imshow (I)
[shows up mostly black if the colormap is set by logical]


With the fix, the behavior is consistent with qt toolkit.

The second bug was the true color case.  In some sense, it might be nice if gnuplot behaved slightly different, but as it is the cbrange also affects three component RGB data.  That complicates matters since cbrange needs to be proper for the colormap as well.  So, what I've done is use the colormaps length as an input to the inverse mapping of what gnuplot uses when applying its cbrange formula.

I've tested with the patch with the following:


x = uint8 ((1:128)' + (1:128));
y = repmat (x, 1, 1, 3);
imshow (x)
imshow (y)

I = (rand (64,64) >= 0.5);
imshow (I)

I = uint8 (255*rand (64,64));
imshow (I)

demo imshow

demo patch


comparing against qt toolkit.  I also reran the tests here:

https://savannah.gnu.org/bugs/?48664#comment6

to make sure that behavior hasn't changed inadvertently.


(file #38621)

Dan Sebald <sebald>
Wed 28 Sep 2016 01:24:02 AM UTC, comment #13: 

Yes, that's right.  If the data is 3-column RGB data then none of the indexing or scaling applies.

Rik <rik5>
Group administrator
Tue 27 Sep 2016 11:56:14 PM UTC, comment #12: 

Oh, OK.  So direct means index directly to the colormap, no scaling.  What sort of threw me off was the association with "true color" data, but I guess logically there is nothing indicating those two scenarios should be treated similarly.  So, true color data is the one that should be treated as an 8-bit unsigned, limit-wise.  I'll pass the mode setting into the mapcdata() routine and see if all three scenarios can be addressed there.

Dan Sebald <sebald>
Tue 27 Sep 2016 11:40:44 PM UTC, comment #11: 

In "direct" mode the clamping is done to the size of the colormap (LUT) itself.  Something like this is correct.


if (strcmp ("mode", "direct"))
  cmin = 1;
  cmax = numel (colormap ());
  cdata(cdata < cmin) = cmin;
  cdata(cdata > cmax) = cmax;
else  # scaled data
  clim = caxis ();
  cmin = clim(1);
  cmax = clim(2);
  cdata(cdata < cmin) = cmin;
  cdata(cdata > cmax) = cmax;
endif


For "direct", the example above is correct for floating point variables (double or single).  For the integer type variables (uint8, int8, ...) the value '0' is supposed to map to the first color in the colormap.  This may have already been taken care of by the time you reach this point in the rendering code.

Rik <rik5>
Group administrator
Tue 27 Sep 2016 11:20:34 PM UTC, comment #10: 

Rik, I see the paragraph you referenced, but it still doesn't answer for me what to do with "direct" data.  The clamping is needed to prevent indexing outside of a colormap, i.e., LUT.  If the index goes outside the LUT, then it is an indexing error.

But if caxis doesn't affect cdata in "direct" mode, then what should be the clamping limits?  [0 2^8-1], i.e., 8-bit unsigned int?  It wouldn't make sense to use the colormap limits if the caxis has no bearing.

Dan Sebald <sebald>
Fri 23 Sep 2016 03:56:31 PM UTC, comment #9: 

@Dan: Your last cset was almost perfect, the question of clamping was all that remained.  Now that the linestyle patch has been checked in this is all that really remains for gnuplot before the 4.2 release.  The smaller bits, like the background color of the axes object can be done in the 4.2.1 bug fix release.

Rik <rik5>
Group administrator
Tue 20 Sep 2016 03:16:29 PM UTC, comment #8: 

From the documentation for caxis


caxis controls the mapping of data values to the colormap. It affects any surface, patch, or image with indexed CData and CDataMapping set to scaled. It does not affect surfaces, patches, or images with true color CData or with CDataMapping set to direct.


So, according to Mathworks, we shouldn't be putting any "direct" data through mapcdata.

But, as I understand it, clamping has to happen for both "direct" and "scaled" cdatamapping.  For example, if the size of the direct colormap is small there will definitely be out of bounds index values.  As an example


graphics_toolkit qt
I = 1 + uint8 (63 * rand (64,64));   # Data in range [1, 64]
imshow (I);
colormap (jet (8))                   # Most indices will now be > 8, map to red



Rik <rik5>
Group administrator
Tue 20 Sep 2016 06:09:07 AM UTC, comment #7: 

Oh yeah, thanks for the reminder.  I had the funny feeling I checked all that scaling formula stuff but couldn't think why it's the way it is.  I will put that formula back.

The clamping is taken care of after each call to mapcdata().  I had checked that.  I suppose the thinking is that there is the possibility of non-mapped data and that should be range-limited as well.  But, now that I think about this and that three-color plane example:


y = repmat (x, 1, 1, 3);
imshow (y)


not looking correct, this could be where some confusion is.  For example,


      if (strcmp (obj.cdatamapping, "direct"))
        r = round (ccol);
      else
        r = mapcdata (ccol, clim, cmap_sz);
      endif
      r = max (1, min (r, cmap_sz));
      color = cmap(r, :);


maybe shouldn't be passing 'direct' data through the cmap()--it's supposed to be direct 8-bit color component.  If the direct data in this imshow(y) example is 8-bit, passed through a 64-level LUT, it's going to saturate just as in the failed example.

So, perhaps that clamping line of code can be moved inside mapcdata() in all cases.  Let me think this over for a day.

Dan Sebald <sebald>
Tue 20 Sep 2016 04:51:11 AM UTC, comment #6: 

According to Matlab's documentation at http://www.mathworks.com/help/matlab/ref/caxis.html.


index = fix((C-cmin)/(cmax-cmin)*m)+1;
%Clamp values outside the range [1 m]
index(index<1) = 1;
index(index>m) = m;


So it would appear that they use N rather than N-1.

It seems like the last bit, clamping, is an additional requirement for the mapcdata function.  Otherwise, you can easily get negative values when cmin is greater than C.

Rik <rik5>
Group administrator
Tue 20 Sep 2016 12:46:09 AM UTC, comment #5: 

Give the attached patch a try.  It uses a function for the color scaling and within that function first changes the class to double so that uint8 doesn't call math overflows.

I also made a subtle change of using multiplier N-1 rather than N.  It seems that is the more proper thing to do, e.g., if

clim(1) = min
clim(2) = max

then

1 + (N-1)*(max - min)/(max - min) = N

This appears to work hear with the two examples of comment #2.  Example


x = uint8 ((1:128)' + (1:128));
y = repmat (x, 1, 1, 3);
imshow (x)


looks good, but not

imshow (y)

Let me know what was intended.

The only thing I'll add is that a month I recall some discussion about whether figures should autoscale (like surfaces and so on), but that is not necessarily the case because images often use a LUT in concept.  The point being that 'clim' of the axes and/or image make not be 100% correct yet.


(file #38561)

Dan Sebald <sebald>
Mon 19 Sep 2016 09:53:01 PM UTC, comment #4: 

For what it's worth, I reverted back to the cset before


changeset:   22419:be969d43d95f
user:        Daniel J Sebald <daniel.sebald@ieee.org>
date:        Sat Aug 06 02:27:11 2016 -0500
summary:     Rework color scaling for gnuplot toolkit (bug #48664)


and the coloring is now correct for logical and uint8 inputs.

At least we know where to start looking for a solution.

Rik <rik5>
Group administrator
Mon 19 Sep 2016 09:42:21 PM UTC, comment #3: 

I recently wrote a clim patch for this bug:

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

The test examples there produce the results as I remember from a month or two ago.  However, it could be that this patch was the source of the change.  Or the recent color processing changes have been causing problems.  I'll have a look...

Dan Sebald <sebald>
Mon 19 Sep 2016 08:16:17 PM UTC, comment #2: 

There are probably multiple problems here.  A simple test example for logical inputs, i.e., black and white is.


close all
graphics_toolkit gnuplot
I = (rand (64,64) >= 0.5);
imshow (I)


A test case for uint8 input is


close all
graphics_toolkit gnuplot
I = uint8 (255*rand (64,64));
imshow (I)


In each case, I get an all white plot with gnuplot 5.0.1.  With gnuplot 4.6.6 I get an all black plot.  This is for tip


parent: 22513:12ea89cb1237 tip
 Correct gnuplot toolkit color processing for patch edgecolor (bug #49108)


If I use Octave 4.0.3, I still don't get the correct result if I use gnuplot 5.0.1, but gnuplot 4.6.6 now works.

Rik <rik5>
Group administrator
Mon 19 Sep 2016 08:09:12 PM UTC, comment #1: 

Confirmed here, works correctly with Octave 4.0.3. Here is a simple demonstration script showing the two errors described here:


x = uint8 ((1:128)' + (1:128));
y = repmat (x, 1, 1, 3);
imshow (x)
imshow (y)


Same behavior with either gnuplot 4 or 5.

Mike Miller <mtmiller>
Group Member
Mon 19 Sep 2016 05:53:48 PM UTC, original submission:  

this code:

graphics_toolkit gnuplot
I=imread('anyimage.png');
imshow(I)

shows a black image if 'anyimage.png' is B&W; and if it is RGB, you can see the image but with intensities 4-5 times higher than expected.

imshow(I,[]) has no effect whatsoever
imagesc(I) has the same problem

However, if the variable is transformed to double, the image is shown as expected, and the following three examples do work as expected:

imshow(double(I),[])
imshow(double(I)/255)
imagesc(double(I)/255)

Using octave 4.2.0-rc1 current as today Sep-19, with latest stable gnuplot (5.0.4). Octave 4.0.3 is ok with this gnupolot; this problem doesn't appear with graphics_toolkit fltk or qt.

Anonymous

 

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

Attach Files:
   
   
Comment:
   

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by sebald (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by rik5
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by None (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 10 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-10-02 rik5 StatusConfirmed Fixed
        Open/ClosedOpen Closed
    2016-09-28 sebald Attached File- Added octave-gnuplot_color_scale_bug49130-djs2016sep28.patch, #38621
    2016-09-20 rik5 StatusNone Confirmed
    2016-09-20 sebald Attached File- Added octave-gnuplot_color_scale_bug49130-djs2016sep19.patch, #38561
    2016-09-19 rik5 Item GroupRegression Incorrect Result
        StatusConfirmed None
        Carbon-Copy- Added sebald
    2016-09-19 mtmiller Item GroupIncorrect Result Regression
    2016-09-19 mtmiller StatusNone Confirmed

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code