Refactor CPDF_Color to use absl::variant for colorspaces
Refactor the CPDF_Color class to utilize absl::variant for managing
colorspaces, this change ensures that there is no risk of
accidentally using the wrong member variable.
Additionally, update all functions that previously used the old member
variables to work with the new absl::variant implementation.
Bug: 347047622
Change-Id: Ic327216710d7f6fce6be707a8fcaf936af0950eb
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/123190
Reviewed-by: Tom Sepez <tsepez@google.com>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/AUTHORS b/AUTHORS
index 4ddeb4d..abd91f7 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -32,6 +32,7 @@
Michael Doppler <m.doppler@gmail.com>
Miklos Vajna <vmiklos@vmiklos.hu>
Minh Trần <myoki.crystal@gmail.com>
+Mostafa Aboalkasim <mostafa.aboalkasim.offical@gmail.com>
Peter Varga <pvarga@inf.u-szeged.hu>
Ralf Sippl <ralf.sippl@gmail.com>
Robert Collyer <rcollyer99@gmail.com>
diff --git a/core/fpdfapi/page/cpdf_color.cpp b/core/fpdfapi/page/cpdf_color.cpp
index 5f4b50e..46cb8db 100644
--- a/core/fpdfapi/page/cpdf_color.cpp
+++ b/core/fpdfapi/page/cpdf_color.cpp
@@ -21,6 +21,10 @@
CPDF_Color::~CPDF_Color() = default;
+bool CPDF_Color::IsNull() const {
+ return absl::holds_alternative<absl::monostate>(color_data_);
+}
+
bool CPDF_Color::IsPattern() const {
return cs_ && IsPatternInternal();
}
@@ -32,18 +36,16 @@
void CPDF_Color::SetColorSpace(RetainPtr<CPDF_ColorSpace> colorspace) {
cs_ = std::move(colorspace);
if (IsPatternInternal()) {
- buffer_.clear();
- value_ = std::make_unique<PatternValue>();
+ color_data_ = std::make_unique<PatternValue>();
} else {
- buffer_ = cs_->CreateBufAndSetDefaultColor();
- value_.reset();
+ color_data_ = cs_->CreateBufAndSetDefaultColor();
}
}
void CPDF_Color::SetValueForNonPattern(std::vector<float> values) {
CHECK(!IsPatternInternal());
CHECK_LE(cs_->ComponentCount(), values.size());
- buffer_ = std::move(values);
+ color_data_ = std::move(values);
}
void CPDF_Color::SetValueForPattern(RetainPtr<CPDF_Pattern> pattern,
@@ -56,8 +58,10 @@
SetColorSpace(
CPDF_ColorSpace::GetStockCS(CPDF_ColorSpace::Family::kPattern));
}
- value_->SetPattern(std::move(pattern));
- value_->SetComps(values);
+
+ auto& pattern_value = absl::get<std::unique_ptr<PatternValue>>(color_data_);
+ pattern_value->SetPattern(std::move(pattern));
+ pattern_value->SetComps(values);
}
CPDF_Color& CPDF_Color::operator=(const CPDF_Color& that) {
@@ -65,9 +69,19 @@
return *this;
}
- buffer_ = that.buffer_;
- value_ = that.value_ ? std::make_unique<PatternValue>(*that.value_) : nullptr;
cs_ = that.cs_;
+
+ if (absl::holds_alternative<std::vector<float>>(that.color_data_)) {
+ color_data_ = absl::get<std::vector<float>>(that.color_data_);
+ } else if (absl::holds_alternative<std::unique_ptr<PatternValue>>(
+ that.color_data_)) {
+ auto& pattern_value =
+ absl::get<std::unique_ptr<PatternValue>>(that.color_data_);
+ color_data_ = std::make_unique<PatternValue>(*pattern_value);
+ } else {
+ color_data_ = absl::monostate();
+ }
+
return *this;
}
@@ -100,12 +114,15 @@
std::optional<FX_RGB_STRUCT<float>> CPDF_Color::GetRGB() const {
if (IsPatternInternal()) {
- if (value_) {
- return cs_->AsPatternCS()->GetPatternRGB(*value_);
+ if (absl::holds_alternative<std::unique_ptr<PatternValue>>(color_data_)) {
+ const auto& pattern_value =
+ absl::get<std::unique_ptr<PatternValue>>(color_data_);
+ return cs_->AsPatternCS()->GetPatternRGB(*pattern_value);
}
} else {
- if (!buffer_.empty()) {
- return cs_->GetRGB(buffer_);
+ if (absl::holds_alternative<std::vector<float>>(color_data_)) {
+ const auto& buffer = absl::get<std::vector<float>>(color_data_);
+ return cs_->GetRGB(buffer);
}
}
return std::nullopt;
@@ -113,5 +130,8 @@
RetainPtr<CPDF_Pattern> CPDF_Color::GetPattern() const {
DCHECK(IsPattern());
- return value_ ? value_->GetPattern() : nullptr;
+
+ const auto& pattern_value =
+ absl::get<std::unique_ptr<PatternValue>>(color_data_);
+ return pattern_value->GetPattern();
}
diff --git a/core/fpdfapi/page/cpdf_color.h b/core/fpdfapi/page/cpdf_color.h
index d570288..34e32f5 100644
--- a/core/fpdfapi/page/cpdf_color.h
+++ b/core/fpdfapi/page/cpdf_color.h
@@ -16,6 +16,7 @@
#include "core/fxcrt/retain_ptr.h"
#include "core/fxcrt/span.h"
#include "core/fxge/dib/fx_dib.h"
+#include "third_party/abseil-cpp/absl/types/variant.h"
class CPDF_ColorSpace;
class CPDF_Pattern;
@@ -30,7 +31,7 @@
CPDF_Color& operator=(const CPDF_Color& that);
- bool IsNull() const { return buffer_.empty() && !value_; }
+ bool IsNull() const;
bool IsPattern() const;
void SetColorSpace(RetainPtr<CPDF_ColorSpace> colorspace);
void SetValueForNonPattern(std::vector<float> values);
@@ -52,8 +53,10 @@
protected:
bool IsPatternInternal() const;
- std::vector<float> buffer_; // Used for non-pattern colorspaces.
- std::unique_ptr<PatternValue> value_; // Used for pattern colorspaces.
+ absl::variant<absl::monostate,
+ std::vector<float>, // Used for non-pattern colorspaces.
+ std::unique_ptr<PatternValue>> // Used for pattern colorspaces.
+ color_data_;
RetainPtr<CPDF_ColorSpace> cs_;
};
