question about code
Lukáš Jirkovský
I've got a problem with understanding following code from vigra_impex/exr.cxx:
void ExrDecoderImpl::nextScanline()
{
file.setFrameBuffer (pixels.data() - position.x - scanline *
width, 1, width); // set frame buffer
file.readPixels (scanline, scanline); // read one line
scanline++; // set row iterator to next line
// convert scanline to float
float * dest = bands.begin();
for (int i=0; i < width; i++) { // what is this used for?
*dest++ = pixels[i].r;
*dest++ = pixels[i].g;
*dest++ = pixels[i].b;
*dest++ = pixels[i].a;
}
}
I don't get the loop in this function. Why there's changed dest, when
it's not used anywhere?
Moreover, what does it really do? This is how I understand in loop:
Let *dest is 0 and let the pixels[0] is {10,20,30,40}. Then in first
iteration it just changes *dest to 11 then to 21 then 31 and then 41.
paul womack
My context-free reading of it says that it's using a routine
to read pixels from a file (I assume pixels are a widely
used data structure in Vigra).
Since it appears that the pixels are not the ultimately desired
format, a local pointer (dest) is set up, pointing to
a buffer (bands.begin). The loop then iterates
over the pixels, transcribing the data from fields (r,g,b,a)
into a simple sequence of floats in a buffer.
When the loop finishes, the local pointer (dest) is
not used again, but it has served its purpose, in writing
to the correct memory locations in bands.begin().
Thus if another routine READS data
from bands.begin() it will find what it expects.
BugBear (guessing!)
Lukáš Jirkovský
Thank you for explanation. I still don't understand how it works (I've
never seen code like this before, but I'll try to find some references
on the internet because none of my books in my bookshelf cover this),
but at least I know what it is supposed to do.
Lukáš Jirkovský
Anyway then using *dest++ seems redundant to me. Shouldn't dest be
increased only on the first line? I'll try to change and I'll see what
falls out.
Lukáš Jirkovský
Interesting, it seems that removing ++ from the code fixes generation
of weight mask in bug #2033756, but the exr image itself is read only
partially.
Lukáš Jirkovský
I think I've go it, because bands is vector of float and pixels if
vector of rgba which is a struct containing floats. Still I dontt
understand what causes this bug.
Lukáš Jirkovský
Finally I understand that code completely and I could say that
handling loading of images how it's used in khan couldn't work
properly. The problem is that it tries to load openexr image as
grayscale, but it seems impossible through existing interface.
Pablo d'Angelo
Lukáš Jirkovský schrieb:
> Hello,
> I've got a problem with understanding following code from vigra_impex/exr.cxx:
>
> void ExrDecoderImpl::nextScanline()
> {
> file.setFrameBuffer (pixels.data() - position.x - scanline *
> width, 1, width); // set frame buffer
> file.readPixels (scanline, scanline); // read one line
These two lines seek to the next scanline the EXR file
> scanline++; // set row iterator to next line
> // convert scanline to float
> float * dest = bands.begin();
This gets the pointer to the buffer for one line in the image. This
buffer is later copied to the output image (somewhere deeper in the
vigra impex library). So the image is read line by line.
Only 4 channel float images are supported by the vigra OpenEXR reader I
have written. If somebody tries to read import a single channel image
with vigra::importImage(), vigra::importImage() will detect that and
throw a vigra::error (or something like that) exception.
> for (int i=0; i < width; i++) { // what is this used for?
This loops over all pixels in the scanline.
> *dest++ = pixels[i].r;
This copies the pixels (stored in half float format) read by the
file.readPixels() function (comes from libopenexr) into the temporary
scanline buffer dest. As the ++ is used as a postfix operator here, the
one line above is actually a shortcut for:
*dest = pixels[i].r;
dest++;
postfix and prefix operators should be explained in any half decent book
about C. Their usage can be potentially confusing, if its done inside a
complex expression, but here its reasonably simple, so its ok to use them.
> *dest++ = pixels[i].g;
copy the green pixel etc.
> *dest++ = pixels[i].b;
> *dest++ = pixels[i].a;
> }
> }
>
> I don't get the loop in this function. Why there's changed dest, when
> it's not used anywhere?
It is later copied by the vigra::importImage() (which calles
nextScanline() for each line in the image) into the destination image in
memory.
> Moreover, what does it really do? This is how I understand in loop:
> Let *dest is 0 and let the pixels[0] is {10,20,30,40}. Then in first
> iteration it just changes *dest to 11 then to 21 then 31 and then 41.
No, it sets
dest[0] = 11
dest[1] = 21
dest[2] = 31
dest[3] = 41
etc.
ciao
Pablo
Lukáš Jirkovský
helpful for me was actually looking what pixels and bands are (see my
previous post).
2009/1/30 Pablo d'Angelo <pablo....@web.de>:
>
> Hi Lukáš
>
> Lukáš Jirkovský schrieb:
>> Hello,
>> I've got a problem with understanding following code from vigra_impex/exr.cxx:
>>
>> void ExrDecoderImpl::nextScanline()
>> {
>> file.setFrameBuffer (pixels.data() - position.x - scanline *
>> width, 1, width); // set frame buffer
>> file.readPixels (scanline, scanline); // read one line
>
> These two lines seek to the next scanline the EXR file
>
>> scanline++; // set row iterator to next line
>> // convert scanline to float
>> float * dest = bands.begin();
>
> This gets the pointer to the buffer for one line in the image. This
> buffer is later copied to the output image (somewhere deeper in the
> vigra impex library). So the image is read line by line.
> Only 4 channel float images are supported by the vigra OpenEXR reader I
> have written. If somebody tries to read import a single channel image
> with vigra::importImage(), vigra::importImage() will detect that and
> throw a vigra::error (or something like that) exception.
Maybe it would be nice to also add exception for case when someone is
trying to read RGBA image as black and white because then it returns
strange results. Maybe I'll take a look at it, but it can be easily
bypassed by opening image in color and then copying it to the new
grayscale image using vigra::RGBToGrayAccessor.
I've not realized that it actually means dest[0] etc before.