Check `FXDIB_Format::kArgbPremul` in CFX_DIBBase::IsPremultiplied()
Remove `CFX_DIBitmap::m_nFormat` and use `CFX_DIBBase::m_Format` as the
canonical source of bitmap format information. Also remove
CFX_DIBitmap::ForcePreMultiply(), as bitmaps can be pre-multiplied to
start with if they are of `FXDIB_Format::kArgbPremul`.
Update some test expectations to match this change. Most of the changes
are off by 1 pixel values due to rounding errors. When comparing the
old and new bitmaps, they are not pixel perfect according to
pdfium_diff, but will pass with --fuzzy.
Also update DEPS with new corpus test expectations to match this change:
https://pdfium.googlesource.com/pdfium_tests/+log/a062df7af5b3..8c3dc126d92c
Bug: 42271020
Change-Id: I7726b3915602c0ca1845a19e28f9d2d6cc107faf
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/122893
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@google.com>
diff --git a/DEPS b/DEPS
index 75e8198..6e7c9cc 100644
--- a/DEPS
+++ b/DEPS
@@ -174,7 +174,7 @@
# Three lines of non-changing comments so that
# the commit queue can handle CLs rolling pdfium_tests
# and whatever else without interference from each other.
- 'pdfium_tests_revision': 'a062df7af5b34db1aaf40424176e1c1914f6d31f',
+ 'pdfium_tests_revision': '8c3dc126d92cfad8b32830404398c6119be598a5',
# Three lines of non-changing comments so that
# the commit queue can handle CLs rolling resultdb
# and whatever else without interference from each other.
diff --git a/core/fpdfapi/render/cpdf_renderstatus.cpp b/core/fpdfapi/render/cpdf_renderstatus.cpp
index 58a86fa..b45b6a3 100644
--- a/core/fpdfapi/render/cpdf_renderstatus.cpp
+++ b/core/fpdfapi/render/cpdf_renderstatus.cpp
@@ -642,10 +642,8 @@
return true;
m_pDevice->GetDIBits(backdrop, rect.left, rect.top);
}
- // TODO(crbug.com/42271020): Consider adding support for
- // `FXDIB_Format::kArgbPremul`
- if (!bitmap_device.CreateWithBackdrop(width, height, FXDIB_Format::kArgb,
- backdrop)) {
+ if (!bitmap_device.CreateWithBackdrop(
+ width, height, GetCompatibleArgbFormat(), std::move(backdrop))) {
return true;
}
@@ -684,13 +682,6 @@
bitmap_render.SetInGroup(transparency.IsGroup());
bitmap_render.Initialize(nullptr, nullptr);
bitmap_render.ProcessObjectNoClip(pPageObj, new_matrix);
-#if defined(PDF_USE_SKIA)
- if (CFX_DefaultRenderDevice::UseSkiaRenderer()) {
- // Safe because `CFX_SkiaDeviceDriver` always uses pre-multiplied alpha.
- // TODO(crbug.com/42271020): Remove the need for this.
- bitmap_device.GetBitmap()->ForcePreMultiply();
- }
-#endif
m_bStopped = bitmap_render.m_bStopped;
if (pSMaskDict) {
CFX_Matrix smask_matrix =
@@ -714,11 +705,6 @@
if (pPageObj->IsForm()) {
transparency.SetGroup();
}
-#if defined(PDF_USE_SKIA)
- if (CFX_DefaultRenderDevice::UseSkiaRenderer()) {
- bitmap_device.GetBitmap()->UnPreMultiply();
- }
-#endif
CompositeDIBitmap(bitmap_device.GetBitmap(), rect.left, rect.top,
/*mask_argb=*/0, /*alpha=*/1.0f, blend_type, transparency);
return true;
@@ -1494,3 +1480,12 @@
static_cast<int>(rgb.green * 255),
static_cast<int>(rgb.blue * 255));
}
+
+FXDIB_Format CPDF_RenderStatus::GetCompatibleArgbFormat() const {
+#if defined(PDF_USE_SKIA)
+ if (m_pDevice->GetDeviceCaps(FXDC_RENDER_CAPS) & FXRC_PREMULTIPLIED_ALPHA) {
+ return FXDIB_Format::kArgbPremul;
+ }
+#endif
+ return FXDIB_Format::kArgb;
+}
diff --git a/core/fpdfapi/render/cpdf_renderstatus.h b/core/fpdfapi/render/cpdf_renderstatus.h
index ef94e35..9df2711 100644
--- a/core/fpdfapi/render/cpdf_renderstatus.h
+++ b/core/fpdfapi/render/cpdf_renderstatus.h
@@ -184,6 +184,8 @@
FX_ARGB GetStrokeArgb(CPDF_PageObject* pObj) const;
FX_RECT GetObjectClippedRect(const CPDF_PageObject* pObj,
const CFX_Matrix& mtObj2Device) const;
+ // Returns the format that is compatible with `m_pDevice`.
+ FXDIB_Format GetCompatibleArgbFormat() const;
CPDF_RenderOptions m_Options;
RetainPtr<const CPDF_Dictionary> m_pFormResource;
diff --git a/core/fpdfapi/render/cpdf_rendertiling.cpp b/core/fpdfapi/render/cpdf_rendertiling.cpp
index cc18e10..8c88d32 100644
--- a/core/fpdfapi/render/cpdf_rendertiling.cpp
+++ b/core/fpdfapi/render/cpdf_rendertiling.cpp
@@ -63,11 +63,6 @@
context.AppendLayer(pPatternForm, mtPattern2Bitmap);
context.Render(&bitmap_device, nullptr, &options, nullptr);
-#if defined(PDF_USE_SKIA)
- if (CFX_DefaultRenderDevice::UseSkiaRenderer()) {
- pBitmap->UnPreMultiply();
- }
-#endif // defined(PDF_USE_SKIA)
return pBitmap;
}
diff --git a/core/fxge/cfx_renderdevice.cpp b/core/fxge/cfx_renderdevice.cpp
index a46b9ab..1f5013f 100644
--- a/core/fxge/cfx_renderdevice.cpp
+++ b/core/fxge/cfx_renderdevice.cpp
@@ -215,6 +215,8 @@
bool normalize,
int x_subpixel,
const FX_BGRA_STRUCT<uint8_t>& bgra) {
+ // TODO(crbug.com/42271020): Add support for `FXDIB_Format::kArgbPremul`.
+ CHECK(!bitmap->IsPremultiplied());
const bool has_alpha = bitmap->IsAlphaFormat();
const int Bpp = has_alpha ? 4 : bitmap->GetBPP() / 8;
for (int row = 0; row < nrows; ++row) {
@@ -467,13 +469,17 @@
return new_path_size != 0;
}
-FXDIB_Format GetCreateCompatibleBitmapFormat(int render_caps) {
+FXDIB_Format GetCreateCompatibleBitmapFormat(int render_caps,
+ bool use_argb_premul) {
if (render_caps & FXRC_BYTEMASK_OUTPUT) {
return FXDIB_Format::k8bppMask;
}
+#if defined(PDF_USE_SKIA)
+ if (use_argb_premul && (render_caps & FXRC_PREMULTIPLIED_ALPHA)) {
+ return FXDIB_Format::kArgbPremul;
+ }
+#endif
if (render_caps & FXRC_ALPHA_OUTPUT) {
- // TODO(crbug.com/42271020): Consider adding support for
- // `FXDIB_Format::kArgbPremul`
return FXDIB_Format::kArgb;
}
return CFX_DIBBase::kPlatformRGBFormat;
@@ -543,8 +549,9 @@
const RetainPtr<CFX_DIBitmap>& pDIB,
int width,
int height) const {
- return pDIB->Create(width, height,
- GetCreateCompatibleBitmapFormat(m_RenderCaps));
+ return pDIB->Create(
+ width, height,
+ GetCreateCompatibleBitmapFormat(m_RenderCaps, /*use_argb_premul=*/true));
}
void CFX_RenderDevice::SetBaseClip(const FX_RECT& rect) {
@@ -1181,8 +1188,13 @@
if (!bitmap->Create(pixel_width, pixel_height, FXDIB_Format::k8bppMask))
return false;
} else {
- if (!CreateCompatibleBitmap(bitmap, pixel_width, pixel_height))
+ // TODO(crbug.com/42271020): Switch to CreateCompatibleBitmap() once
+ // DrawNormalTextHelper() supports `FXDIB_Format::kArgbPremul`.
+ if (!bitmap->Create(pixel_width, pixel_height,
+ GetCreateCompatibleBitmapFormat(
+ m_RenderCaps, /*use_argb_premul=*/false))) {
return false;
+ }
}
if (!bitmap->IsAlphaFormat() && !bitmap->IsMaskFormat()) {
bitmap->Clear(0xFFFFFFFF);
diff --git a/core/fxge/dib/cfx_dibbase.cpp b/core/fxge/dib/cfx_dibbase.cpp
index 1e285e3..55354ab 100644
--- a/core/fxge/dib/cfx_dibbase.cpp
+++ b/core/fxge/dib/cfx_dibbase.cpp
@@ -1130,7 +1130,3 @@
#endif
}
}
-
-bool CFX_DIBBase::IsPremultiplied() const {
- return false;
-}
diff --git a/core/fxge/dib/cfx_dibbase.h b/core/fxge/dib/cfx_dibbase.h
index 6e70dd5..905d283 100644
--- a/core/fxge/dib/cfx_dibbase.h
+++ b/core/fxge/dib/cfx_dibbase.h
@@ -107,8 +107,13 @@
int& src_top,
const CFX_ClipRgn* pClipRgn) const;
- // Whether alpha is premultiplied (if `IsAlphaFormat()`).
- virtual bool IsPremultiplied() const;
+ bool IsPremultiplied() const {
+#if defined(PDF_USE_SKIA)
+ return GetFormat() == FXDIB_Format::kArgbPremul;
+#else
+ return false;
+#endif
+ }
#if defined(PDF_USE_SKIA)
// Realizes an `SkImage` from this DIB.
diff --git a/core/fxge/dib/cfx_dibitmap.cpp b/core/fxge/dib/cfx_dibitmap.cpp
index b8f9c18..918dd86 100644
--- a/core/fxge/dib/cfx_dibitmap.cpp
+++ b/core/fxge/dib/cfx_dibitmap.cpp
@@ -14,6 +14,7 @@
#include "core/fxcrt/check.h"
#include "core/fxcrt/check_op.h"
#include "core/fxcrt/compiler_specific.h"
+#include "core/fxcrt/containers/contains.h"
#include "core/fxcrt/data_vector.h"
#include "core/fxcrt/fx_coordinates.h"
#include "core/fxcrt/fx_memcpy_wrappers.h"
@@ -867,27 +868,58 @@
}
bool CFX_DIBitmap::ConvertFormat(FXDIB_Format dest_format) {
- // TODO(crbug.com/42271020): Consider adding support for
- // `FXDIB_Format::kArgbPremul`
- DCHECK(dest_format == FXDIB_Format::k8bppMask ||
- dest_format == FXDIB_Format::kArgb ||
- dest_format == FXDIB_Format::kRgb);
+ static constexpr FXDIB_Format kAllowedDestFormats[] = {
+ FXDIB_Format::k8bppMask,
+ FXDIB_Format::kArgb,
+#if defined(PDF_USE_SKIA)
+ FXDIB_Format::kArgbPremul,
+#endif
+ FXDIB_Format::kRgb,
+ };
+ CHECK(pdfium::Contains(kAllowedDestFormats, dest_format));
if (dest_format == GetFormat()) {
return true;
}
- if (dest_format == FXDIB_Format::k8bppMask &&
- GetFormat() == FXDIB_Format::k8bppRgb && !HasPalette()) {
- SetFormat(FXDIB_Format::k8bppMask);
- return true;
- }
+ switch (dest_format) {
+ case FXDIB_Format::k8bppMask:
+ if (GetFormat() == FXDIB_Format::k8bppRgb && !HasPalette()) {
+ SetFormat(FXDIB_Format::k8bppMask);
+ return true;
+ }
+ break;
- if (dest_format == FXDIB_Format::kArgb &&
- GetFormat() == FXDIB_Format::kRgb32) {
- SetFormat(FXDIB_Format::kArgb);
- SetUniformOpaqueAlpha();
- return true;
+ case FXDIB_Format::kArgb:
+ if (GetFormat() == FXDIB_Format::kRgb32) {
+ SetFormat(FXDIB_Format::kArgb);
+ SetUniformOpaqueAlpha();
+ return true;
+ }
+#if defined(PDF_USE_SKIA)
+ if (GetFormat() == FXDIB_Format::kArgbPremul) {
+ UnPreMultiply();
+ return true;
+ }
+#endif // defined(PDF_USE_SKIA)
+ break;
+
+#if defined(PDF_USE_SKIA)
+ case FXDIB_Format::kArgbPremul:
+ if (GetFormat() == FXDIB_Format::kRgb32) {
+ SetFormat(FXDIB_Format::kArgbPremul);
+ SetUniformOpaqueAlpha();
+ return true;
+ }
+ if (GetFormat() == FXDIB_Format::kArgb) {
+ PreMultiply();
+ return true;
+ }
+ break;
+#endif // defined(PDF_USE_SKIA)
+
+ default:
+ break;
}
std::optional<PitchAndSize> pitch_size =
diff --git a/core/fxge/dib/cfx_dibitmap.h b/core/fxge/dib/cfx_dibitmap.h
index 7b55f04..53e646f 100644
--- a/core/fxge/dib/cfx_dibitmap.h
+++ b/core/fxge/dib/cfx_dibitmap.h
@@ -161,22 +161,11 @@
// Converts from/to un-pre-multiplied alpha if necessary.
void PreMultiply();
void UnPreMultiply();
-
- // Forces pre-multiplied alpha without conversion.
- // TODO(crbug.com/42271020): Remove the need for this.
- void ForcePreMultiply();
-
- // CFX_DIBBase:
- bool IsPremultiplied() const override;
#endif // defined(PDF_USE_SKIA)
private:
enum class Channel : uint8_t { kRed, kAlpha };
-#if defined(PDF_USE_SKIA)
- enum class Format { kCleared, kPreMultiplied, kUnPreMultiplied };
-#endif
-
CFX_DIBitmap();
CFX_DIBitmap(const CFX_DIBitmap& src);
~CFX_DIBitmap() override;
@@ -206,9 +195,6 @@
int src_top);
MaybeOwned<uint8_t, FxFreeDeleter> m_pBuffer;
-#if defined(PDF_USE_SKIA)
- Format m_nFormat = Format::kCleared;
-#endif
};
#endif // CORE_FXGE_DIB_CFX_DIBITMAP_H_
diff --git a/core/fxge/dib/cfx_dibitmap_unittest.cpp b/core/fxge/dib/cfx_dibitmap_unittest.cpp
index b40f47c..c46110f 100644
--- a/core/fxge/dib/cfx_dibitmap_unittest.cpp
+++ b/core/fxge/dib/cfx_dibitmap_unittest.cpp
@@ -119,24 +119,9 @@
}
#if defined(PDF_USE_SKIA)
-TEST(CFXDIBitmapTest, UnPreMultiplyFromCleared) {
- auto bitmap = pdfium::MakeRetain<CFX_DIBitmap>();
- ASSERT_TRUE(bitmap->Create(1, 1, FXDIB_Format::kArgb));
- // TODO(crbug.com/42271020): This is wrong. Either IsPremultiplied() should
- // return true, or UnPreMultiply() should do nothing.
- EXPECT_FALSE(bitmap->IsPremultiplied());
- UNSAFE_TODO(FXARGB_SetDIB(bitmap->GetWritableBuffer().data(), 0x7f'7f'7f'7f));
-
- bitmap->UnPreMultiply();
- EXPECT_FALSE(bitmap->IsPremultiplied());
- EXPECT_THAT(bitmap->GetBuffer(), ElementsAre(0xff, 0xff, 0xff, 0x7f));
-}
-
TEST(CFXDIBitmapTest, UnPreMultiplyFromPreMultiplied) {
auto bitmap = pdfium::MakeRetain<CFX_DIBitmap>();
- ASSERT_TRUE(bitmap->Create(1, 1, FXDIB_Format::kArgb));
- EXPECT_FALSE(bitmap->IsPremultiplied());
- bitmap->ForcePreMultiply();
+ ASSERT_TRUE(bitmap->Create(1, 1, FXDIB_Format::kArgbPremul));
EXPECT_TRUE(bitmap->IsPremultiplied());
UNSAFE_TODO(FXARGB_SetDIB(bitmap->GetWritableBuffer().data(), 0x7f'7f'7f'7f));
@@ -149,12 +134,32 @@
auto bitmap = pdfium::MakeRetain<CFX_DIBitmap>();
ASSERT_TRUE(bitmap->Create(1, 1, FXDIB_Format::kArgb));
EXPECT_FALSE(bitmap->IsPremultiplied());
- bitmap->UnPreMultiply();
- EXPECT_FALSE(bitmap->IsPremultiplied());
UNSAFE_TODO(FXARGB_SetDIB(bitmap->GetWritableBuffer().data(), 0x7f'ff'ff'ff));
bitmap->UnPreMultiply();
EXPECT_FALSE(bitmap->IsPremultiplied());
EXPECT_THAT(bitmap->GetBuffer(), ElementsAre(0xff, 0xff, 0xff, 0x7f));
}
+
+TEST(CFXDIBitmapTest, PreMultiplyFromUnPreMultiplied) {
+ auto bitmap = pdfium::MakeRetain<CFX_DIBitmap>();
+ ASSERT_TRUE(bitmap->Create(1, 1, FXDIB_Format::kArgb));
+ EXPECT_FALSE(bitmap->IsPremultiplied());
+ UNSAFE_TODO(FXARGB_SetDIB(bitmap->GetWritableBuffer().data(), 0x7f'ff'ff'ff));
+
+ bitmap->PreMultiply();
+ EXPECT_TRUE(bitmap->IsPremultiplied());
+ EXPECT_THAT(bitmap->GetBuffer(), ElementsAre(0x7f, 0x7f, 0x7f, 0x7f));
+}
+
+TEST(CFXDIBitmapTest, PreMultiplyFromPreMultiplied) {
+ auto bitmap = pdfium::MakeRetain<CFX_DIBitmap>();
+ ASSERT_TRUE(bitmap->Create(1, 1, FXDIB_Format::kArgbPremul));
+ EXPECT_TRUE(bitmap->IsPremultiplied());
+ UNSAFE_TODO(FXARGB_SetDIB(bitmap->GetWritableBuffer().data(), 0x7f'7f'7f'7f));
+
+ bitmap->PreMultiply();
+ EXPECT_TRUE(bitmap->IsPremultiplied());
+ EXPECT_THAT(bitmap->GetBuffer(), ElementsAre(0x7f, 0x7f, 0x7f, 0x7f));
+}
#endif // defined(PDF_USE_SKIA)
diff --git a/core/fxge/render_defines.h b/core/fxge/render_defines.h
index 1b20ca0..98858de 100644
--- a/core/fxge/render_defines.h
+++ b/core/fxge/render_defines.h
@@ -26,6 +26,7 @@
#if defined(PDF_USE_SKIA)
#define FXRC_FILLSTROKE_PATH 0x80
#define FXRC_SHADING 0x100
+#define FXRC_PREMULTIPLIED_ALPHA 0x200
#endif
#endif // CORE_FXGE_RENDER_DEFINES_H_
diff --git a/core/fxge/skia/cfx_dibbase_skia.cpp b/core/fxge/skia/cfx_dibbase_skia.cpp
index 7d122c2..e1564b8 100644
--- a/core/fxge/skia/cfx_dibbase_skia.cpp
+++ b/core/fxge/skia/cfx_dibbase_skia.cpp
@@ -248,20 +248,23 @@
});
case 32:
- if (GetFormat() == FXDIB_Format::kRgb32) {
- return CreateSkiaImageFromTransformedDib</*source_bits_per_pixel=*/32>(
- *this, kBGRA_8888_SkColorType, kOpaque_SkAlphaType,
- [](uint8_t red, uint8_t green, uint8_t blue) {
- return SkPackARGB32(0xFF, red, green, blue);
- });
+ switch (GetFormat()) {
+ case FXDIB_Format::kRgb32:
+ return CreateSkiaImageFromTransformedDib<
+ /*source_bits_per_pixel=*/32>(
+ *this, kBGRA_8888_SkColorType, kOpaque_SkAlphaType,
+ [](uint8_t red, uint8_t green, uint8_t blue) {
+ return SkPackARGB32(0xFF, red, green, blue);
+ });
+ case FXDIB_Format::kArgb:
+ return CreateSkiaImageFromDib(this, kBGRA_8888_SkColorType,
+ kUnpremul_SkAlphaType);
+ case FXDIB_Format::kArgbPremul:
+ return CreateSkiaImageFromDib(this, kBGRA_8888_SkColorType,
+ kPremul_SkAlphaType);
+ default:
+ NOTREACHED_NORETURN();
}
- // TODO(crbug.com/42271020): Consider adding support for
- // `FXDIB_Format::kArgbPremul`
- CHECK_EQ(GetFormat(), FXDIB_Format::kArgb);
- return CreateSkiaImageFromDib(
- this, kBGRA_8888_SkColorType,
- IsPremultiplied() ? kPremul_SkAlphaType : kUnpremul_SkAlphaType);
-
default:
NOTREACHED_NORETURN();
}
diff --git a/core/fxge/skia/fx_skia_device.cpp b/core/fxge/skia/fx_skia_device.cpp
index a8ca2f0..b6e2671 100644
--- a/core/fxge/skia/fx_skia_device.cpp
+++ b/core/fxge/skia/fx_skia_device.cpp
@@ -701,6 +701,7 @@
m_bGroupKnockout(bGroupKnockout) {
SkColorType color_type;
const int bpp = m_pBitmap->GetBPP();
+ SkAlphaType alpha_type = kPremul_SkAlphaType;
if (bpp == 8) {
color_type = m_pBitmap->IsAlphaFormat() || m_pBitmap->IsMaskFormat()
? kAlpha_8_SkColorType
@@ -715,9 +716,7 @@
const int height = m_pOriginalBitmap->GetHeight();
m_pBitmap = pdfium::MakeRetain<CFX_DIBitmap>();
- // TODO(crbug.com/42271020): Consider adding support for
- // `FXDIB_Format::kArgbPremul`
- if (!m_pBitmap->Create(width, height, FXDIB_Format::kArgb) ||
+ if (!m_pBitmap->Create(width, height, FXDIB_Format::kArgbPremul) ||
!m_pBitmap->TransferBitmap(width, height, m_pOriginalBitmap,
/*src_left=*/0, /*src_top=*/0)) {
// Skip creating SkCanvas if the 32-bpp bitmap creation fails.
@@ -733,11 +732,16 @@
} else {
DCHECK_EQ(bpp, 32);
color_type = Get32BitSkColorType(bRgbByteOrder);
+ FXDIB_Format format = m_pBitmap->GetFormat();
+ if (format == FXDIB_Format::kRgb32) {
+ alpha_type = kOpaque_SkAlphaType;
+ } else if (format == FXDIB_Format::kArgb) {
+ alpha_type = kUnpremul_SkAlphaType;
+ }
}
- SkImageInfo imageInfo =
- SkImageInfo::Make(m_pBitmap->GetWidth(), m_pBitmap->GetHeight(),
- color_type, kPremul_SkAlphaType);
+ SkImageInfo imageInfo = SkImageInfo::Make(
+ m_pBitmap->GetWidth(), m_pBitmap->GetHeight(), color_type, alpha_type);
surface_ = SkSurfaces::WrapPixels(
imageInfo, m_pBitmap->GetWritableBuffer().data(), m_pBitmap->GetPitch());
m_pCanvas = surface_->getCanvas();
@@ -1013,7 +1017,7 @@
case FXDC_RENDER_CAPS:
return FXRC_GET_BITS | FXRC_ALPHA_PATH | FXRC_ALPHA_IMAGE |
FXRC_BLEND_MODE | FXRC_SOFT_CLIP | FXRC_ALPHA_OUTPUT |
- FXRC_FILLSTROKE_PATH | FXRC_SHADING;
+ FXRC_FILLSTROKE_PATH | FXRC_SHADING | FXRC_PREMULTIPLIED_ALPHA;
default:
NOTREACHED_NORETURN();
}
@@ -1374,16 +1378,16 @@
uint8_t* output_buffer = bitmap->GetWritableBuffer().data();
DCHECK(output_buffer);
- SkImageInfo input_info =
- SkImageInfo::Make(m_pBitmap->GetWidth(), m_pBitmap->GetHeight(),
- SkColorType::kN32_SkColorType, kPremul_SkAlphaType);
+ SkImageInfo input_info = m_pCanvas->imageInfo();
sk_sp<SkImage> input = SkImages::RasterFromPixmap(
SkPixmap(input_info, input_buffer, m_pBitmap->GetPitch()),
/*rasterReleaseProc=*/nullptr, /*releaseContext=*/nullptr);
+ CHECK_EQ(32, bitmap->GetBPP());
SkImageInfo output_info = SkImageInfo::Make(
bitmap->GetWidth(), bitmap->GetHeight(),
- Get32BitSkColorType(m_bRgbByteOrder), kPremul_SkAlphaType);
+ Get32BitSkColorType(m_bRgbByteOrder),
+ bitmap->IsPremultiplied() ? kPremul_SkAlphaType : kUnpremul_SkAlphaType);
sk_sp<SkSurface> output =
SkSurfaces::WrapPixels(output_info, output_buffer, bitmap->GetPitch());
@@ -1458,11 +1462,8 @@
}
void CFX_DIBitmap::PreMultiply() {
- // TODO(crbug.com/42271020): Implement.
-}
-
-void CFX_DIBitmap::UnPreMultiply() {
- if (m_nFormat == Format::kUnPreMultiplied || GetBPP() != 32) {
+ CHECK(CFX_DefaultRenderDevice::UseSkiaRenderer());
+ if (GetFormat() != FXDIB_Format::kArgb) {
return;
}
@@ -1471,7 +1472,31 @@
return;
}
- m_nFormat = Format::kUnPreMultiplied;
+ SetFormat(FXDIB_Format::kArgbPremul);
+ int height = GetHeight();
+ int width = GetWidth();
+ int row_bytes = GetPitch();
+ SkImageInfo premultiplied_info =
+ SkImageInfo::Make(width, height, kN32_SkColorType, kPremul_SkAlphaType);
+ SkPixmap premultiplied(premultiplied_info, buffer, row_bytes);
+ SkImageInfo unpremultiplied_info =
+ SkImageInfo::Make(width, height, kN32_SkColorType, kUnpremul_SkAlphaType);
+ SkPixmap unpremultiplied(unpremultiplied_info, buffer, row_bytes);
+ unpremultiplied.readPixels(premultiplied);
+}
+
+void CFX_DIBitmap::UnPreMultiply() {
+ CHECK(CFX_DefaultRenderDevice::UseSkiaRenderer());
+ if (GetFormat() != FXDIB_Format::kArgbPremul) {
+ return;
+ }
+
+ void* buffer = GetWritableBuffer().data();
+ if (!buffer) {
+ return;
+ }
+
+ SetFormat(FXDIB_Format::kArgb);
int height = GetHeight();
int width = GetWidth();
int row_bytes = GetPitch();
@@ -1484,14 +1509,6 @@
premultiplied.readPixels(unpremultiplied);
}
-void CFX_DIBitmap::ForcePreMultiply() {
- m_nFormat = Format::kPreMultiplied;
-}
-
-bool CFX_DIBitmap::IsPremultiplied() const {
- return m_nFormat == Format::kPreMultiplied;
-}
-
bool CFX_SkiaDeviceDriver::DrawBitsWithMask(RetainPtr<const CFX_DIBBase> bitmap,
RetainPtr<const CFX_DIBBase> mask,
float alpha,
diff --git a/fpdfsdk/fpdf_edit_embeddertest.cpp b/fpdfsdk/fpdf_edit_embeddertest.cpp
index 0b1d327..2aac797 100644
--- a/fpdfsdk/fpdf_edit_embeddertest.cpp
+++ b/fpdfsdk/fpdf_edit_embeddertest.cpp
@@ -4635,13 +4635,13 @@
const char* smask_checksum = []() {
if (CFX_DefaultRenderDevice::UseSkiaRenderer()) {
- return "0653a18f3bf9b4d8413a2aa10bc11c38";
+ return "a85ca0183ac6aee8851c30c5bdc2f594";
}
return "5a3ae4a660ce919e29c42ec2258142f1";
}();
const char* no_smask_checksum = []() {
if (CFX_DefaultRenderDevice::UseSkiaRenderer()) {
- return "0da49e63e7d6337aca78b19938e3bf65";
+ return "712f832dcbfb6cefc74f39bef459bea4";
}
return "67504e83f5d78214ea00efc19082c5c1";
}();
@@ -4975,7 +4975,7 @@
const char* checksum = []() {
if (CFX_DefaultRenderDevice::UseSkiaRenderer()) {
#if BUILDFLAG(IS_WIN)
- return "5cef5b3e56e91e1a66b6780fb26bb5e3";
+ return "764e3503960ef0b176796faa3543b9c7";
#elif BUILDFLAG(IS_APPLE)
return "9e7774173acee966fcaa72e599eb9a93";
#else
@@ -5031,7 +5031,7 @@
const char* checksum = []() {
if (CFX_DefaultRenderDevice::UseSkiaRenderer()) {
#if BUILDFLAG(IS_WIN)
- return "336be21110c795cefcab9bbdbc3afcdd";
+ return "4cdba7492317bcae2643bd4090e18812";
#elif BUILDFLAG(IS_APPLE)
return "0b9efedcb8f5aa9246c52e90811cb046";
#else
@@ -5155,7 +5155,7 @@
ASSERT_TRUE(bitmap);
const char* checksum = []() {
if (CFX_DefaultRenderDevice::UseSkiaRenderer()) {
- return "1d74731d23a056c0e3fb88f2f85b2581";
+ return "2a45a0034911f21b528621bebe0aab4b";
}
return "e8154fa8ededf4d9b8b35b5260897b6c";
}();
@@ -5181,7 +5181,7 @@
const char* checksum = []() {
if (CFX_DefaultRenderDevice::UseSkiaRenderer()) {
#if BUILDFLAG(IS_WIN)
- return "ef501232372617a545ae35d7664fd9ec";
+ return "6d88537a49fa2dccfa0f58ac325c5b75";
#elif BUILDFLAG(IS_APPLE)
return "a637d62f2e8ae10c3267b2ff5fcc2246";
#else
diff --git a/fpdfsdk/fpdf_editimg.cpp b/fpdfsdk/fpdf_editimg.cpp
index f29bc4a..1eb2b23 100644
--- a/fpdfsdk/fpdf_editimg.cpp
+++ b/fpdfsdk/fpdf_editimg.cpp
@@ -338,11 +338,7 @@
if (!renderer.GetResult())
return nullptr;
-#if defined(PDF_USE_SKIA)
- if (CFX_DefaultRenderDevice::UseSkiaRenderer()) {
- result_bitmap->UnPreMultiply();
- }
-#endif
+ CHECK(!result_bitmap->IsPremultiplied());
// Caller takes ownership.
return FPDFBitmapFromCFXDIBitmap(result_bitmap.Leak());
diff --git a/fpdfsdk/fpdf_formfill.cpp b/fpdfsdk/fpdf_formfill.cpp
index 19a79ba..8e794a5 100644
--- a/fpdfsdk/fpdf_formfill.cpp
+++ b/fpdfsdk/fpdf_formfill.cpp
@@ -211,7 +211,6 @@
if (dest_is_bitmap) {
holder.Reset(absl::get<CFX_DIBitmap*>(dest));
CHECK(holder);
- CHECK(!holder->IsPremultiplied());
} else {
#if defined(PDF_USE_SKIA)
if (!CFX_DefaultRenderDevice::UseSkiaRenderer()) {
@@ -733,6 +732,10 @@
return;
}
+#if defined(PDF_USE_SKIA)
+ CFX_DIBitmap::ScopedPremultiplier scoped_premultiplier(
+ pdfium::WrapRetain(cbitmap), CFX_DefaultRenderDevice::UseSkiaRenderer());
+#endif
FFLCommon(hHandle, page, cbitmap, start_x, start_y, size_x, size_y, rotate,
flags);
}
diff --git a/fpdfsdk/fpdf_progressive.cpp b/fpdfsdk/fpdf_progressive.cpp
index 455d42d..150b46a 100644
--- a/fpdfsdk/fpdf_progressive.cpp
+++ b/fpdfsdk/fpdf_progressive.cpp
@@ -68,6 +68,12 @@
CPDF_PageRenderContext* context = owned_context.get();
pPage->SetRenderContext(std::move(owned_context));
+#if defined(PDF_USE_SKIA)
+ if (CFX_DefaultRenderDevice::UseSkiaRenderer()) {
+ pBitmap->PreMultiply();
+ }
+#endif
+
auto device = std::make_unique<CFX_DefaultRenderDevice>();
device->AttachWithRgbByteOrder(pBitmap, !!(flags & FPDF_REVERSE_BYTE_ORDER));
context->m_pDevice = std::move(device);
@@ -77,17 +83,37 @@
size_y, rotate, flags, color_scheme,
/*need_to_restore=*/false, &pause_adapter);
+ if (!context->m_pRenderer) {
+#if defined(PDF_USE_SKIA)
+ if (CFX_DefaultRenderDevice::UseSkiaRenderer()) {
+ pBitmap->UnPreMultiply();
+ }
+#endif // defined(PDF_USE_SKIA)
+
+ return FPDF_RENDER_FAILED;
+ }
+
+ int status = ToFPDFStatus(context->m_pRenderer->GetStatus());
+ if (status == FPDF_RENDER_TOBECONTINUED) {
+ // Note that `pBitmap` is still pre-multiplied here, as the caller is
+ // expected to pass it to FPDF_RenderPage_Continue(). Then
+ // FPDF_RenderPage_Continue() can continue rendering into it without doing
+ // another round of (un)pre-multiplication. FPDF_RenderPage_Continue() will
+ // call UnPreMultiply() when done.
+ //
+ // Normally, PDFium would not return a pre-multiplied bitmap to the caller,
+ // but in this case, the bitmap is in an indeterminate state while it is
+ // being progressively rendered. So many an exception here, as it can
+ // greatly improve performance.
+ return FPDF_RENDER_TOBECONTINUED;
+ }
+
#if defined(PDF_USE_SKIA)
if (CFX_DefaultRenderDevice::UseSkiaRenderer()) {
pBitmap->UnPreMultiply();
}
#endif // defined(PDF_USE_SKIA)
-
- if (!context->m_pRenderer) {
- return FPDF_RENDER_FAILED;
- }
-
- return ToFPDFStatus(context->m_pRenderer->GetStatus());
+ return status;
}
FPDF_EXPORT int FPDF_CALLCONV FPDF_RenderPageBitmap_Start(FPDF_BITMAP bitmap,
@@ -121,12 +147,17 @@
CPDFSDK_PauseAdapter pause_adapter(pause);
pContext->m_pRenderer->Continue(&pause_adapter);
+ int status = ToFPDFStatus(pContext->m_pRenderer->GetStatus());
+ if (status == FPDF_RENDER_TOBECONTINUED) {
+ return FPDF_RENDER_TOBECONTINUED;
+ }
+
#if defined(PDF_USE_SKIA)
if (CFX_DefaultRenderDevice::UseSkiaRenderer()) {
pContext->m_pDevice->GetBitmap()->UnPreMultiply();
}
#endif // defined(PDF_USE_SKIA)
- return ToFPDFStatus(pContext->m_pRenderer->GetStatus());
+ return status;
}
FPDF_EXPORT void FPDF_CALLCONV FPDF_RenderPage_Close(FPDF_PAGE page) {
diff --git a/fpdfsdk/fpdf_view.cpp b/fpdfsdk/fpdf_view.cpp
index 458b604..2fb04a7 100644
--- a/fpdfsdk/fpdf_view.cpp
+++ b/fpdfsdk/fpdf_view.cpp
@@ -629,12 +629,6 @@
/*need_to_restore=*/true,
/*pause=*/nullptr);
-#if defined(PDF_USE_SKIA)
- if (CFX_DefaultRenderDevice::UseSkiaRenderer()) {
- pBitmap->UnPreMultiply();
- }
-#endif
-
if (!bHasMask) {
CPDF_WindowsRenderDevice win_dc(dc, render_data->GetPSFontTracker());
bool bitsStretched = false;
@@ -719,20 +713,19 @@
CPDF_Page::RenderContextClearer clearer(pPage);
pPage->SetRenderContext(std::move(owned_context));
+#if defined(PDF_USE_SKIA)
+ CFX_DIBitmap::ScopedPremultiplier scoped_premultiplier(
+ pBitmap, CFX_DefaultRenderDevice::UseSkiaRenderer());
+#endif
auto device = std::make_unique<CFX_DefaultRenderDevice>();
- device->AttachWithRgbByteOrder(pBitmap, !!(flags & FPDF_REVERSE_BYTE_ORDER));
+ device->AttachWithRgbByteOrder(std::move(pBitmap),
+ !!(flags & FPDF_REVERSE_BYTE_ORDER));
context->m_pDevice = std::move(device);
CPDFSDK_RenderPageWithContext(context, pPage, start_x, start_y, size_x,
size_y, rotate, flags, /*color_scheme=*/nullptr,
/*need_to_restore=*/true,
/*pause=*/nullptr);
-
-#if defined(PDF_USE_SKIA)
- if (CFX_DefaultRenderDevice::UseSkiaRenderer()) {
- pBitmap->UnPreMultiply();
- }
-#endif
}
FPDF_EXPORT void FPDF_CALLCONV
@@ -757,6 +750,10 @@
CPDF_Page::RenderContextClearer clearer(pPage);
pPage->SetRenderContext(std::move(owned_context));
+#if defined(PDF_USE_SKIA)
+ CFX_DIBitmap::ScopedPremultiplier scoped_premultiplier(
+ pBitmap, CFX_DefaultRenderDevice::UseSkiaRenderer());
+#endif
auto device = std::make_unique<CFX_DefaultRenderDevice>();
device->AttachWithRgbByteOrder(std::move(pBitmap),
!!(flags & FPDF_REVERSE_BYTE_ORDER));
diff --git a/testing/SUPPRESSIONS_EXACT_MATCHING b/testing/SUPPRESSIONS_EXACT_MATCHING
index b9734d9..494d784 100644
--- a/testing/SUPPRESSIONS_EXACT_MATCHING
+++ b/testing/SUPPRESSIONS_EXACT_MATCHING
@@ -20,9 +20,20 @@
# Try to keep the file alphabetized within each category of test.
#
+# Pixel tests
+#
+
+# Sensitive to transparency code changes.
+bug_1258968.in * * * skia
+
+#
# Corpus tests
#
+# Sensitive to transparency code changes.
+1_10_watermark.pdf * * * skia
+
# Device-specific ColorBurn differences (see pdfium:1959).
+xfermodes.pdf * * * skia
xfermodes2.pdf * * * skia
xfermodes3.pdf * * * skia
diff --git a/testing/resources/pixel/bug_1161_expected_skia.pdf.0.png b/testing/resources/pixel/bug_1161_expected_skia.pdf.0.png
index 4197dcb..e83cf25 100644
--- a/testing/resources/pixel/bug_1161_expected_skia.pdf.0.png
+++ b/testing/resources/pixel/bug_1161_expected_skia.pdf.0.png
Binary files differ
diff --git a/xfa/fde/cfde_textout_unittest.cpp b/xfa/fde/cfde_textout_unittest.cpp
index 85c24f0..79536cf 100644
--- a/xfa/fde/cfde_textout_unittest.cpp
+++ b/xfa/fde/cfde_textout_unittest.cpp
@@ -97,7 +97,7 @@
const char* checksum = []() {
#if BUILDFLAG(IS_WIN)
if (CFX_DefaultRenderDevice::UseSkiaRenderer()) {
- return "cdd8f00144e421bf18f22d09896838b0";
+ return "bc1f736237b08d13db06c09f6becc9f7";
}
#endif
return "b26f1c171fcdbf185823364185adacf0";
@@ -134,7 +134,7 @@
const char* GetLargeTextBlobChecksum() {
if (CFX_DefaultRenderDevice::UseSkiaRenderer()) {
- return "cd357c6afbf17bb2ac48817df5d9eaad";
+ return "6181929583fd7651169306852397806f";
}
return "268b71a8660b51e31c6bf30fc7ff1e08";
}
