[rust png] Introduce separate `enum class DecodingColorType`.
Having a separate `DecodingColorType` means that:
* `DecodingColorType` can have just 2 variants. And this means that
drop `NOTREACHED` `case`s from a `switch` in `png_decoder.cpp` and
then hoist the only remaining `case` out of the `switch` (dropping
the `switch`ing altogether)
* The variants of `DecodingColorType` can be named more directly
(truecolor doesn't necessarily imply BGR vs RGB). And this in turn
should hopefully make follow-up Skia integration easier.
* "in-out" parameter of `PngReadHeader` can be replaced with two
separate parameters - one "in", and one "out". This seems easier
to read and understand than the old code.
Bug: 444045690
Change-Id: I93ac808159404d6bd45a0a4e3df8976b2e4d4b2e
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/135615
Commit-Queue: Łukasz Anforowicz <lukasza@google.com>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/core/fxcodec/png/png_decoder.cpp b/core/fxcodec/png/png_decoder.cpp
index 9403dc9..950e28a 100644
--- a/core/fxcodec/png/png_decoder.cpp
+++ b/core/fxcodec/png/png_decoder.cpp
@@ -24,6 +24,7 @@
#define PNG_ERROR_SIZE 256
+using DecodedColorType = fxcodec::PngDecoder::Delegate::DecodedColorType;
using EncodedColorType = fxcodec::PngDecoder::Delegate::EncodedColorType;
class CPngContext final : public ProgressiveDecoderIface::Context {
@@ -121,11 +122,12 @@
static_assert(static_cast<int>(EncodedColorType::kTruecolorWithAlpha) ==
PNG_COLOR_TYPE_RGB_ALPHA);
static_assert(sizeof(EncodedColorType) == sizeof(int));
- auto color_type = static_cast<EncodedColorType>(libpng_color_type);
+ auto src_color_type = static_cast<EncodedColorType>(libpng_color_type);
+ DecodedColorType dst_color_type;
double gamma = 1.0;
- if (!pContext->delegate_->PngReadHeader(width, height, bpc, pass, &color_type,
- &gamma)) {
+ if (!pContext->delegate_->PngReadHeader(
+ width, height, bpc, pass, src_color_type, &dst_color_type, &gamma)) {
// Note that `png_error` function is marked as `PNG_NORETURN`.
png_error(pContext->png_, "Read Header Callback Error");
}
@@ -140,35 +142,19 @@
png_set_gamma(png_ptr, gamma, 0.45455);
}
}
- switch (color_type) {
- case EncodedColorType::kGrayscale:
- case EncodedColorType::kGrayscaleWithAlpha:
- case EncodedColorType::kIndexedColor:
- // These cases are never reached, because the only delegate impl only
- // ever asks to decode into either `kTruecolor` or `kTruecolorWithAlpha`.
- NOTREACHED();
- break;
- case EncodedColorType::kTruecolor:
- case EncodedColorType::kTruecolorWithAlpha:
- if (!(libpng_color_type & PNG_COLOR_MASK_COLOR)) {
- png_set_gray_to_rgb(png_ptr);
- }
- png_set_bgr(png_ptr);
- break;
+ if (!(libpng_color_type & PNG_COLOR_MASK_COLOR)) {
+ png_set_gray_to_rgb(png_ptr);
}
- switch (color_type) {
- case EncodedColorType::kTruecolor:
+ png_set_bgr(png_ptr);
+ switch (dst_color_type) {
+ case DecodedColorType::kBgr:
png_set_strip_alpha(png_ptr);
break;
- case EncodedColorType::kTruecolorWithAlpha:
+ case DecodedColorType::kBgra:
if (!(libpng_color_type & PNG_COLOR_MASK_ALPHA)) {
png_set_filler(png_ptr, 0xff, PNG_FILLER_AFTER);
}
break;
- default:
- // The only delegate impl never asks to decode into other color types.
- NOTREACHED();
- break;
}
png_read_update_info(png_ptr, info_ptr);
}
diff --git a/core/fxcodec/png/png_decoder.h b/core/fxcodec/png/png_decoder.h
index 1957f5f..d1f1cb6 100644
--- a/core/fxcodec/png/png_decoder.h
+++ b/core/fxcodec/png/png_decoder.h
@@ -38,25 +38,30 @@
kTruecolorWithAlpha = 6,
};
+ // Color format to decode into.
+ enum class DecodedColorType {
+ kBgr,
+ kBgra,
+ };
+
// Called by `PngDecoder` after decoding the image header:
//
// * `width` and `height` specify image dimensions in pixels
// * `bpc` is number of bits per component (e.g. per red, or per alpha)
- // * `*color_type` is initially set to the color type the image has been
- // encoded with
+ // * `src_color_type` is the color type the image has been encoded with
//
// Implementation should:
//
// * Return `true` upon success (and `false` otherwise)
- // * Set `*color_type` to the color type to decode into
- // (currently only `kTruecolor` and `kTruecolorWithAlpha` are supported)
+ // * Set `*dst_color_type` to the color type to decode into
// * Set `*gamma` to the target gamma to decode with
// * TODO(crbug.com/355630556): Add out parameter for desired alpha-premul.
virtual bool PngReadHeader(int width,
int height,
int bpc,
int pass,
- EncodedColorType* color_type,
+ EncodedColorType src_color_type,
+ DecodedColorType* dst_color_type,
double* gamma) = 0;
// Called by `PngDecoder` to ask where to write decoded pixels.
diff --git a/core/fxcodec/progressive_decoder.cpp b/core/fxcodec/progressive_decoder.cpp
index 7285f67..4fa0e8e 100644
--- a/core/fxcodec/progressive_decoder.cpp
+++ b/core/fxcodec/progressive_decoder.cpp
@@ -48,6 +48,7 @@
constexpr size_t kBlockSize = 4096;
#ifdef PDF_ENABLE_XFA_PNG
+using PngDecodedColorType = fxcodec::PngDecoder::Delegate::DecodedColorType;
using PngEncodedColorType = fxcodec::PngDecoder::Delegate::EncodedColorType;
#if BUILDFLAG(IS_APPLE)
const double kPngGamma = 1.7;
@@ -99,14 +100,15 @@
int height,
int bpc,
int pass,
- PngEncodedColorType* color_type,
+ PngEncodedColorType src_color_type,
+ PngDecodedColorType* dst_color_type,
double* gamma) {
if (!device_bitmap_) {
src_width_ = width;
src_height_ = height;
src_bpc_ = bpc;
src_pass_number_ = pass;
- src_components_ = GetNumberOfSrcComponents(*color_type);
+ src_components_ = GetNumberOfSrcComponents(src_color_type);
return false;
}
switch (device_bitmap_->GetFormat()) {
@@ -117,11 +119,11 @@
case FXDIB_Format::k8bppRgb:
NOTREACHED();
case FXDIB_Format::kBgr:
- *color_type = PngEncodedColorType::kTruecolor;
+ *dst_color_type = DecodedColorType::kBgr;
break;
case FXDIB_Format::kBgrx:
case FXDIB_Format::kBgra:
- *color_type = PngEncodedColorType::kTruecolorWithAlpha;
+ *dst_color_type = DecodedColorType::kBgra;
break;
#if defined(PDF_USE_SKIA)
case FXDIB_Format::kBgraPremul:
diff --git a/core/fxcodec/progressive_decoder.h b/core/fxcodec/progressive_decoder.h
index 3c7ab8e..3589dfb 100644
--- a/core/fxcodec/progressive_decoder.h
+++ b/core/fxcodec/progressive_decoder.h
@@ -90,7 +90,8 @@
int height,
int bpc,
int pass,
- PngDecoder::Delegate::EncodedColorType* color_type,
+ PngDecoder::Delegate::EncodedColorType src_color_type,
+ PngDecoder::Delegate::DecodedColorType* dst_color_type,
double* gamma) override;
uint8_t* PngAskScanlineBuf(int line) override;
void PngFillScanlineBufCompleted(int line) override;
