Simplify TIFF decoding
- Change ProgressiveDecoder to always use `FXDIB_Format::kArgb`, since
libtiff can decode images into "kAbgr" format. Stop ProgressiveDecoder
from doing ARGB to RGB conversions.
- Change TiffDecoder to also only use `FXDIB_Format::kArgb`. Also don't
try to fall back to scanline decoding if (whole) image decoding fails.
Then a bunch of scanline decoding code can be removed.
- Pass bitmaps between ProgressiveDecoder and TiffDecoder using move
semantics.
- Replace the remaining TiffBGRA2RGBA() usage with a FX_BGRA_STRUCT
component swap.
Change-Id: I16f7da722720916f15a98f2a352f471332bca690
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/122774
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Tom Sepez <tsepez@google.com>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/core/fxcodec/progressive_decoder.cpp b/core/fxcodec/progressive_decoder.cpp
index c0db252..50299f3 100644
--- a/core/fxcodec/progressive_decoder.cpp
+++ b/core/fxcodec/progressive_decoder.cpp
@@ -727,127 +727,14 @@
}
FXCODEC_STATUS ProgressiveDecoder::TiffContinueDecode() {
- bool ret = false;
- if (m_pDeviceBitmap->GetBPP() == 32) {
- ret = TiffDecoder::Decode(m_pTiffContext.get(), m_pDeviceBitmap);
- m_pDeviceBitmap = nullptr;
- m_pFile = nullptr;
- if (!ret) {
- m_status = FXCODEC_STATUS::kError;
- return m_status;
- }
- m_status = FXCODEC_STATUS::kDecodeFinished;
- return m_status;
- }
-
// TODO(crbug.com/355630556): Consider adding support for
// `FXDIB_Format::kArgbPremul`
- auto pDIBitmap = pdfium::MakeRetain<CFX_DIBitmap>();
- if (!pDIBitmap->Create(m_SrcWidth, m_SrcHeight, FXDIB_Format::kArgb)) {
- m_pDeviceBitmap = nullptr;
- m_pFile = nullptr;
- m_status = FXCODEC_STATUS::kError;
- return m_status;
- }
- ret = TiffDecoder::Decode(m_pTiffContext.get(), pDIBitmap);
- if (!ret) {
- m_pDeviceBitmap = nullptr;
- m_pFile = nullptr;
- m_status = FXCODEC_STATUS::kError;
- return m_status;
- }
- RetainPtr<CFX_DIBitmap> pFormatBitmap;
- bool created_format_bitmap = false;
- switch (m_pDeviceBitmap->GetFormat()) {
- case FXDIB_Format::kInvalid:
- case FXDIB_Format::k1bppRgb:
- case FXDIB_Format::k1bppMask:
- case FXDIB_Format::k8bppRgb:
- case FXDIB_Format::k8bppMask:
- NOTREACHED_NORETURN();
- case FXDIB_Format::kRgb:
- pFormatBitmap = pdfium::MakeRetain<CFX_DIBitmap>();
- created_format_bitmap = pFormatBitmap->Create(
- pDIBitmap->GetWidth(), pDIBitmap->GetHeight(), FXDIB_Format::kRgb);
- break;
- case FXDIB_Format::kRgb32:
- pFormatBitmap = pdfium::MakeRetain<CFX_DIBitmap>();
- created_format_bitmap = pFormatBitmap->Create(
- pDIBitmap->GetWidth(), pDIBitmap->GetHeight(), FXDIB_Format::kRgb32);
- break;
- case FXDIB_Format::kArgb:
- pFormatBitmap = pDIBitmap;
- created_format_bitmap = true;
- break;
-#if defined(PDF_USE_SKIA)
- case FXDIB_Format::kArgbPremul:
- // TODO(crbug.com/355630556): Consider adding support for
- // `FXDIB_Format::kArgbPremul`
- NOTREACHED_NORETURN();
-#endif
- }
- if (!created_format_bitmap) {
- m_pDeviceBitmap = nullptr;
- m_pFile = nullptr;
- m_status = FXCODEC_STATUS::kError;
- return m_status;
- }
-
- UNSAFE_TODO({
- switch (m_pDeviceBitmap->GetFormat()) {
- case FXDIB_Format::kInvalid:
- case FXDIB_Format::k1bppRgb:
- case FXDIB_Format::k1bppMask:
- case FXDIB_Format::k8bppRgb:
- case FXDIB_Format::k8bppMask:
- NOTREACHED_NORETURN();
- case FXDIB_Format::kRgb:
- case FXDIB_Format::kRgb32: {
- int32_t desBpp =
- (m_pDeviceBitmap->GetFormat() == FXDIB_Format::kRgb) ? 3 : 4;
- for (int32_t row = 0; row < pDIBitmap->GetHeight(); row++) {
- const uint8_t* src_line = pDIBitmap->GetScanline(row).data();
- uint8_t* dest_line = pFormatBitmap->GetWritableScanline(row).data();
- for (int32_t col = 0; col < pDIBitmap->GetWidth(); col++) {
- uint8_t _a = 255 - src_line[3];
- uint8_t b = (src_line[0] * src_line[3] + 0xFF * _a) / 255;
- uint8_t g = (src_line[1] * src_line[3] + 0xFF * _a) / 255;
- uint8_t r = (src_line[2] * src_line[3] + 0xFF * _a) / 255;
- *dest_line++ = b;
- *dest_line++ = g;
- *dest_line++ = r;
- dest_line += desBpp - 3;
- src_line += 4;
- }
- }
- break;
- }
- case FXDIB_Format::kArgb:
- break;
-#if defined(PDF_USE_SKIA)
- case FXDIB_Format::kArgbPremul:
- // TODO(crbug.com/355630556): Consider adding support for
- // `FXDIB_Format::kArgbPremul`
- NOTREACHED_NORETURN();
-#endif
- }
- });
- FXDIB_ResampleOptions options;
- options.bInterpolateBilinear = true;
- RetainPtr<CFX_DIBitmap> pStrechBitmap =
- pFormatBitmap->StretchTo(m_SrcWidth, m_SrcHeight, options, nullptr);
- pFormatBitmap = nullptr;
- if (!pStrechBitmap) {
- m_pDeviceBitmap = nullptr;
- m_pFile = nullptr;
- m_status = FXCODEC_STATUS::kError;
- return m_status;
- }
- m_pDeviceBitmap->TransferBitmap(m_SrcWidth, m_SrcHeight,
- std::move(pStrechBitmap), 0, 0);
- m_pDeviceBitmap = nullptr;
+ CHECK_EQ(m_pDeviceBitmap->GetFormat(), FXDIB_Format::kArgb);
+ m_status =
+ TiffDecoder::Decode(m_pTiffContext.get(), std::move(m_pDeviceBitmap))
+ ? FXCODEC_STATUS::kDecodeFinished
+ : FXCODEC_STATUS::kError;
m_pFile = nullptr;
- m_status = FXCODEC_STATUS::kDecodeFinished;
return m_status;
}
#endif // PDF_ENABLE_XFA_TIFF
@@ -1250,14 +1137,14 @@
#ifdef PDF_ENABLE_XFA_BMP
case FXCODEC_IMAGE_BMP:
#endif // PDF_ENABLE_XFA_BMP
-#ifdef PDF_ENABLE_XFA_TIFF
- case FXCODEC_IMAGE_TIFF:
-#endif // PDF_ENABLE_XFA_TIFF
return GetBitsPerPixel() <= 24 ? FXDIB_Format::kRgb
: FXDIB_Format::kRgb32;
#ifdef PDF_ENABLE_XFA_PNG
case FXCODEC_IMAGE_PNG:
#endif // PDF_ENABLE_XFA_PNG
+#ifdef PDF_ENABLE_XFA_TIFF
+ case FXCODEC_IMAGE_TIFF:
+#endif // PDF_ENABLE_XFA_TIFF
default:
// TODO(crbug.com/355630556): Consider adding support for
// `FXDIB_Format::kArgbPremul`
diff --git a/core/fxcodec/tiff/tiff_decoder.cpp b/core/fxcodec/tiff/tiff_decoder.cpp
index c9ff433..c9468e8 100644
--- a/core/fxcodec/tiff/tiff_decoder.cpp
+++ b/core/fxcodec/tiff/tiff_decoder.cpp
@@ -6,8 +6,8 @@
#include "core/fxcodec/tiff/tiff_decoder.h"
-#include <limits>
#include <memory>
+#include <utility>
#include "core/fxcodec/cfx_codec_memory.h"
#include "core/fxcodec/fx_codec.h"
@@ -55,31 +55,13 @@
int32_t* comps,
int32_t* bpc,
CFX_DIBAttribute* pAttribute);
- bool Decode(const RetainPtr<CFX_DIBitmap>& pDIBitmap);
+ bool Decode(RetainPtr<CFX_DIBitmap> bitmap);
RetainPtr<IFX_SeekableReadStream> io_in() const { return m_io_in; }
uint32_t offset() const { return m_offset; }
void set_offset(uint32_t offset) { m_offset = offset; }
private:
- bool IsSupport(const RetainPtr<CFX_DIBitmap>& pDIBitmap) const;
- void SetPalette(const RetainPtr<CFX_DIBitmap>& pDIBitmap, uint16_t bps);
- bool Decode1bppRGB(const RetainPtr<CFX_DIBitmap>& pDIBitmap,
- int32_t height,
- int32_t width,
- uint16_t bps,
- uint16_t spp);
- bool Decode8bppRGB(const RetainPtr<CFX_DIBitmap>& pDIBitmap,
- int32_t height,
- int32_t width,
- uint16_t bps,
- uint16_t spp);
- bool Decode24bppRGB(const RetainPtr<CFX_DIBitmap>& pDIBitmap,
- int32_t height,
- int32_t width,
- uint16_t bps,
- uint16_t spp);
-
RetainPtr<IFX_SeekableReadStream> m_io_in;
uint32_t m_offset = 0;
std::unique_ptr<TIFF, TiffDeleter> m_tif_ctx;
@@ -192,17 +174,6 @@
void tiff_unmap(thandle_t context, tdata_t, toff_t) {}
-void TiffBGRA2RGBA(uint8_t* pBuf, int32_t pixel, int32_t spp) {
- UNSAFE_TODO({
- for (int32_t n = 0; n < pixel; n++) {
- uint8_t tmp = pBuf[0];
- pBuf[0] = pBuf[2];
- pBuf[2] = tmp;
- pBuf += spp;
- }
- });
-}
-
} // namespace
bool CTiffContext::InitDecoder(
@@ -268,199 +239,36 @@
return true;
}
-bool CTiffContext::IsSupport(const RetainPtr<CFX_DIBitmap>& pDIBitmap) const {
- if (TIFFIsTiled(m_tif_ctx.get()))
- return false;
-
- uint16_t photometric = 0;
- if (!TIFFGetField(m_tif_ctx.get(), TIFFTAG_PHOTOMETRIC, &photometric))
- return false;
-
- switch (pDIBitmap->GetBPP()) {
- case 1:
- case 8:
- if (photometric != PHOTOMETRIC_PALETTE) {
- return false;
- }
- break;
- case 24:
- if (photometric != PHOTOMETRIC_RGB) {
- return false;
- }
- break;
- default:
- return false;
- }
- uint16_t planarconfig = 0;
- if (!TIFFGetFieldDefaulted(m_tif_ctx.get(), TIFFTAG_PLANARCONFIG,
- &planarconfig))
- return false;
-
- return planarconfig != PLANARCONFIG_SEPARATE;
-}
-
-void CTiffContext::SetPalette(const RetainPtr<CFX_DIBitmap>& pDIBitmap,
- uint16_t bps) {
- uint16_t* red_orig = nullptr;
- uint16_t* green_orig = nullptr;
- uint16_t* blue_orig = nullptr;
- TIFFGetField(m_tif_ctx.get(), TIFFTAG_COLORMAP, &red_orig, &green_orig,
- &blue_orig);
-#define CVT(x) ((uint16_t)((x) >> 8))
- UNSAFE_TODO({
- for (int32_t i = pdfium::checked_cast<int32_t>((1L << bps) - 1); i >= 0;
- i--) {
- red_orig[i] = CVT(red_orig[i]);
- green_orig[i] = CVT(green_orig[i]);
- blue_orig[i] = CVT(blue_orig[i]);
- }
- int32_t len = 1 << bps;
- for (int32_t index = 0; index < len; index++) {
- uint32_t r = red_orig[index] & 0xFF;
- uint32_t g = green_orig[index] & 0xFF;
- uint32_t b = blue_orig[index] & 0xFF;
- uint32_t color = (uint32_t)b | ((uint32_t)g << 8) | ((uint32_t)r << 16) |
- (((uint32_t)0xffL) << 24);
- pDIBitmap->SetPaletteArgb(index, color);
- }
- });
-#undef CVT
-}
-
-bool CTiffContext::Decode1bppRGB(const RetainPtr<CFX_DIBitmap>& pDIBitmap,
- int32_t height,
- int32_t width,
- uint16_t bps,
- uint16_t spp) {
- if (pDIBitmap->GetBPP() != 1 || spp != 1 || bps != 1 ||
- !IsSupport(pDIBitmap)) {
- return false;
- }
- SetPalette(pDIBitmap, bps);
- int32_t size = static_cast<int32_t>(TIFFScanlineSize(m_tif_ctx.get()));
- uint8_t* buf = reinterpret_cast<uint8_t*>(_TIFFmalloc(size));
- if (!buf) {
- TIFFError(TIFFFileName(m_tif_ctx.get()), "No space for scanline buffer");
- return false;
- }
- for (int32_t row = 0; row < height; row++) {
- uint8_t* bitMapbuffer = pDIBitmap->GetWritableScanline(row).data();
- TIFFReadScanline(m_tif_ctx.get(), buf, row, 0);
- for (int32_t j = 0; j < size; j++) {
- UNSAFE_TODO(bitMapbuffer[j] = buf[j]);
- }
- }
- _TIFFfree(buf);
- return true;
-}
-
-bool CTiffContext::Decode8bppRGB(const RetainPtr<CFX_DIBitmap>& pDIBitmap,
- int32_t height,
- int32_t width,
- uint16_t bps,
- uint16_t spp) {
- if (pDIBitmap->GetBPP() != 8 || spp != 1 || (bps != 4 && bps != 8) ||
- !IsSupport(pDIBitmap)) {
- return false;
- }
- SetPalette(pDIBitmap, bps);
- int32_t size = static_cast<int32_t>(TIFFScanlineSize(m_tif_ctx.get()));
- uint8_t* buf = reinterpret_cast<uint8_t*>(_TIFFmalloc(size));
- if (!buf) {
- TIFFError(TIFFFileName(m_tif_ctx.get()), "No space for scanline buffer");
- return false;
- }
- for (int32_t row = 0; row < height; row++) {
- uint8_t* bitMapbuffer = pDIBitmap->GetWritableScanline(row).data();
- TIFFReadScanline(m_tif_ctx.get(), buf, row, 0);
- UNSAFE_TODO({
- for (int32_t j = 0; j < size; j++) {
- switch (bps) {
- case 4:
- bitMapbuffer[2 * j + 0] = (buf[j] & 0xF0) >> 4;
- bitMapbuffer[2 * j + 1] = (buf[j] & 0x0F) >> 0;
- break;
- case 8:
- bitMapbuffer[j] = buf[j];
- break;
- }
- }
- });
- }
- _TIFFfree(buf);
- return true;
-}
-
-bool CTiffContext::Decode24bppRGB(const RetainPtr<CFX_DIBitmap>& pDIBitmap,
- int32_t height,
- int32_t width,
- uint16_t bps,
- uint16_t spp) {
- if (pDIBitmap->GetBPP() != 24 || !IsSupport(pDIBitmap))
- return false;
-
- int32_t size = static_cast<int32_t>(TIFFScanlineSize(m_tif_ctx.get()));
- uint8_t* buf = reinterpret_cast<uint8_t*>(_TIFFmalloc(size));
- if (!buf) {
- TIFFError(TIFFFileName(m_tif_ctx.get()), "No space for scanline buffer");
- return false;
- }
- for (int32_t row = 0; row < height; row++) {
- uint8_t* bitMapbuffer = pDIBitmap->GetWritableScanline(row).data();
- TIFFReadScanline(m_tif_ctx.get(), buf, row, 0);
- UNSAFE_TODO({
- for (int32_t j = 0; j < size - 2; j += 3) {
- bitMapbuffer[j + 0] = buf[j + 2];
- bitMapbuffer[j + 1] = buf[j + 1];
- bitMapbuffer[j + 2] = buf[j + 0];
- }
- });
- }
- _TIFFfree(buf);
- return true;
-}
-
-bool CTiffContext::Decode(const RetainPtr<CFX_DIBitmap>& pDIBitmap) {
- uint32_t img_width = pDIBitmap->GetWidth();
- uint32_t img_height = pDIBitmap->GetHeight();
+bool CTiffContext::Decode(RetainPtr<CFX_DIBitmap> bitmap) {
+ // TODO(crbug.com/355630556): Consider adding support for
+ // `FXDIB_Format::kArgbPremul`
+ CHECK_EQ(bitmap->GetFormat(), FXDIB_Format::kArgb);
+ const uint32_t img_width = bitmap->GetWidth();
+ const uint32_t img_height = bitmap->GetHeight();
uint32_t width = 0;
uint32_t height = 0;
TIFFGetField(m_tif_ctx.get(), TIFFTAG_IMAGEWIDTH, &width);
TIFFGetField(m_tif_ctx.get(), TIFFTAG_IMAGELENGTH, &height);
- if (img_width != width || img_height != height)
+ if (img_width != width || img_height != height) {
return false;
+ }
- if (pDIBitmap->GetBPP() == 32) {
- uint16_t rotation = ORIENTATION_TOPLEFT;
- TIFFGetField(m_tif_ctx.get(), TIFFTAG_ORIENTATION, &rotation);
- uint32_t* data =
- fxcrt::reinterpret_span<uint32_t>(pDIBitmap->GetWritableBuffer())
- .data();
- if (TIFFReadRGBAImageOriented(m_tif_ctx.get(), img_width, img_height, data,
- rotation, 1)) {
- for (uint32_t row = 0; row < img_height; row++) {
- uint8_t* row_buf = pDIBitmap->GetWritableScanline(row).data();
- TiffBGRA2RGBA(row_buf, img_width, 4);
- }
- return true;
+ uint16_t rotation = ORIENTATION_TOPLEFT;
+ TIFFGetField(m_tif_ctx.get(), TIFFTAG_ORIENTATION, &rotation);
+ uint32_t* data =
+ fxcrt::reinterpret_span<uint32_t>(bitmap->GetWritableBuffer()).data();
+ if (!TIFFReadRGBAImageOriented(m_tif_ctx.get(), img_width, img_height, data,
+ rotation, 1)) {
+ return false;
+ }
+
+ for (uint32_t row = 0; row < img_height; row++) {
+ auto row_span = bitmap->GetWritableScanlineAs<FX_BGRA_STRUCT<uint8_t>>(row);
+ for (auto& pixel : row_span) {
+ std::swap(pixel.blue, pixel.red);
}
}
- uint16_t spp = 0;
- uint16_t bps = 0;
- TIFFGetField(m_tif_ctx.get(), TIFFTAG_SAMPLESPERPIXEL, &spp);
- TIFFGetField(m_tif_ctx.get(), TIFFTAG_BITSPERSAMPLE, &bps);
- FX_SAFE_UINT32 safe_bpp = bps;
- safe_bpp *= spp;
- if (!safe_bpp.IsValid())
- return false;
- uint32_t bpp = safe_bpp.ValueOrDie();
- if (bpp == 1)
- return Decode1bppRGB(pDIBitmap, height, width, bps, spp);
- if (bpp <= 8)
- return Decode8bppRGB(pDIBitmap, height, width, bps, spp);
- if (bpp <= 24)
- return Decode24bppRGB(pDIBitmap, height, width, bps, spp);
- return false;
+ return true;
}
namespace fxcodec {
@@ -491,9 +299,9 @@
// static
bool TiffDecoder::Decode(ProgressiveDecoderIface::Context* pContext,
- const RetainPtr<CFX_DIBitmap>& pDIBitmap) {
+ RetainPtr<CFX_DIBitmap> bitmap) {
auto* ctx = static_cast<CTiffContext*>(pContext);
- return ctx->Decode(pDIBitmap);
+ return ctx->Decode(std::move(bitmap));
}
} // namespace fxcodec
diff --git a/core/fxcodec/tiff/tiff_decoder.h b/core/fxcodec/tiff/tiff_decoder.h
index 8804095..d8e63be 100644
--- a/core/fxcodec/tiff/tiff_decoder.h
+++ b/core/fxcodec/tiff/tiff_decoder.h
@@ -35,8 +35,9 @@
int32_t* comps,
int32_t* bpc,
CFX_DIBAttribute* pAttribute);
+ // `bitmap` must be `FXDIB_Format::kArgb`.
static bool Decode(ProgressiveDecoderIface::Context* ctx,
- const RetainPtr<CFX_DIBitmap>& pDIBitmap);
+ RetainPtr<CFX_DIBitmap> bitmap);
TiffDecoder() = delete;
TiffDecoder(const TiffDecoder&) = delete;
