|
From: Daniel J S. <dan...@ie...> - 2006-02-20 13:03:12
Attachments:
interp_color_bug_20feb2006.patch
|
Petr,
I'm having problems plotting an image via
imshow(r,g,b)
in Octave. There may be multiple bugs and I can't seem to reproduce things exactly in pure gnuplot, even using the files that Octave creates.
It began in a case where image(r,g,b) worked OK in x11, but not with the postscript terminal. The bug was causing gnuplot to crash.
In Octave, currently, image(r,g,b) is done using a palette as opposed to true color.
I think it is too difficult to describe what I did for that, but then I started sniffing around and found this: In octave type (with x11 terminal)
A = loadimage ("default.img");
image(A); # no problem
imshow(A,A,A); # I see a crash
image(A); # continues to crash
imshow(A); # seems to fix problem
image(A); # works again
I think I have traced this bug to the routine interpolate_color_from_gray(). To verify this, replace the couple commands:
/* find index, bisecting would be faster */
for (idx = 0; sm_palette.gradient[idx].pos < gray; ++idx)
; /* do nothing */
with
/* find index, bisecting would be faster */
for (idx = 0; sm_palette.gradient[idx].pos < gray; ++idx)
// ; /* do nothing */
if (idx == maxidx)
fprintf(stderr,"interp 2, %d %d\n", maxidx, idx);
fprintf(stderr,"interp 3, %d %d\n", maxidx, idx);
and you will see that when imshow(A,A,A) is issued in Octave idx = maxidx, which is one beyond the limits of the array and the next fprintf("interp 3") doesn't execute.
Attached is a patch to fix this bug. It uses a bisecting method. Please check it over and test it. You'll find that the imshow(A,A,A) now works but doesn't look to be the correct brightness. We'll address that problem next.
Thanks,
Dan
|
|
From: Daniel J S. <dan...@ie...> - 2006-02-20 13:08:31
|
Daniel J Sebald wrote: > Attached is a patch to fix this bug. It uses a bisecting method. > Please check it over and test it. You'll find that the imshow(A,A,A) > now works but doesn't look to be the correct brightness. We'll address > that problem next. Oh, btw, I made the patch behave similar to the existing routine in that it rounds up, like a ceil() function. I didn't look closely, but perhaps you'd like the routine to be round(), unless that's been compensated for somewhere else already. Dan |
|
From: Petr M. <mi...@ph...> - 2006-02-20 17:26:56
|
>> Attached is a patch to fix this bug. It uses a bisecting method. Please
>> check it over and test it. You'll find that the imshow(A,A,A) now works
>> but doesn't look to be the correct brightness. We'll address that problem
>> next.
OK, now the patch, gnuplot does not crash.
On the other hand, it shows the following garbage:
octave|3> A = loadimage ("default.img");
octave|4> imshow(A,A,A);
gnuplot_x11: unknown command
<.'.'.'<.<.<.>/>/>/$@0@0@0,@0@0@03@0@0@0;@0@0@0C@p@p@pJ@p@p@pR@p@p@pZ?p?p?pb>o>o>oi<n<n<nq9m9m9my:m:m:m:m:m:m8l8l8l8l8l8l9,9,9,9m9m9m
>
gnuplot_x11: unknown command
<@0@0@0@0@0@0@p@p@p!@p@p@p)@p@p@p1@p@p@p8@p@p@p@@0@0@0H?p?p?pP>o>o>oW:-:-:-_;-;-;-g;n;n;no:-:-:-v9,9,9,~8l8l8l8l8l8l*e*e*e*%*%*%)$)$)$%(d(d(d,)$)$)$4'd'd'd<($($($D&c&c&cK'd'd'dS'#'#'#[&c&c&cc'd'd'dj'd'd'dr'd'd'dz(d(d(d(d(d(d($($($'d'd'd($($($($($($('#'#'#0&c&c&c8'#'#'#?&c&c&cG'#'#'#O'd'd'dV'#'#'#^'d'd'df'#'#'#n&c&c&cu%c%c%c}($($($*%*%*%
>
gnuplot_x11: unknown command
<.'.'.'<.<.<.>/>/>/$@0@0@0,@0@0@03@0@0@0;@0@0@0C@p@p@pJ@p@p@pR@p@p@pZ?p?p?pb>o>o>oi<n<n<nq9m9m9my:m:m:m:m:m:m8l8l8l8l8l8l9,9,9,9m9m9m
>
octave|5>
> Oh, btw, I made the patch behave similar to the existing routine in that it
> rounds up, like a ceil() function. I didn't look closely, but perhaps you'd
> like the routine to be round(), unless that's been compensated for somewhere
> else already.
I have no idea what you mean. But it does not matter, please send me the
patch as close as to the "traditional" octave or Matlab behaviour as
possible.
Petr
|
|
From: Daniel J S. <dan...@ie...> - 2006-02-20 19:28:02
|
Petr Mikulik wrote:
>>> Attached is a patch to fix this bug. It uses a bisecting method.
>>> Please check it over and test it. You'll find that the imshow(A,A,A)
>>> now works but doesn't look to be the correct brightness. We'll
>>> address that problem next.
>
>
> OK, now the patch, gnuplot does not crash.
>
> On the other hand, it shows the following garbage:
>
> octave|3> A = loadimage ("default.img");
> octave|4> imshow(A,A,A);
> gnuplot_x11: unknown command
> <.'.'.'<.<.<.>/>/>/$@0@0@0 ...
OK, I'm seeing that too, so we're consistent. I think this is a bug different from the patch I sent. I'll see if I can find that quick.
I see there is a strip along the right edge in the image that is not appearing. It looks as though that last line is not being read in and is instead being interpretted as a command. Could be an Octave problem.
> > Oh, btw, I made the patch behave similar to the existing routine in that it rounds up, like a ceil() function. I didn't look closely, but perhaps you'd like the routine to be round(), unless that's been compensated for somewhere else already.
>
> I have no idea what you mean.
What I mean is that by rounding up, if the number of palette colors is of less precision than the value of "gray" that is continually passed in, rounding up will mean that the "zero" value will occur only if it exactly equals the zeroeth value in the palette as opposed. An alternative might be instead the comparison
if ((sm_palette.gradient[tmpidx].pos + sm_palette.gradient[tmpidx+1].pos)/2 < gray)
that's all. (I believe the tmpidx+1 above is safe from ever going out of range and causing a crash... tmpidx will never be maxidx - 1 as the bisecting algorithm is because the loop will break first.
> But it does not matter, please send me the patch as close as to the "traditional" octave or Matlab behaviour as possible.
Well, I think the choice should be in gnuplot, then adjust the Octave script appropriately. So whatever you choose is fine.
Dan
|
|
From: Daniel J S. <dan...@ie...> - 2006-02-21 08:09:09
|
Daniel J Sebald wrote:
> Petr Mikulik wrote:
>
>>>> Attached is a patch to fix this bug. It uses a bisecting method.
>>>> Please check it over and test it. You'll find that the
>>>> imshow(A,A,A) now works but doesn't look to be the correct
>>>> brightness. We'll address that problem next.
>>
>>
>>
>> OK, now the patch, gnuplot does not crash.
>>
>> On the other hand, it shows the following garbage:
>>
>> octave|3> A = loadimage ("default.img");
>> octave|4> imshow(A,A,A);
>> gnuplot_x11: unknown command
>> <.'.'.'<.<.<.>/>/>/$@0@0@0,@0@0@03@0@0@0;@0@0@0C@p@p@pJ@p@p@pR@p@p@pZ?p?p?pb>o>o>oi<n<n<nq9m9m9my:m:m:m:m:m:m8l8l8l8l8l8l9,9,9,9m9m9m
I think I found this little easter egg. After printing out tons of stuff I finally saw this for the buffer offset:
buff_offset = 0
i = 1250
buff_offset = 0
i = 1300
buff_offset = 193
DONE PAL
what is a makepal
total_chars = 6785
buff_offset = 0
This is from comments added to the hunk of 50 palette values sent through the pipe, and somewhere around the 1300 value the pipe paused for other traffic (the middle three lines came from the core fprintf) and only a partial buffer (buff_offset = 193) was read. So, the shorter palettes were generally making it through in time, but not this longer one.
In the palette code there were these comments:
read_input(); /* FIXME: discarding status */
which means that when a partial read was done the code continued on its merry way. The partial read didn't advance the buffer, so there was all kinds of garbage left at the end.
Patch attached.
There is also the bug fix for the interpolation, with a ChangeLog comment.
Could someone please add these and then let me know when CVS has been updated.
Thank you,
Dan
|
|
From: Petr M. <mi...@ph...> - 2006-02-21 09:28:34
|
> There is also the bug fix for the interpolation, with a ChangeLog comment. > > Could someone please add these and then let me know when CVS has been > updated. It is in cvs. I think you should become developer of gnuplot@SF so that you have rights to contribute your code directly to cvs. (Hans-Bernhard or Lars could add you.) --- PM |
|
From: Daniel J S. <dan...@ie...> - 2006-02-21 10:44:05
|
Petr Mikulik wrote: >> There is also the bug fix for the interpolation, with a ChangeLog >> comment. >> >> Could someone please add these and then let me know when CVS has been >> updated. > > > It is in cvs. Thanks, I will have one more short bug fix for something else in a second. > > I think you should become developer of gnuplot@SF so that you have > rights to contribute your code directly to cvs. (Hans-Bernhard or Lars > could add you.) I've thought about it. It's up to you. But I kind of like having more computer science oriented people browse the code to sort of hash things out. (I'm more the DSP/math type.) Perhaps this summer if some larger group-oriented projects emerge. Dan |