vcl/qa/cppunit/graphicfilter/data/tiff/fail/ofz47589.tiff |binary vcl/source/filter/itiff/itiff.cxx | 207 +++----------- 2 files changed, 50 insertions(+), 157 deletions(-)
New commits: commit e4977792c501305502100c0d45b5ca605981008f Author: Caolán McNamara <caol...@redhat.com> AuthorDate: Tue May 24 12:09:51 2022 +0100 Commit: Caolán McNamara <caol...@redhat.com> CommitDate: Tue May 24 16:37:13 2022 +0200 ofz#47589 can't avoid passing in an output in the general case after all there's a flip case which assumes the dest buffer is available, so the 'ALTERNATE RASTER FORMATS' case of man TIFFRGBAImageGet isn't as useful as it looked initially. This reverts commit e912a446210fdae61be3fc04d20d90488cedcdf6, etc Change-Id: I09a614a5469c8996f93af103581099242747fd7d Reviewed-on: https://gerrit.libreoffice.org/c/core/+/134866 Tested-by: Jenkins Reviewed-by: Caolán McNamara <caol...@redhat.com> diff --git a/vcl/qa/cppunit/graphicfilter/data/tiff/fail/ofz47589.tiff b/vcl/qa/cppunit/graphicfilter/data/tiff/fail/ofz47589.tiff new file mode 100644 index 000000000000..936afbd992e4 Binary files /dev/null and b/vcl/qa/cppunit/graphicfilter/data/tiff/fail/ofz47589.tiff differ diff --git a/vcl/source/filter/itiff/itiff.cxx b/vcl/source/filter/itiff/itiff.cxx index 4514ce916959..b682d759f115 100644 --- a/vcl/source/filter/itiff/itiff.cxx +++ b/vcl/source/filter/itiff/itiff.cxx @@ -36,83 +36,10 @@ namespace { SvStream& rStream; tsize_t nSize; - /* - ORIENTATION_TOPLEFT = 1 - ORIENTATION_TOPRIGHT = 2 - ORIENTATION_BOTRIGHT = 3 - ORIENTATION_BOTLEFT = 4 - ORIENTATION_LEFTTOP = 5 - ORIENTATION_RIGHTTOP = 6 - ORIENTATION_RIGHTBOT = 7 - ORIENTATION_LEFTBOT = 8 - */ - uint16_t nOrientation; - - tileContigRoutine pOrigContig; - tileSeparateRoutine pOrigSeparate; - BitmapWriteAccess* pWriteAccess; - BitmapWriteAccess* pAlphaAccess; - std::vector<uint32_t> aBuffer; - Context(SvStream& rInStream, tsize_t nInSize) : rStream(rInStream) , nSize(nInSize) - , nOrientation(0) - , pOrigContig(nullptr) - , pOrigSeparate(nullptr) - , pWriteAccess(nullptr) - , pAlphaAccess(nullptr) - { - } - - uint32_t* GetBuffer(uint32_t w, uint32_t h, int32_t toskew) { - uint32_t nExtraPerRow; - if (toskew >= 0) - nExtraPerRow = toskew; - else - { - int32_t nExtraNeg = w + toskew + w; - nExtraPerRow = std::abs(nExtraNeg); - } - uint32_t nScanLine = w + nExtraPerRow; - aBuffer.resize(nScanLine * h); - uint32_t* pBuffer = aBuffer.data(); - if (toskew < 0) - pBuffer += h * nScanLine - nScanLine; - - return pBuffer; - } - - void SetPixels(uint32_t x, uint32_t y, uint32_t w, uint32_t h, const uint32_t* pSrc, int32_t skew) - { - uint32_t nDestRow = y; - for (uint32_t nRow = 0; nRow < h; ++nRow) - { - for (uint32_t nCol = 0; nCol < w; ++nCol) - { - uint32_t nDestCol; - switch (nOrientation) - { - case ORIENTATION_LEFTBOT: - nDestCol = x + w - 1 - nCol; - break; - default: - nDestCol = x + nCol; - break; - } - - pWriteAccess->SetPixel(nDestRow, nDestCol, - Color(TIFFGetR(*pSrc), TIFFGetG(*pSrc), TIFFGetB(*pSrc))); - pAlphaAccess->SetPixelIndex(nDestRow, nDestCol, 255 - TIFFGetA(*pSrc)); - ++pSrc; - } - pSrc += skew; - if (skew >= 0) - ++nDestRow; - else - --nDestRow; - } } }; } @@ -163,36 +90,6 @@ static toff_t tiff_size(thandle_t handle) return pContext->nSize; } -static void putContigPixel(TIFFRGBAImage* img, uint32_t* /*raster*/, - uint32_t x, uint32_t y, uint32_t w, uint32_t h, - int32_t fromskew, int32_t toskew, - unsigned char* cp) -{ - Context* pContext = static_cast<Context*>(TIFFClientdata(img->tif)); - - uint32_t* pBuffer = pContext->GetBuffer(w, h, toskew); - - (pContext->pOrigContig)(img, pBuffer, 0, 0, w, h, - fromskew, toskew, cp); - - pContext->SetPixels(x, y, w, h, pBuffer, toskew); -} - -static void putSeparatePixel(TIFFRGBAImage* img, uint32_t* /*raster*/, - uint32_t x, uint32_t y, uint32_t w, uint32_t h, - int32_t fromskew, int32_t toskew, - unsigned char* r, unsigned char* g, unsigned char* b, unsigned char* a) -{ - Context* pContext = static_cast<Context*>(TIFFClientdata(img->tif)); - - uint32_t* pBuffer = pContext->GetBuffer(w, h, toskew); - - (pContext->pOrigSeparate)(img, pBuffer, 0, 0, w, h, - fromskew, toskew, r, g, b, a); - - pContext->SetPixels(x, y, w, h, pBuffer, toskew); -} - bool ImportTiffGraphicImport(SvStream& rTIFF, Graphic& rGraphic) { Context aContext(rTIFF, rTIFF.remainingSize()); @@ -228,68 +125,65 @@ bool ImportTiffGraphicImport(SvStream& rTIFF, Graphic& rGraphic) break; } - aContext.nOrientation = 0; - TIFFGetField(tif, TIFFTAG_ORIENTATION, &aContext.nOrientation); - - Bitmap bitmap(Size(w, h), vcl::PixelFormat::N24_BPP); - AlphaMask bitmapAlpha(Size(w, h)); - - BitmapScopedWriteAccess access(bitmap); - AlphaScopedWriteAccess accessAlpha(bitmapAlpha); - - if (!access || !accessAlpha) + size_t npixels = w * h; + std::vector<uint32_t> raster(npixels); + if (TIFFReadRGBAImageOriented(tif, w, h, raster.data(), ORIENTATION_TOPLEFT, 1)) { - SAL_WARN("filter.tiff", "could not create bitmaps"); - break; - } + Bitmap bitmap(Size(w, h), vcl::PixelFormat::N24_BPP); + AlphaMask bitmapAlpha(Size(w, h)); + + BitmapScopedWriteAccess access(bitmap); + AlphaScopedWriteAccess accessAlpha(bitmapAlpha); + + /* + ORIENTATION_TOPLEFT = 1 + ORIENTATION_TOPRIGHT = 2 + ORIENTATION_BOTRIGHT = 3 + ORIENTATION_BOTLEFT = 4 + ORIENTATION_LEFTTOP = 5 + ORIENTATION_RIGHTTOP = 6 + ORIENTATION_RIGHTBOT = 7 + ORIENTATION_LEFTBOT = 8 + */ + uint16_t nOrientation; + if (TIFFGetField(tif, TIFFTAG_ORIENTATION, &nOrientation) != 1) + nOrientation = 0; + + for (uint32_t y = 0; y < h; ++y) + { + const uint32_t* src = raster.data() + w * y; + for (uint32_t x = 0; x < w; ++x) + { + sal_uInt8 r = TIFFGetR(*src); + sal_uInt8 g = TIFFGetG(*src); + sal_uInt8 b = TIFFGetB(*src); + sal_uInt8 a = TIFFGetA(*src); - aContext.pWriteAccess = access.get(); - aContext.pAlphaAccess = accessAlpha.get(); + uint32_t dest; + switch (nOrientation) + { + case ORIENTATION_LEFTBOT: + dest = w - 1 - x; + break; + default: + dest = x; + break; + } - char emsg[1024] = ""; - TIFFRGBAImage img; - bool bOk = false; - // Expanded out TIFFReadRGBAImageOriented to create this block then - // inserted custom "put" methods to write (via a limited size buffer) - // into the final Bitmap incrementally - if (TIFFRGBAImageOK(tif, emsg) && TIFFRGBAImageBegin(&img, tif, 1, emsg)) - { - img.req_orientation = ORIENTATION_TOPLEFT; - assert(img.isContig); - if (img.isContig) - { - aContext.pOrigContig = img.put.contig; - img.put.contig = putContigPixel; - } - else - { - aContext.pOrigSeparate = img.put.separate; - img.put.separate = putSeparatePixel; + access->SetPixel(y, dest, Color(r, g, b)); + accessAlpha->SetPixelIndex(y, dest, 255 - a); + ++src; + } } - // we don't access TIFFRGBAImageGet's raster argument in our custom putContigPixel/ - // putSeparatePixel functions, but TIFFRGBAImageGet nevertheless internally - // advances that pointer, so passing nullptr would cause UBSan nullptr-with-offset - // errors; while technically still UB, this HACK of passing a non-null pointer keeps - // UBSan happy for now (and better use an artificial pointer value which would - // hopefully cause SIGSEGV if it should erroneously be dereferenced after all) - uint32_t* unused_raster = reinterpret_cast<uint32_t *>(sizeof (uint32_t)); - bOk = TIFFRGBAImageGet(&img, unused_raster, w, img.height); - TIFFRGBAImageEnd(&img); - } - else - { - SAL_WARN("filter.tiff", "cannot import tiff, error is: " << emsg); - } + raster.clear(); - access.reset(); - accessAlpha.reset(); + access.reset(); + accessAlpha.reset(); - if (bOk) - { BitmapEx aBitmapEx(bitmap, bitmapAlpha); - switch (aContext.nOrientation) + switch (nOrientation) { case ORIENTATION_LEFTBOT: aBitmapEx.Rotate(2700_deg10, COL_BLACK); @@ -302,7 +196,6 @@ bool ImportTiffGraphicImport(SvStream& rTIFF, Graphic& rGraphic) ANIMATION_TIMEOUT_ON_CLICK, Disposal::Back); aAnimation.Insert(aAnimationBitmap); } - } while (TIFFReadDirectory(tif)); TIFFClose(tif);