Remove core/fxge dependency from core/fxcrt/css core/fxcrt is the lowest-level foundation layer, but its css/ subcomponent previously depended on core/fxge solely for the FX_ARGB type alias and ArgbEncode() helper in core/fxge/dib/fx_dib.h. Use a local equivalent instead and break the dependence. This avoids dragging in fx_dib.h across several compilation units. TAG=agy CONV=1827ad5a-75b4-4315-a226-f6b7baa3ec5e Change-Id: Ie46cd75fc82848afd2f6d473dd5cfb4593732d23 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/157911 Commit-Queue: Tom Sepez <tsepez@chromium.org> Auto-Submit: Tom Sepez <tsepez@chromium.org> Reviewed-by: Andy Phan <andyphan@chromium.org>
diff --git a/core/fxcrt/DEPS b/core/fxcrt/DEPS index 284f463..054b467 100644 --- a/core/fxcrt/DEPS +++ b/core/fxcrt/DEPS
@@ -1,4 +1,5 @@ include_rules = [ + '-core/fxge', '+partition_alloc', '+third_party/icu', ]
diff --git a/core/fxcrt/css/BUILD.gn b/core/fxcrt/css/BUILD.gn index cd138b6..22bec47 100644 --- a/core/fxcrt/css/BUILD.gn +++ b/core/fxcrt/css/BUILD.gn
@@ -55,10 +55,7 @@ "../../../:pdfium_strict_config", "../../../:pdfium_noshorten_config", ] - deps = [ - "../", - "../../fxge", - ] + deps = [ "../" ] visibility = [ "../../../*" ] }
diff --git a/core/fxcrt/css/cfx_csscolorvalue.cpp b/core/fxcrt/css/cfx_csscolorvalue.cpp index aba2fbf..3945b91 100644 --- a/core/fxcrt/css/cfx_csscolorvalue.cpp +++ b/core/fxcrt/css/cfx_csscolorvalue.cpp
@@ -6,7 +6,7 @@ #include "core/fxcrt/css/cfx_csscolorvalue.h" -CFX_CSSColorValue::CFX_CSSColorValue(FX_ARGB value) +CFX_CSSColorValue::CFX_CSSColorValue(CFX_CSSColor value) : CFX_CSSValue(PrimitiveType::kRGB), value_(value) {} CFX_CSSColorValue::~CFX_CSSColorValue() = default;
diff --git a/core/fxcrt/css/cfx_csscolorvalue.h b/core/fxcrt/css/cfx_csscolorvalue.h index 3e551fc..1f4b20f 100644 --- a/core/fxcrt/css/cfx_csscolorvalue.h +++ b/core/fxcrt/css/cfx_csscolorvalue.h
@@ -7,18 +7,32 @@ #ifndef CORE_FXCRT_CSS_CFX_CSSCOLORVALUE_H_ #define CORE_FXCRT_CSS_CFX_CSSCOLORVALUE_H_ +#include <stdint.h> + #include "core/fxcrt/css/cfx_cssvalue.h" -#include "core/fxge/dib/fx_dib.h" + +// ARGB color packed as 0xAARRGGBB. +using CFX_CSSColor = uint32_t; + +// Explicit ARGB byte packing avoids an upward layering dependency on FX_ARGB +// and fxge pixel-packing functions/macros (such as ArgbEncode()). +constexpr CFX_CSSColor CFX_CSSColorPack(uint8_t a, + uint8_t r, + uint8_t g, + uint8_t b) { + return (static_cast<uint32_t>(a) << 24) | (static_cast<uint32_t>(r) << 16) | + (static_cast<uint32_t>(g) << 8) | static_cast<uint32_t>(b); +} class CFX_CSSColorValue final : public CFX_CSSValue { public: - explicit CFX_CSSColorValue(FX_ARGB color); + explicit CFX_CSSColorValue(CFX_CSSColor color); ~CFX_CSSColorValue() override; - FX_ARGB Value() const { return value_; } + CFX_CSSColor Value() const { return value_; } private: - FX_ARGB value_; + CFX_CSSColor value_; }; #endif // CORE_FXCRT_CSS_CFX_CSSCOLORVALUE_H_
diff --git a/core/fxcrt/css/cfx_csscomputedstyle.cpp b/core/fxcrt/css/cfx_csscomputedstyle.cpp index 3f13756..d574fc2 100644 --- a/core/fxcrt/css/cfx_csscomputedstyle.cpp +++ b/core/fxcrt/css/cfx_csscomputedstyle.cpp
@@ -53,7 +53,7 @@ return inherited_data_.ffont_size_; } -FX_ARGB CFX_CSSComputedStyle::GetColor() const { +CFX_CSSColor CFX_CSSComputedStyle::GetColor() const { return inherited_data_.font_color_; } @@ -73,7 +73,7 @@ inherited_data_.ffont_size_ = fFontSize; } -void CFX_CSSComputedStyle::SetColor(FX_ARGB dwFontColor) { +void CFX_CSSComputedStyle::SetColor(CFX_CSSColor dwFontColor) { inherited_data_.font_color_ = dwFontColor; }
diff --git a/core/fxcrt/css/cfx_csscomputedstyle.h b/core/fxcrt/css/cfx_csscomputedstyle.h index e67d1e8..0905ad1 100644 --- a/core/fxcrt/css/cfx_csscomputedstyle.h +++ b/core/fxcrt/css/cfx_csscomputedstyle.h
@@ -11,11 +11,11 @@ #include <vector> #include "core/fxcrt/css/cfx_css.h" +#include "core/fxcrt/css/cfx_csscolorvalue.h" #include "core/fxcrt/css/cfx_csscustomproperty.h" #include "core/fxcrt/mask.h" #include "core/fxcrt/retain_ptr.h" #include "core/fxcrt/widestring.h" -#include "core/fxge/dib/fx_dib.h" class CFX_CSSValueList; @@ -32,7 +32,7 @@ RetainPtr<CFX_CSSValueList> font_family_; float ffont_size_ = 12.0f; float fline_height_ = 14.0f; - FX_ARGB font_color_ = 0xFF000000; + CFX_CSSColor font_color_ = 0xFF000000; uint16_t wfont_weight_ = 400; CFX_CSSFontVariant font_variant_ = CFX_CSSFontVariant::Normal; CFX_CSSFontStyle font_style_ = CFX_CSSFontStyle::Normal; @@ -66,12 +66,12 @@ CFX_CSSFontVariant GetFontVariant() const; CFX_CSSFontStyle GetFontStyle() const; float GetFontSize() const; - FX_ARGB GetColor() const; + CFX_CSSColor GetColor() const; void SetFontWeight(uint16_t wFontWeight); void SetFontVariant(CFX_CSSFontVariant eFontVariant); void SetFontStyle(CFX_CSSFontStyle eFontStyle); void SetFontSize(float fFontSize); - void SetColor(FX_ARGB dwFontColor); + void SetColor(CFX_CSSColor dwFontColor); const CFX_CSSRect* GetBorderWidth() const; const CFX_CSSRect* GetMarginWidth() const;
diff --git a/core/fxcrt/css/cfx_cssdata.h b/core/fxcrt/css/cfx_cssdata.h index d342be3..1473ead 100644 --- a/core/fxcrt/css/cfx_cssdata.h +++ b/core/fxcrt/css/cfx_cssdata.h
@@ -8,10 +8,10 @@ #define CORE_FXCRT_CSS_CFX_CSSDATA_H_ #include "core/fxcrt/css/cfx_css.h" +#include "core/fxcrt/css/cfx_csscolorvalue.h" #include "core/fxcrt/css/cfx_cssnumbervalue.h" #include "core/fxcrt/css/cfx_cssvalue.h" #include "core/fxcrt/widestring.h" -#include "core/fxge/dib/fx_dib.h" class CFX_CSSData { public: @@ -33,7 +33,7 @@ struct Color { const char* name; // Raw, POD struct. - FX_ARGB value; + CFX_CSSColor value; }; static const Property* GetPropertyByName(WideStringView name);
diff --git a/core/fxcrt/css/cfx_cssdeclaration.cpp b/core/fxcrt/css/cfx_cssdeclaration.cpp index 7ace32c..c5130b9 100644 --- a/core/fxcrt/css/cfx_cssdeclaration.cpp +++ b/core/fxcrt/css/cfx_cssdeclaration.cpp
@@ -73,20 +73,21 @@ } // static. -std::optional<FX_ARGB> CFX_CSSDeclaration::ParseCSSColor(WideStringView value) { +std::optional<CFX_CSSColor> CFX_CSSDeclaration::ParseCSSColor( + WideStringView value) { if (value.Front() == '#') { // Note: empty-tolerant Front(). switch (value.GetLength()) { case 4: { uint8_t red = Hex2Dec((uint8_t)value[1], (uint8_t)value[1]); uint8_t green = Hex2Dec((uint8_t)value[2], (uint8_t)value[2]); uint8_t blue = Hex2Dec((uint8_t)value[3], (uint8_t)value[3]); - return ArgbEncode(255, red, green, blue); + return CFX_CSSColorPack(255, red, green, blue); } case 7: { uint8_t red = Hex2Dec((uint8_t)value[1], (uint8_t)value[2]); uint8_t green = Hex2Dec((uint8_t)value[3], (uint8_t)value[4]); uint8_t blue = Hex2Dec((uint8_t)value[5], (uint8_t)value[6]); - return ArgbEncode(255, red, green, blue); + return CFX_CSSColorPack(255, red, green, blue); } default: return std::nullopt; @@ -113,7 +114,7 @@ ? FXSYS_roundf(maybe_number.value().value * 2.55f) : FXSYS_roundf(maybe_number.value().value); } - return ArgbEncode(255, rgb[0], rgb[1], rgb[2]); + return CFX_CSSColorPack(255, rgb[0], rgb[1], rgb[2]); } const CFX_CSSData::Color* pColor = CFX_CSSData::GetColorByName(value); @@ -340,7 +341,7 @@ break; case CFX_CSSValue::PrimitiveType::kRGB: if (dwType & CFX_CSSVALUETYPE_MaybeColor) { - FX_ARGB color = + CFX_CSSColor color = ParseCSSColor(maybe_next.value().string_view).value_or(0); list.push_back(pdfium::MakeRetain<CFX_CSSColorValue>(color)); }
diff --git a/core/fxcrt/css/cfx_cssdeclaration.h b/core/fxcrt/css/cfx_cssdeclaration.h index 21bce28..c55a0ef 100644 --- a/core/fxcrt/css/cfx_cssdeclaration.h +++ b/core/fxcrt/css/cfx_cssdeclaration.h
@@ -11,6 +11,7 @@ #include <optional> #include <vector> +#include "core/fxcrt/css/cfx_css.h" #include "core/fxcrt/css/cfx_cssdata.h" #include "core/fxcrt/retain_ptr.h" #include "core/fxcrt/widestring.h" @@ -26,7 +27,7 @@ std::vector<std::unique_ptr<CFX_CSSCustomProperty>>::const_iterator; static std::optional<WideStringView> ParseCSSString(WideStringView value); - static std::optional<FX_ARGB> ParseCSSColor(WideStringView value); + static std::optional<CFX_CSSColor> ParseCSSColor(WideStringView value); CFX_CSSDeclaration(); ~CFX_CSSDeclaration(); @@ -47,7 +48,7 @@ void AddProperty(const WideString& prop, const WideString& value); size_t PropertyCountForTesting() const; - std::optional<FX_ARGB> ParseColorForTest(WideStringView value); + std::optional<CFX_CSSColor> ParseColorForTest(WideStringView value); private: void ParseFontProperty(WideStringView value, bool bImportant);
diff --git a/core/fxcrt/css/cfx_cssdeclaration_unittest.cpp b/core/fxcrt/css/cfx_cssdeclaration_unittest.cpp index 081ba52..67fe06b 100644 --- a/core/fxcrt/css/cfx_cssdeclaration_unittest.cpp +++ b/core/fxcrt/css/cfx_cssdeclaration_unittest.cpp
@@ -4,12 +4,15 @@ #include "core/fxcrt/css/cfx_cssdeclaration.h" +#include <stdint.h> + #include <optional> +#include "core/fxcrt/css/cfx_csscolorvalue.h" #include "testing/gtest/include/gtest/gtest.h" TEST(CFXCSSDeclarationTest, HexEncodingParsing) { - std::optional<FX_ARGB> maybe_color; + std::optional<CFX_CSSColor> maybe_color; // Length value invalid. EXPECT_FALSE(CFX_CSSDeclaration::ParseCSSColor(L"#00")); @@ -19,51 +22,37 @@ // Invalid characters maybe_color = CFX_CSSDeclaration::ParseCSSColor(L"#zxytlm"); ASSERT_TRUE(maybe_color.has_value()); - EXPECT_EQ(0, FXARGB_R(maybe_color.value())); - EXPECT_EQ(0, FXARGB_G(maybe_color.value())); - EXPECT_EQ(0, FXARGB_B(maybe_color.value())); + EXPECT_EQ(0xFF000000u, maybe_color.value()); maybe_color = CFX_CSSDeclaration::ParseCSSColor(L"#000"); ASSERT_TRUE(maybe_color.has_value()); - EXPECT_EQ(0, FXARGB_R(maybe_color.value())); - EXPECT_EQ(0, FXARGB_G(maybe_color.value())); - EXPECT_EQ(0, FXARGB_B(maybe_color.value())); + EXPECT_EQ(0xFF000000u, maybe_color.value()); maybe_color = CFX_CSSDeclaration::ParseCSSColor(L"#FFF"); ASSERT_TRUE(maybe_color.has_value()); - EXPECT_EQ(255, FXARGB_R(maybe_color.value())); - EXPECT_EQ(255, FXARGB_G(maybe_color.value())); - EXPECT_EQ(255, FXARGB_B(maybe_color.value())); + EXPECT_EQ(0xFFFFFFFFu, maybe_color.value()); maybe_color = CFX_CSSDeclaration::ParseCSSColor(L"#F0F0F0"); ASSERT_TRUE(maybe_color.has_value()); - EXPECT_EQ(240, FXARGB_R(maybe_color.value())); - EXPECT_EQ(240, FXARGB_G(maybe_color.value())); - EXPECT_EQ(240, FXARGB_B(maybe_color.value())); + EXPECT_EQ(0xFFF0F0F0u, maybe_color.value()); // Upper and lower case characters. maybe_color = CFX_CSSDeclaration::ParseCSSColor(L"#1b2F3c"); ASSERT_TRUE(maybe_color.has_value()); - EXPECT_EQ(27, FXARGB_R(maybe_color.value())); - EXPECT_EQ(47, FXARGB_G(maybe_color.value())); - EXPECT_EQ(60, FXARGB_B(maybe_color.value())); + EXPECT_EQ(0xFF1B2F3Cu, maybe_color.value()); } TEST(CFXCSSDeclarationTest, RGBEncodingParsing) { - std::optional<FX_ARGB> maybe_color; + std::optional<CFX_CSSColor> maybe_color; // Invalid input for rgb() syntax. EXPECT_FALSE(CFX_CSSDeclaration::ParseCSSColor(L"blahblahblah")); maybe_color = CFX_CSSDeclaration::ParseCSSColor(L"rgb(0, 0, 0)"); ASSERT_TRUE(maybe_color.has_value()); - EXPECT_EQ(0, FXARGB_R(maybe_color.value())); - EXPECT_EQ(0, FXARGB_G(maybe_color.value())); - EXPECT_EQ(0, FXARGB_B(maybe_color.value())); + EXPECT_EQ(0xFF000000u, maybe_color.value()); maybe_color = CFX_CSSDeclaration::ParseCSSColor(L"rgb(128,255,48)"); ASSERT_TRUE(maybe_color.has_value()); - EXPECT_EQ(128, FXARGB_R(maybe_color.value())); - EXPECT_EQ(255, FXARGB_G(maybe_color.value())); - EXPECT_EQ(48, FXARGB_B(maybe_color.value())); + EXPECT_EQ(0xFF80FF30u, maybe_color.value()); }