Remove out-parameters from CPDF_Color::GetRGB()
Return std::optional<FX_COLORREF> instead, and adjust callers to use
that. Add some TODOs for possible improvements in the future.
Change-Id: I3f2988a98db8bbfced5ab607fc3b42dab6e2d3ab
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/117092
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Thomas Sepez <tsepez@google.com>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp b/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp
index 24d4496..1532b13 100644
--- a/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp
+++ b/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp
@@ -8,6 +8,7 @@
#include <map>
#include <memory>
+#include <optional>
#include <set>
#include <sstream>
#include <tuple>
@@ -51,15 +52,21 @@
// Value: The resource names of a given type.
using ResourcesMap = std::map<ByteString, std::set<ByteString>>;
+// TODO(thestig): Remove out parameter and raw pointer.
bool GetColor(const CPDF_Color* pColor, float* rgb) {
- int intRGB[3];
- if (!pColor || !pColor->IsColorSpaceRGB() ||
- !pColor->GetRGB(&intRGB[0], &intRGB[1], &intRGB[2])) {
+ if (!pColor || !pColor->IsColorSpaceRGB()) {
return false;
}
- rgb[0] = intRGB[0] / 255.0f;
- rgb[1] = intRGB[1] / 255.0f;
- rgb[2] = intRGB[2] / 255.0f;
+
+ std::optional<FX_COLORREF> colors = pColor->GetRGB();
+ if (!colors.has_value()) {
+ return false;
+ }
+
+ // TODO(thestig): Remove float to int to float conversion.
+ rgb[0] = FXSYS_GetRValue(colors.value()) / 255.0f;
+ rgb[1] = FXSYS_GetGValue(colors.value()) / 255.0f;
+ rgb[2] = FXSYS_GetBValue(colors.value()) / 255.0f;
return true;
}
diff --git a/core/fpdfapi/page/cpdf_color.cpp b/core/fpdfapi/page/cpdf_color.cpp
index 9ab04b1..32ffaeb 100644
--- a/core/fpdfapi/page/cpdf_color.cpp
+++ b/core/fpdfapi/page/cpdf_color.cpp
@@ -6,6 +6,7 @@
#include "core/fpdfapi/page/cpdf_color.h"
+#include <optional>
#include <utility>
#include "core/fpdfapi/page/cpdf_patterncs.h"
@@ -78,7 +79,7 @@
CPDF_ColorSpace::GetStockCS(CPDF_ColorSpace::Family::kDeviceRGB);
}
-bool CPDF_Color::GetRGB(int* R, int* G, int* B) const {
+std::optional<FX_COLORREF> CPDF_Color::GetRGB() const {
float r = 0.0f;
float g = 0.0f;
float b = 0.0f;
@@ -92,13 +93,13 @@
if (!m_Buffer.empty())
result = m_pCS->GetRGB(m_Buffer, &r, &g, &b);
}
- if (!result)
- return false;
+ if (!result) {
+ return std::nullopt;
+ }
- *R = static_cast<int32_t>(r * 255 + 0.5f);
- *G = static_cast<int32_t>(g * 255 + 0.5f);
- *B = static_cast<int32_t>(b * 255 + 0.5f);
- return true;
+ 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));
}
RetainPtr<CPDF_Pattern> CPDF_Color::GetPattern() const {
diff --git a/core/fpdfapi/page/cpdf_color.h b/core/fpdfapi/page/cpdf_color.h
index 1a745e8..236b2f0 100644
--- a/core/fpdfapi/page/cpdf_color.h
+++ b/core/fpdfapi/page/cpdf_color.h
@@ -10,10 +10,12 @@
#include <stdint.h>
#include <memory>
+#include <optional>
#include <vector>
#include "core/fxcrt/retain_ptr.h"
#include "core/fxcrt/span.h"
+#include "core/fxge/dib/fx_dib.h"
class CPDF_ColorSpace;
class CPDF_Pattern;
@@ -37,7 +39,7 @@
uint32_t ComponentCount() const;
bool IsColorSpaceRGB() const;
- bool GetRGB(int* R, int* G, int* B) const;
+ std::optional<FX_COLORREF> GetRGB() const;
// Should only be called if IsPattern() returns true.
RetainPtr<CPDF_Pattern> GetPattern() const;
diff --git a/core/fpdfapi/page/cpdf_colorstate.cpp b/core/fpdfapi/page/cpdf_colorstate.cpp
index 0b8ef97..ee725a7 100644
--- a/core/fpdfapi/page/cpdf_colorstate.cpp
+++ b/core/fpdfapi/page/cpdf_colorstate.cpp
@@ -127,22 +127,18 @@
if (!color.IsPattern()) {
color.SetValueForNonPattern(std::move(values));
}
- int R;
- int G;
- int B;
- return color.GetRGB(&R, &G, &B) ? FXSYS_BGR(B, G, R) : 0xFFFFFFFF;
+ return color.GetRGB().value_or(0xFFFFFFFF);
}
FX_COLORREF CPDF_ColorState::SetPattern(RetainPtr<CPDF_Pattern> pattern,
pdfium::span<float> values,
CPDF_Color& color) {
color.SetValueForPattern(pattern, values);
- int R;
- int G;
- int B;
- if (color.GetRGB(&R, &G, &B)) {
- return FXSYS_BGR(B, G, R);
+ std::optional<FX_COLORREF> colorref = color.GetRGB();
+ if (colorref.has_value()) {
+ return colorref.value();
}
+
CPDF_TilingPattern* tiling = pattern->AsTilingPattern();
return tiling && tiling->colored() ? 0x00BFBFBF : 0xFFFFFFFF;
}
