DanBloomberg / leptonica

Leptonica is an open source library containing software that is broadly useful for image processing and image analysis applications. The official github repository for Leptonica is: danbloomberg/leptonica. See leptonica.org for more documentation.
Other
1.74k stars 387 forks source link

Saving odd-width 24bpp image to BMP+PNG produces color error in rightmost pixel column #686

Closed GerHobbelt closed 1 year ago

GerHobbelt commented 1 year ago

See also comment at #675 for code extract as sample code zip.


WARNING: this issue assumes #684 fixed (via #685), or the 24bpp/spp BMPs will be b0rked severely and this won't be reproducible yet.


Here's the BMP output vs. PNG output of same, as shown in Beyond Compare image comparison view:

image

Relevant code sequence extract shown here (knowing pixReadWithHint is same as pixRead or BMP, but this was coded while looking at #675, where pixReadWithHint was initially used) :

    pix[5] = pixReadWithHint(DEMOPATH("test-rgba.bmp"), IFF_BMP);
    // identical to: pix[5] = pixRead(DEMOPATH("test-rgba.bmp"));

       // reminder: width = 113 px 

    d = pixGetDepth(pix[5]);
    assert(d == 32);
    spp = pixGetSpp(pix[5]);
    assert(spp == 4);
    pix[11] = pixConvert32To24(pix[5]);  // <<-- required to reproduce this issue: only happens for 24bpp PIX
    d = pixGetDepth(pix[11]);
    assert(d == 24);
    spp = pixGetSpp(pix[11]);
    assert(spp == 3);
    ret |= pixWrite("/tmp/lept/bmp-test/target-rgba24.bmp", pix[11], IFF_BMP);
    ret |= pixWrite("/tmp/lept/bmp-test/target-rgba24.png", pix[11], IFF_PNG);

    pix[15] = pixRead("/tmp/lept/bmp-test/target-rgba24.bmp");
    pix[16] = pixRead("/tmp/lept/bmp-test/target-rgba24.png");
    pixEqual(pix[15], pix[5], &same);
    lept_stderr("target: BMP 24A: ..... same: %d\n", same);  // <<-- OK
    pixEqual(pix[16], pix[5], &same);
    lept_stderr("target: BMP 24B: ..... same: %d\n", same);   // <<-- FAIL

Sorry, I don't have a fix for this one yet. Just observed and the issue didn't go away when I blinked. 😉

DanBloomberg commented 1 year ago

Beautiful detective work! It appears that png is screwing up the right-most column when writing 24 bpp, but bmp is ok. So although the problem may be with pixConvert32To24(), it seems more likely it's in the png writer.

I will investigate.

Dan

DanBloomberg commented 1 year ago

Investigated. See: 6c68e008a1b98226d295ee4695f2124b2a968ef5 (incorrectly stated issue #682 instead of issue #686) The error is in png write of 24 bit pix, and I just found it, so will fix it (finally). I was calling pixSetPadBits() on a 24 bit pix, and this was setting some of the components to 0.