Fix embedder test FPDFFormFillEmbedderTest.BUG_1281 for Skia.
When drawing the second bitmap with FPDF_REVERSE_BYTE_ORDER,
Skia/SkiaPaths renders a blue rectangle instead of a red one. This is
because |bRgbByteOrder| is ignored when setting color type for bitmap
inside CFX_SkiaDeviceDriver, while it's supposed to enforce the reverse
byte order of color.
For color rendering, Skia and SkiaPaths are always using color type
|kN32_SkColorType|, of which the value changes between
|kRGBA_8888_SkColorType| and |kBGRA_8888_SkColorType| depending on
whether the program is running on a big endian processor or not.
To make the byte order of color changeable instead of depending on the
processor, this CL makes Skia and SkiaPaths both respect the value of
|bRgbByteOrder| that's passed into CFX_SkiaDeviceDriver, and use
|m_bRgbByteOrder| to hold its value and keep the color type consistent
throughout the rendering process just like AGG.
In addition to the fix, this CL also adds the expected hash values for
Skia/SkiaPaths rendering results in the embedder test, since the small
discrepancies for rendering small rectangles are expected with
Skia/SkiaPaths enabled.
Bug: pdfium:1500
Change-Id: Iad0762a04cc3f266accb4d48177ccb8fd95af894
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/68831
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Hui Yingst <nigi@chromium.org>
diff --git a/core/fxge/skia/fx_skia_device.cpp b/core/fxge/skia/fx_skia_device.cpp
index 9e2e423..9f0414a 100644
--- a/core/fxge/skia/fx_skia_device.cpp
+++ b/core/fxge/skia/fx_skia_device.cpp
@@ -292,6 +292,10 @@
constexpr int kAlternateOrWindingFillModeMask =
FXFILL_ALTERNATE | FXFILL_WINDING;
+SkColorType Get32BitSkColorType(bool is_rgb_byte_order) {
+ return is_rgb_byte_order ? kRGBA_8888_SkColorType : kBGRA_8888_SkColorType;
+}
+
int GetAlternateOrWindingFillMode(int fill_mode) {
return fill_mode & kAlternateOrWindingFillModeMask;
}
@@ -626,7 +630,8 @@
SkBitmap* skBitmap,
int* widthPtr,
int* heightPtr,
- bool forceAlpha) {
+ bool forceAlpha,
+ bool bRgbByteOrder) {
void* buffer = pSource->GetBuffer();
if (!buffer)
return false;
@@ -674,7 +679,7 @@
}
buffer = dst32Storage.get();
rowBytes = width * sizeof(uint32_t);
- colorType = SkColorType::kN32_SkColorType;
+ colorType = Get32BitSkColorType(bRgbByteOrder);
}
break;
case 24: {
@@ -691,12 +696,12 @@
}
buffer = dst32Storage.get();
rowBytes = width * sizeof(uint32_t);
- colorType = SkColorType::kN32_SkColorType;
+ colorType = Get32BitSkColorType(bRgbByteOrder);
alphaType = kOpaque_SkAlphaType;
break;
}
case 32:
- colorType = SkColorType::kN32_SkColorType;
+ colorType = Get32BitSkColorType(bRgbByteOrder);
alphaType = kPremul_SkAlphaType;
pSource->DebugVerifyBitmapIsPreMultiplied(buffer);
break;
@@ -1615,8 +1620,8 @@
#ifdef _SKIA_SUPPORT_PATHS_
m_pClipRgn(nullptr),
m_FillFlags(0),
- m_bRgbByteOrder(bRgbByteOrder),
#endif // _SKIA_SUPPORT_PATHS_
+ m_bRgbByteOrder(bRgbByteOrder),
m_bGroupKnockout(bGroupKnockout) {
SkBitmap skBitmap;
SkColorType color_type;
@@ -1627,7 +1632,7 @@
: kGray_8_SkColorType;
} else {
ASSERT(bpp == 32);
- color_type = kN32_SkColorType;
+ color_type = Get32BitSkColorType(bRgbByteOrder);
}
SkImageInfo imageInfo =
@@ -2363,7 +2368,8 @@
int dstHeight = pBitmap->GetHeight();
int dstRowBytes = dstWidth * sizeof(uint32_t);
SkImageInfo dstImageInfo = SkImageInfo::Make(
- dstWidth, dstHeight, SkColorType::kN32_SkColorType, kPremul_SkAlphaType);
+ dstWidth, dstHeight, Get32BitSkColorType(m_bRgbByteOrder),
+ kPremul_SkAlphaType);
SkBitmap skDstBitmap;
skDstBitmap.installPixels(dstImageInfo, dstBuffer, dstRowBytes);
SkCanvas canvas(skDstBitmap);
@@ -2504,7 +2510,7 @@
SkBitmap skBitmap;
int width, height;
if (!Upsample(pSource, dst8Storage, dst32Storage, &skBitmap, &width, &height,
- false)) {
+ false, m_bRgbByteOrder)) {
return false;
}
{
@@ -2634,11 +2640,11 @@
SkBitmap skBitmap, skMask;
int srcWidth, srcHeight, maskWidth, maskHeight;
if (!Upsample(pSource, src8Storage, src32Storage, &skBitmap, &srcWidth,
- &srcHeight, false)) {
+ &srcHeight, false, m_bRgbByteOrder)) {
return false;
}
if (!Upsample(pMask, mask8Storage, mask32Storage, &skMask, &maskWidth,
- &maskHeight, true)) {
+ &maskHeight, true, m_bRgbByteOrder)) {
return false;
}
{
diff --git a/core/fxge/skia/fx_skia_device.h b/core/fxge/skia/fx_skia_device.h
index 2571769..d423f6f 100644
--- a/core/fxge/skia/fx_skia_device.h
+++ b/core/fxge/skia/fx_skia_device.h
@@ -179,8 +179,8 @@
std::unique_ptr<CFX_ClipRgn> m_pClipRgn;
std::vector<std::unique_ptr<CFX_ClipRgn>> m_StateStack;
int m_FillFlags;
- bool m_bRgbByteOrder;
#endif
+ bool m_bRgbByteOrder;
bool m_bGroupKnockout;
};
#endif // defined _SKIA_SUPPORT_ || defined _SKIA_SUPPORT_PATHS_
diff --git a/fpdfsdk/fpdf_formfill_embeddertest.cpp b/fpdfsdk/fpdf_formfill_embeddertest.cpp
index 436a44c..dfee33c 100644
--- a/fpdfsdk/fpdf_formfill_embeddertest.cpp
+++ b/fpdfsdk/fpdf_formfill_embeddertest.cpp
@@ -1303,15 +1303,14 @@
// Tests using FPDF_REVERSE_BYTE_ORDER with FPDF_FFLDraw(). The two rendered
// bitmaps should be different.
-// TODO(crbug.com/pdfium/11): Fix this test and enable.
+TEST_F(FPDFFormFillEmbedderTest, BUG_1281) {
#if defined(_SKIA_SUPPORT_) || defined(_SKIA_SUPPORT_PATHS_)
-#define MAYBE_BUG_1281 DISABLED_BUG_1281
+ const char kMd5Normal[] = "793689536cf64fe792c2f241888c0cf3";
+ const char kMd5ReverseByteOrder[] = "8077970bbd10333f18186a9bb459bbe6";
#else
-#define MAYBE_BUG_1281 BUG_1281
-#endif
-TEST_F(FPDFFormFillEmbedderTest, MAYBE_BUG_1281) {
const char kMd5Normal[] = "6c674642154408e877d88c6c082d67e9";
const char kMd5ReverseByteOrder[] = "24fff03d1e663b7ece5f6e69ad837124";
+#endif
ASSERT_TRUE(OpenDocument("bug_890322.pdf"));
FPDF_PAGE page = LoadPage(0);
