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;
