Add CPDF_ColorSpace::GetColorRef()
GetColorRef() is a wrapper around GetRGB() that returns a
std::optional<FX_COLORREF>. For it to work correctly, the GetRGB()
results are clamped before being rounded to become integer color
components. PDF 32000-1:2008 section 8.6.5.6 says to adjust to the
nearest valid value when the value is out of range, so this is trying
to implement that. With this change, update the associated pixel test
result now that a large positive value clamps down to 1.
Change CPDF_Color::GetRGB() to use GetColorRef(), so the FX_COLORREF
values just pass straight through. Now GetRGB() no longer needs to do
manual rounding and FX_COLORREF construction. As a result, it cannot
cause undefined behavior if the GetRGB() values are outside of the range
[0, 1].
Also change CPDF_PatternCS::GetPatternRGB() to call GetColorRef().
Change the GetPatternRGB() return type to match GetColorRef() to avoid
extra conversions. This finally gets rid of dead code where
GetPatternRGB() writes to the out-parameters and then returns false so
the out-parameters do not get used.
Bug: b/324312389
Change-Id: I10852794803c7c9b9881ac3d835e9545b5dabab5
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/117094
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Thomas Sepez <tsepez@google.com>
diff --git a/core/fpdfapi/page/cpdf_color.cpp b/core/fpdfapi/page/cpdf_color.cpp
index 32ffaeb..73bc00f 100644
--- a/core/fpdfapi/page/cpdf_color.cpp
+++ b/core/fpdfapi/page/cpdf_color.cpp
@@ -80,26 +80,16 @@
}
std::optional<FX_COLORREF> CPDF_Color::GetRGB() const {
- float r = 0.0f;
- float g = 0.0f;
- float b = 0.0f;
- bool result = false;
if (IsPatternInternal()) {
if (m_pValue) {
- const CPDF_PatternCS* pPatternCS = m_pCS->AsPatternCS();
- result = pPatternCS->GetPatternRGB(*m_pValue, &r, &g, &b);
+ return m_pCS->AsPatternCS()->GetPatternRGB(*m_pValue);
}
} else {
- if (!m_Buffer.empty())
- result = m_pCS->GetRGB(m_Buffer, &r, &g, &b);
+ if (!m_Buffer.empty()) {
+ return m_pCS->GetColorRef(m_Buffer);
+ }
}
- if (!result) {
- return std::nullopt;
- }
-
- return FXSYS_BGR(static_cast<int32_t>(b * 255 + 0.5f),
- static_cast<int32_t>(g * 255 + 0.5f),
- static_cast<int32_t>(r * 255 + 0.5f));
+ return std::nullopt;
}
RetainPtr<CPDF_Pattern> CPDF_Color::GetPattern() const {
diff --git a/core/fpdfapi/page/cpdf_colorspace.cpp b/core/fpdfapi/page/cpdf_colorspace.cpp
index 6468ae0..d458d5e 100644
--- a/core/fpdfapi/page/cpdf_colorspace.cpp
+++ b/core/fpdfapi/page/cpdf_colorspace.cpp
@@ -12,6 +12,7 @@
#include <algorithm>
#include <limits>
#include <memory>
+#include <optional>
#include <type_traits>
#include <utility>
#include <vector>
@@ -39,11 +40,13 @@
#include "core/fxcrt/fx_2d_size.h"
#include "core/fxcrt/fx_memory_wrappers.h"
#include "core/fxcrt/fx_safe_types.h"
+#include "core/fxcrt/fx_system.h"
#include "core/fxcrt/maybe_owned.h"
#include "core/fxcrt/notreached.h"
#include "core/fxcrt/scoped_set_insertion.h"
#include "core/fxcrt/span_util.h"
#include "core/fxcrt/stl_util.h"
+#include "core/fxge/dib/fx_dib.h"
namespace {
@@ -619,6 +622,22 @@
return m_nComponents;
}
+std::optional<FX_COLORREF> CPDF_ColorSpace::GetColorRef(
+ pdfium::span<const float> buffer) {
+ float r;
+ float g;
+ float b;
+ if (!GetRGB(buffer, &r, &g, &b)) {
+ return std::nullopt;
+ }
+
+ r = std::clamp(r, 0.0f, 1.0f);
+ g = std::clamp(g, 0.0f, 1.0f);
+ b = std::clamp(b, 0.0f, 1.0f);
+ return FXSYS_BGR(FXSYS_roundf(b * 255), FXSYS_roundf(g * 255),
+ FXSYS_roundf(r * 255));
+}
+
void CPDF_ColorSpace::GetDefaultValue(int iComponent,
float* value,
float* min,
diff --git a/core/fpdfapi/page/cpdf_colorspace.h b/core/fpdfapi/page/cpdf_colorspace.h
index e6a10dd..8be74e5 100644
--- a/core/fpdfapi/page/cpdf_colorspace.h
+++ b/core/fpdfapi/page/cpdf_colorspace.h
@@ -11,6 +11,7 @@
#include <stdint.h>
#include <array>
+#include <optional>
#include <set>
#include <utility>
#include <vector>
@@ -23,6 +24,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 CPDF_Document;
class CPDF_IndexedCS;
@@ -101,6 +103,11 @@
GetFamily() == Family::kPattern;
}
+ // Wrapper around GetRGB() that returns the RGB value as FX_COLORREF. The
+ // GetRGB() return value is sanitized to fit into FX_COLORREF, where the color
+ // components are integers.
+ std::optional<FX_COLORREF> GetColorRef(pdfium::span<const float> buffer);
+
// Use CPDF_Pattern::GetPatternRGB() instead of GetRGB() for patterns.
virtual bool GetRGB(pdfium::span<const float> pBuf,
float* R,
diff --git a/core/fpdfapi/page/cpdf_patterncs.cpp b/core/fpdfapi/page/cpdf_patterncs.cpp
index 462bf67..c358862 100644
--- a/core/fpdfapi/page/cpdf_patterncs.cpp
+++ b/core/fpdfapi/page/cpdf_patterncs.cpp
@@ -6,6 +6,8 @@
#include "core/fpdfapi/page/cpdf_patterncs.h"
+#include <optional>
+
#include "core/fpdfapi/page/cpdf_docpagedata.h"
#include "core/fpdfapi/parser/cpdf_array.h"
#include "core/fpdfapi/parser/cpdf_document.h"
@@ -53,15 +55,11 @@
return this;
}
-bool CPDF_PatternCS::GetPatternRGB(const PatternValue& value,
- float* R,
- float* G,
- float* B) const {
- if (m_pBaseCS && m_pBaseCS->GetRGB(value.GetComps(), R, G, B))
- return true;
+std::optional<FX_COLORREF> CPDF_PatternCS::GetPatternRGB(
+ const PatternValue& value) const {
+ if (!m_pBaseCS) {
+ return std::nullopt;
+ }
- *R = 0.75f;
- *G = 0.75f;
- *B = 0.75f;
- return false;
+ return m_pBaseCS->GetColorRef(value.GetComps());
}
diff --git a/core/fpdfapi/page/cpdf_patterncs.h b/core/fpdfapi/page/cpdf_patterncs.h
index 896a95c..2fba834 100644
--- a/core/fpdfapi/page/cpdf_patterncs.h
+++ b/core/fpdfapi/page/cpdf_patterncs.h
@@ -7,10 +7,12 @@
#ifndef CORE_FPDFAPI_PAGE_CPDF_PATTERNCS_H_
#define CORE_FPDFAPI_PAGE_CPDF_PATTERNCS_H_
+#include <optional>
#include <set>
#include "core/fpdfapi/page/cpdf_basedcs.h"
#include "core/fxcrt/retain_ptr.h"
+#include "core/fxge/dib/fx_dib.h"
class CPDF_Document;
@@ -33,10 +35,7 @@
const CPDF_Array* pArray,
std::set<const CPDF_Object*>* pVisited) override;
- bool GetPatternRGB(const PatternValue& value,
- float* R,
- float* G,
- float* B) const;
+ std::optional<FX_COLORREF> GetPatternRGB(const PatternValue& value) const;
private:
CPDF_PatternCS();
diff --git a/testing/resources/pixel/icc_profile_bad_value_expected.pdf.0.png b/testing/resources/pixel/icc_profile_bad_value_expected.pdf.0.png
index f6f907e2..319932a 100644
--- a/testing/resources/pixel/icc_profile_bad_value_expected.pdf.0.png
+++ b/testing/resources/pixel/icc_profile_bad_value_expected.pdf.0.png
Binary files differ
