Avoid exposing JpxDecodeAction
Move JpxDecodeAction out of jpx_decode_conversion.h, and replace JpxDecodeConversion::action() with the set of getters that returns the information CPDF_DIB::LoadJpxBitmap() needs. This encapsulates more JPX-specific decoding logic in JpxDecodeConversion and reduces the size of LoadJpxBitmap().
Change-Id: If05b600f411c5ff8fffb2d5799fe76b16673a137
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/135391
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/core/fpdfapi/page/cpdf_dib.cpp b/core/fpdfapi/page/cpdf_dib.cpp
index 940497a..dbc9a8e 100644
--- a/core/fpdfapi/page/cpdf_dib.cpp
+++ b/core/fpdfapi/page/cpdf_dib.cpp
@@ -626,26 +626,11 @@
DCHECK_NE(0u, components_);
}
- FXDIB_Format format;
- if (conversion.action() == JpxDecodeAction::kUseGray ||
- conversion.action() == JpxDecodeAction::kUseIndexed) {
- format = FXDIB_Format::k8bppRgb;
- } else if (conversion.action() == JpxDecodeAction::kUseRgb &&
- image_info.channels == 3) {
- format = FXDIB_Format::kBgr;
- } else if (conversion.action() == JpxDecodeAction::kUseRgb &&
- image_info.channels == 4) {
- format = FXDIB_Format::kBgrx;
- } else if (conversion.action() == JpxDecodeAction::kConvertArgbToRgb) {
- CHECK_GE(image_info.channels, 4);
- format = FXDIB_Format::kBgrx;
- } else {
- image_info.width = (image_info.width * image_info.channels + 2) / 3;
- format = FXDIB_Format::kBgr;
- }
+ image_info.width = conversion.width();
auto result_bitmap = pdfium::MakeRetain<CFX_DIBitmap>();
- if (!result_bitmap->Create(image_info.width, image_info.height, format)) {
+ if (!result_bitmap->Create(image_info.width, image_info.height,
+ conversion.format())) {
return nullptr;
}
@@ -656,7 +641,7 @@
return nullptr;
}
- if (conversion.action() == JpxDecodeAction::kConvertArgbToRgb) {
+ if (conversion.convert_argb_to_rgb()) {
result_bitmap = ConvertArgbJpxBitmapToRgb(result_bitmap, image_info.width,
image_info.height);
if (!result_bitmap) {
diff --git a/core/fpdfapi/page/jpx_decode_conversion.cpp b/core/fpdfapi/page/jpx_decode_conversion.cpp
index 10773ad..ca4aa91 100644
--- a/core/fpdfapi/page/jpx_decode_conversion.cpp
+++ b/core/fpdfapi/page/jpx_decode_conversion.cpp
@@ -11,9 +11,19 @@
#include "core/fxcrt/check_op.h"
#include "core/fxcrt/notreached.h"
#include "core/fxcrt/retain_ptr.h"
+#include "core/fxge/dib/fx_dib.h"
namespace {
+enum class JpxDecodeAction {
+ kDoNothing,
+ kUseGray,
+ kUseIndexed,
+ kUseRgb,
+ kUseCmyk,
+ kConvertArgbToRgb,
+};
+
// ISO 32000-1:2008 section 7.4.9 says the PDF and JPX colorspaces should have
// the same number of color channels. This helper function checks the
// colorspaces match, but also tolerates unknowns.
@@ -123,6 +133,26 @@
NOTREACHED();
}
+std::optional<FXDIB_Format> GetFormatFromJpxDecodeActionAndImageInfo(
+ JpxDecodeAction action,
+ uint32_t channels) {
+ if (action == JpxDecodeAction::kUseGray ||
+ action == JpxDecodeAction::kUseIndexed) {
+ return FXDIB_Format::k8bppRgb;
+ }
+ if (action == JpxDecodeAction::kUseRgb && channels == 3) {
+ return FXDIB_Format::kBgr;
+ }
+ if (action == JpxDecodeAction::kUseRgb && channels == 4) {
+ return FXDIB_Format::kBgrx;
+ }
+ if (action == JpxDecodeAction::kConvertArgbToRgb) {
+ CHECK_GE(channels, 4);
+ return FXDIB_Format::kBgrx;
+ }
+ return std::nullopt;
+}
+
} // namespace
// static
@@ -139,8 +169,7 @@
}
JpxDecodeConversion conversion;
- conversion.action_ = maybe_action.value();
- switch (conversion.action_) {
+ switch (maybe_action.value()) {
case JpxDecodeAction::kDoNothing:
break;
@@ -154,6 +183,7 @@
case JpxDecodeAction::kUseRgb:
DCHECK_GE(jpx_info.channels, 3);
+ conversion.swap_rgb_ = true;
conversion.override_colorspace_ = nullptr;
break;
@@ -163,6 +193,8 @@
break;
case JpxDecodeAction::kConvertArgbToRgb:
+ conversion.swap_rgb_ = true;
+ conversion.convert_argb_to_rgb_ = true;
conversion.override_colorspace_ = nullptr;
break;
}
@@ -173,6 +205,18 @@
conversion.jpx_components_count_ =
GetComponentCountFromJpxImageInfo(jpx_info);
}
+
+ std::optional<FXDIB_Format> maybe_format =
+ GetFormatFromJpxDecodeActionAndImageInfo(maybe_action.value(),
+ jpx_info.channels);
+ if (maybe_format.has_value()) {
+ conversion.format_ = maybe_format.value();
+ conversion.width_ = jpx_info.width;
+ } else {
+ conversion.format_ = FXDIB_Format::kBgr;
+ conversion.width_ = (jpx_info.width * jpx_info.channels + 2) / 3;
+ }
+
return conversion;
}
diff --git a/core/fpdfapi/page/jpx_decode_conversion.h b/core/fpdfapi/page/jpx_decode_conversion.h
index ea2bbb1..0a2feb9 100644
--- a/core/fpdfapi/page/jpx_decode_conversion.h
+++ b/core/fpdfapi/page/jpx_decode_conversion.h
@@ -9,18 +9,10 @@
#include "core/fxcodec/jpx/cjpx_decoder.h"
#include "core/fxcrt/retain_ptr.h"
+#include "core/fxge/dib/fx_dib.h"
class CPDF_ColorSpace;
-enum class JpxDecodeAction {
- kDoNothing,
- kUseGray,
- kUseIndexed,
- kUseRgb,
- kUseCmyk,
- kConvertArgbToRgb,
-};
-
class JpxDecodeConversion {
public:
static std::optional<JpxDecodeConversion> Create(
@@ -33,8 +25,6 @@
JpxDecodeConversion& operator=(JpxDecodeConversion&&) noexcept;
~JpxDecodeConversion();
- JpxDecodeAction action() const { return action_; }
-
const std::optional<RetainPtr<CPDF_ColorSpace>>& override_colorspace() const {
return override_colorspace_;
}
@@ -43,15 +33,18 @@
return jpx_components_count_;
}
- bool swap_rgb() const {
- return action_ == JpxDecodeAction::kUseRgb ||
- action_ == JpxDecodeAction::kConvertArgbToRgb;
- }
+ bool swap_rgb() const { return swap_rgb_; }
+ bool convert_argb_to_rgb() const { return convert_argb_to_rgb_; }
+ FXDIB_Format format() const { return format_; }
+ uint32_t width() const { return width_; }
private:
JpxDecodeConversion();
- JpxDecodeAction action_;
+ bool swap_rgb_ = false;
+ bool convert_argb_to_rgb_ = false;
+ FXDIB_Format format_ = FXDIB_Format::kInvalid;
+ uint32_t width_ = 0;
// The colorspace to override the existing colorspace.
//
