Endianness bug in expandBinaryPower2Low

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Generate a binary image 51 pixels wide by any amount of pixels high.
2. Perform expandBinaryPower2 with factor==2 on image.
3. Look at right side of resulting image.

What is the expected output? What do you see instead?

The right side of the resulting image is not doubled vertically.

What version of the product are you using? On what operating system?

1.61, on MacOSX 10.5.7

Please provide any additional information below.

Attached are my test files -- ScaleGrayToBinaryFast.png is in the input, and 
ExpandBinaryPower2.png is the resulting output.

I believe the problem is caused by a faulty assumption in the machine's 
endianness in 
expandBinaryPower2Low. Let us assume an 8-pixel wide image. Then the WPL of the 
image is 1.

After doubling, the image is 16 pixels wide, also with a WPL of 1. Suppose that 
we now perform 
GET_DATA_TWO_BYTES(data, 0) on this image, on a little-endian machine. This 
macro will 
correctly look at the last two bytes in the 32-bit word.

However, binExpandPower2Low performs the following operation after horizontally 
doubling a 
line:

            memcpy((char *)(lined + wpld), (char *)lined, 2 * sbytes);

The first (2*sbytes)/4 32-bit words will always be copied correctly, but the 
last (2*sbytes)%4 
bytes will not, because they are assumed to be ordered big-endian, when the 
machine is little-
endian.

There is no corresponding problem with factor=={4, 8, 16} because there the 
memcpy length is 
4*(some integer), which is always aligned on a 32-bit word boundary.

Original issue reported on code.google.com by autoph...@gmail.com on 23 Jun 2009 at 10:11

Attachments:

• ScaleGrayToBinaryFast.png
• ExpandBinaryPower2.png

[GoogleCodeExporter]

My opinion is that it's probably just better to copy by 4*wpld, and eat the one 
extra machine cycle.

Original comment by autoph...@gmail.com on 23 Jun 2009 at 10:16

[GoogleCodeExporter]

Nice catch!

Both your diagnosis of the problem and solution are correct.
I wasn't really trying to save a machine cycle  ;-)

This will be fixed in 1.62, which should be out within 2 weeks.

Thanks for reporting the problem and giving the fix.

  -- Dan

Original comment by dan.bloo...@gmail.com on 23 Jun 2009 at 11:02

[GoogleCodeExporter]

:) A similar bug can be found in pixExpandReplicate. By the way, where are your 
regression test cases? I have 
found these bugs by writing my own :)

Original comment by autoph...@gmail.com on 23 Jun 2009 at 11:03

[GoogleCodeExporter]

Correct again.  Thanks.  Fixed for 1.62.

This was tested with:

    pixs = pixRead("speckle.png");   // your small image
    pixt1 = pixConvert1To2(NULL, pixs, 3, 0);
    pixd = pixExpandReplicate(pixt1, 2);
    pixWrite("junkpixd.png", pixd, IFF_PNG);   // look at it
    pixDestroy(&pixd);
    pixDestroy(&pixt1);
    pixDestroy(&pixs);

There are regression tests in the prog directory, but as you can imagine, they 
don't
cover every possible case.  I am continually adding to the set of regression 
tests,
both for new functions and for existing functions that may not have been 
rigorously
tested.  Check out the *_reg.c files in prog.

Original comment by dan.bloo...@gmail.com on 23 Jun 2009 at 11:36

[GoogleCodeExporter]

Fixed in 1.62.  Regression test added for this and similar scaling.

Original comment by dan.bloo...@gmail.com on 26 Jun 2009 at 3:12

• Changed state: Fixed