Tidy some types in fxcodec/bmp. -- Use FX_*_STRUCT types corresponding to byte layout. -- Use FX_ARGB to identify representation. -- Comment about FX_ARGB in terms of high bits (not pix order). -- Remove BMP_PAL_ENCODE in favor of ArgbEncode(). -- Return span rather than vector<>**. -- Avoid memcpy(). Change-Id: I687174add5bc54d10816a21bea70279455d56681 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/120130 Reviewed-by: Lei Zhang <thestig@chromium.org> Reviewed-by: Thomas Sepez <tsepez@google.com> Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fxcodec/bmp/bmp_decoder.cpp b/core/fxcodec/bmp/bmp_decoder.cpp index 014685a..eac054d 100644 --- a/core/fxcodec/bmp/bmp_decoder.cpp +++ b/core/fxcodec/bmp/bmp_decoder.cpp
@@ -13,7 +13,6 @@ #include "core/fxcodec/fx_codec.h" #include "core/fxcodec/fx_codec_def.h" #include "core/fxcrt/check.h" -#include "core/fxge/dib/fx_dib.h" namespace fxcodec { @@ -30,8 +29,7 @@ int32_t* height, bool* tb_flag, int32_t* components, - int32_t* pal_num, - const std::vector<uint32_t>** palette, + pdfium::span<const FX_ARGB>* palette, CFX_DIBAttribute* pAttribute) { DCHECK(pAttribute); @@ -44,7 +42,6 @@ *height = ctx->m_Bmp.height(); *tb_flag = ctx->m_Bmp.img_tb_flag(); *components = ctx->m_Bmp.components(); - *pal_num = ctx->m_Bmp.pal_num(); *palette = ctx->m_Bmp.palette(); pAttribute->m_wDPIUnit = CFX_DIBAttribute::kResUnitMeter; pAttribute->m_nXDPI = ctx->m_Bmp.dpi_x();
diff --git a/core/fxcodec/bmp/bmp_decoder.h b/core/fxcodec/bmp/bmp_decoder.h index d78e92c..0a619a7 100644 --- a/core/fxcodec/bmp/bmp_decoder.h +++ b/core/fxcodec/bmp/bmp_decoder.h
@@ -8,12 +8,12 @@ #define CORE_FXCODEC_BMP_BMP_DECODER_H_ #include <memory> -#include <vector> #include "core/fxcodec/progressive_decoder_iface.h" #include "core/fxcrt/fx_system.h" #include "core/fxcrt/retain_ptr.h" #include "core/fxcrt/span.h" +#include "core/fxge/dib/fx_dib.h" #ifndef PDF_ENABLE_XFA_BMP #error "BMP must be enabled" @@ -41,8 +41,7 @@ int32_t* height, bool* tb_flag, int32_t* components, - int32_t* pal_num, - const std::vector<uint32_t>** palette, + pdfium::span<const FX_ARGB>* palette, CFX_DIBAttribute* pAttribute); static Status LoadImage(ProgressiveDecoderIface::Context* pContext); static FX_FILESIZE GetAvailInput(ProgressiveDecoderIface::Context* pContext);
diff --git a/core/fxcodec/bmp/cfx_bmpdecompressor.cpp b/core/fxcodec/bmp/cfx_bmpdecompressor.cpp index 80b0576..437168e 100644 --- a/core/fxcodec/bmp/cfx_bmpdecompressor.cpp +++ b/core/fxcodec/bmp/cfx_bmpdecompressor.cpp
@@ -21,15 +21,11 @@ #include "core/fxcrt/numerics/safe_math.h" #include "core/fxcrt/span_util.h" #include "core/fxge/calculate_pitch.h" -#include "core/fxge/dib/fx_dib.h" namespace fxcodec { namespace { -#define BMP_PAL_ENCODE(a, r, g, b) \ - (((uint32_t)(a) << 24) | ((r) << 16) | ((g) << 8) | (b)) - constexpr size_t kBmpCoreHeaderSize = 12; constexpr size_t kBmpInfoHeaderSize = 40; @@ -279,39 +275,41 @@ mask_green_ = 0x03E0; mask_blue_ = 0x001F; } - pal_num_ = 0; + uint32_t palette_entries = 0; if (bit_counts_ < 16) { - pal_num_ = 1 << bit_counts_; - if (color_used_ != 0) - pal_num_ = color_used_; - size_t src_pal_size = pal_num_ * PaletteChannelCount(); - DataVector<uint8_t> src_pal(src_pal_size); + palette_entries = 1 << bit_counts_; + if (color_used_ != 0) { + palette_entries = color_used_; + } + size_t pal_bytes = palette_entries * PaletteChannelCount(); + DataVector<uint8_t> src_pal(pal_bytes); if (!ReadAllOrNone(src_pal)) return BmpDecoder::Status::kContinue; - palette_.resize(pal_num_); + palette_.resize(palette_entries); if (pal_type_ == PalType::kOld) { auto src_pal_data = - fxcrt::truncating_reinterpret_span<FX_RGB_STRUCT<uint8_t>, uint8_t>( + fxcrt::truncating_reinterpret_span<FX_BGR_STRUCT<uint8_t>, uint8_t>( src_pal); for (auto& dest : palette_) { const auto& entry = src_pal_data.front(); - dest = BMP_PAL_ENCODE(0x00, entry.blue, entry.green, entry.red); + dest = ArgbEncode(0x00, entry.red, entry.green, entry.blue); src_pal_data = src_pal_data.subspan(1); } } else { auto src_pal_data = - fxcrt::truncating_reinterpret_span<FX_RGBA_STRUCT<uint8_t>, uint8_t>( + fxcrt::truncating_reinterpret_span<FX_BGRA_STRUCT<uint8_t>, uint8_t>( src_pal); for (auto& dest : palette_) { const auto& entry = src_pal_data.front(); - dest = BMP_PAL_ENCODE(entry.alpha, entry.blue, entry.green, entry.red); + dest = ArgbEncode(entry.alpha, entry.red, entry.green, entry.blue); src_pal_data = src_pal_data.subspan(1); } } } - header_offset_ = std::max( - header_offset_, 14 + img_ifh_size_ + pal_num_ * PaletteChannelCount()); + header_offset_ = + std::max(header_offset_, + 14 + img_ifh_size_ + palette_entries * PaletteChannelCount()); SaveDecodingStatus(DecodeStatus::kDataPre); return BmpDecoder::Status::kSuccess; } @@ -360,7 +358,7 @@ } bool CFX_BmpDecompressor::ValidateColorIndex(uint8_t val) const { - return val < pal_num_; + return val < palette_.size(); } BmpDecoder::Status CFX_BmpDecompressor::DecodeRGB() {
diff --git a/core/fxcodec/bmp/cfx_bmpdecompressor.h b/core/fxcodec/bmp/cfx_bmpdecompressor.h index e5227a7..8061a77 100644 --- a/core/fxcodec/bmp/cfx_bmpdecompressor.h +++ b/core/fxcodec/bmp/cfx_bmpdecompressor.h
@@ -17,6 +17,7 @@ #include "core/fxcrt/retain_ptr.h" #include "core/fxcrt/span.h" #include "core/fxcrt/unowned_ptr.h" +#include "core/fxge/dib/fx_dib.h" class CFX_CodecMemory; @@ -34,12 +35,11 @@ void SetInputBuffer(RetainPtr<CFX_CodecMemory> codec_memory); FX_FILESIZE GetAvailInput() const; - const std::vector<uint32_t>* palette() const { return &palette_; } + pdfium::span<const FX_ARGB> palette() const { return palette_; } uint32_t width() const { return width_; } uint32_t height() const { return height_; } int32_t components() const { return components_; } bool img_tb_flag() const { return img_tb_flag_; } - int32_t pal_num() const { return pal_num_; } int32_t dpi_x() const { return dpi_x_; } int32_t dpi_y() const { return dpi_y_; } @@ -73,7 +73,7 @@ UnownedPtr<const CFX_BmpContext> const context_; DataVector<uint8_t> out_row_buffer_; - std::vector<uint32_t> palette_; + std::vector<FX_ARGB> palette_; uint32_t header_offset_ = 0; uint32_t width_ = 0; uint32_t height_ = 0; @@ -84,7 +84,6 @@ bool img_tb_flag_ = false; uint16_t bit_counts_ = 0; uint32_t color_used_ = 0; - int32_t pal_num_ = 0; PalType pal_type_ = PalType::kNew; uint32_t data_offset_ = 0; uint32_t data_size_ = 0;
diff --git a/core/fxcodec/progressive_decoder.cpp b/core/fxcodec/progressive_decoder.cpp index 24b3980..3d9b9f5 100644 --- a/core/fxcodec/progressive_decoder.cpp +++ b/core/fxcodec/progressive_decoder.cpp
@@ -389,7 +389,6 @@ pPalette = m_pGifPalette; } m_SrcPalette.resize(pal_num); - m_SrcPaletteNumber = pal_num; for (int i = 0; i < pal_num; i++) { m_SrcPalette[i] = ArgbEncode(0xff, pPalette[i].r, pPalette[i].g, pPalette[i].b); @@ -649,10 +648,10 @@ BmpDecoder::StartDecode(this); BmpDecoder::Input(pBmpContext.get(), m_pCodecMemory); - const std::vector<uint32_t>* palette; + pdfium::span<const FX_ARGB> palette; BmpDecoder::Status read_result = BmpDecoder::ReadHeader( pBmpContext.get(), &m_SrcWidth, &m_SrcHeight, &m_BmpIsTopBottom, - &m_SrcComponents, &m_SrcPaletteNumber, &palette, pAttribute); + &m_SrcComponents, &palette, pAttribute); while (read_result == BmpDecoder::Status::kContinue) { FXCODEC_STATUS error_status = FXCODEC_STATUS::kError; if (!BmpReadMoreData(pBmpContext.get(), &error_status)) { @@ -661,7 +660,7 @@ } read_result = BmpDecoder::ReadHeader( pBmpContext.get(), &m_SrcWidth, &m_SrcHeight, &m_BmpIsTopBottom, - &m_SrcComponents, &m_SrcPaletteNumber, &palette, pAttribute); + &m_SrcComponents, &palette, pAttribute); } if (read_result != BmpDecoder::Status::kSuccess) { @@ -709,10 +708,9 @@ m_SrcBPC = 8; m_clipBox = FX_RECT(0, 0, m_SrcWidth, m_SrcHeight); m_pBmpContext = std::move(pBmpContext); - if (m_SrcPaletteNumber) { - m_SrcPalette.resize(m_SrcPaletteNumber); - FXSYS_memcpy(m_SrcPalette.data(), palette->data(), - m_SrcPaletteNumber * sizeof(FX_ARGB)); + if (!palette.empty()) { + m_SrcPalette.resize(palette.size()); + fxcrt::spancpy(pdfium::make_span(m_SrcPalette), palette); } else { m_SrcPalette.clear(); }
diff --git a/core/fxcodec/progressive_decoder.h b/core/fxcodec/progressive_decoder.h index f648595..5b105ab 100644 --- a/core/fxcodec/progressive_decoder.h +++ b/core/fxcodec/progressive_decoder.h
@@ -257,7 +257,6 @@ int m_sizeX = 0; int m_sizeY = 0; int m_TransMethod = -1; - int m_SrcPaletteNumber = 0; int m_SrcRow = 0; FXCodec_Format m_SrcFormat = FXCodec_Invalid; int m_SrcPassNumber = 0;
diff --git a/core/fxge/dib/fx_dib.h b/core/fxge/dib/fx_dib.h index 901b904..d35b2d5 100644 --- a/core/fxge/dib/fx_dib.h +++ b/core/fxge/dib/fx_dib.h
@@ -29,14 +29,16 @@ kArgb = 0x220, }; -using FX_ARGB = uint32_t; -using FX_CMYK = uint32_t; +// Endian-dependent (in theory). +using FX_ARGB = uint32_t; // A in high bits, ..., B in low bits. +using FX_CMYK = uint32_t; // C in high bits, ..., K in low bits. // FX_COLORREF, like win32 COLORREF, is BGR. i.e. 0x00BBGGRR. // Note that while the non-existent alpha component should be set to 0, some // parts of the codebase use 0xFFFFFFFF as a sentinel value to indicate error. using FX_COLORREF = uint32_t; +// Endian-independent, name-ordered by increasing address. template <typename T> struct FX_RGB_STRUCT { T red;