[Skia] Remove DCHECKs on RGB channels during upsampling
For a bitmap, if it's 1 bit-per-pixel and it has a color palette, the
palette will contain 2 colors. If the R,G,B channels of either color
have the same values, we can tell those two colors are grayscale and
they will be the only 2 colors used for rendering that bitmap.
However, the R,G,B channel of either color are not always guaranteed
to be equal. In this case, the two colors represents a range of colors
to be used for rendering the bitmap.
This CL removes the incorrect DCHECKs which enforce R,G,B channels to
be equal and adds the detection on when we should use a range of
colors instead of only 2 colors for bitmap rendering.
Meanwhile, rename `color1` and`color2` to `color0` and `color1` to
match the index for CFX_DIBBase::GetPaletteArgb() calls.
Bug: pdfium:1810
Change-Id: Idd83e68a2a654fa5db9c6552e31b42f9b9482acf
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/98174
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Nigi <nigi@chromium.org>
diff --git a/core/fxge/skia/fx_skia_device.cpp b/core/fxge/skia/fx_skia_device.cpp
index 5a7a047..c4040ae 100644
--- a/core/fxge/skia/fx_skia_device.cpp
+++ b/core/fxge/skia/fx_skia_device.cpp
@@ -287,6 +287,11 @@
#endif // DRAW_SKIA_CLIP
#if defined(_SKIA_SUPPORT_)
+bool IsRGBColorGrayScale(uint32_t color) {
+ return FXARGB_R(color) == FXARGB_G(color) &&
+ FXARGB_R(color) == FXARGB_B(color);
+}
+
static void DebugValidate(const RetainPtr<CFX_DIBitmap>& bitmap,
const RetainPtr<CFX_DIBitmap>& device) {
if (bitmap) {
@@ -680,31 +685,45 @@
int rowBytes = pSource->GetPitch();
switch (pSource->GetBPP()) {
case 1: {
- dst8_storage = DataVector<uint8_t>(Fx2DSizeOrDie(width, height));
- pdfium::span<uint8_t> dst8_pixels(dst8_storage);
// By default, the two colors for grayscale are 0xFF and 0x00 unless they
// are specified in the palette.
- uint8_t color1 = 0x00;
- uint8_t color2 = 0xFF;
+ uint8_t color0 = 0x00;
+ uint8_t color1 = 0xFF;
+ bool use_two_colors = true;
if (pSource->GetFormat() == FXDIB_Format::k1bppRgb &&
pSource->HasPalette()) {
- color1 = FXARGB_R(pSource->GetPaletteArgb(0));
- color2 = FXARGB_R(pSource->GetPaletteArgb(1));
- DCHECK_EQ(color1, FXARGB_G(pSource->GetPaletteArgb(0)));
- DCHECK_EQ(color1, FXARGB_B(pSource->GetPaletteArgb(0)));
- DCHECK_EQ(color2, FXARGB_G(pSource->GetPaletteArgb(1)));
- DCHECK_EQ(color2, FXARGB_B(pSource->GetPaletteArgb(1)));
+ // When `pSource` has 1 bit per pixel, its palette will contain 2
+ // colors, and the R,G,B channels of either color will have equal
+ // values. However, there are cases that R,G,B are not all the same and
+ // these 2 colors stand for the boundaries of a range of colors to be
+ // used for bitmap rendering. We need to handle the color palette
+ // differently depending on these conditions.
+ uint32_t palette_color0 = pSource->GetPaletteArgb(0);
+ uint32_t palette_color1 = pSource->GetPaletteArgb(1);
+ use_two_colors = IsRGBColorGrayScale(palette_color0) &&
+ IsRGBColorGrayScale(palette_color1);
+ if (use_two_colors) {
+ color0 = FXARGB_R(palette_color0);
+ color1 = FXARGB_R(palette_color1);
+ }
+
+ // TODO(pdfium:1810): Handle upsampling when a range of colors (more
+ // than 2 colors) are provided in the source palette.
}
- for (int y = 0; y < height; ++y) {
- const uint8_t* srcRow =
- static_cast<const uint8_t*>(buffer) + y * rowBytes;
- pdfium::span<uint8_t> dst_row = dst8_pixels.subspan(y * width);
- for (int x = 0; x < width; ++x)
- dst_row[x] = srcRow[x >> 3] & (1 << (~x & 0x07)) ? color2 : color1;
+ if (use_two_colors) {
+ dst8_storage = DataVector<uint8_t>(Fx2DSizeOrDie(width, height));
+ pdfium::span<uint8_t> dst8_pixels(dst8_storage);
+ for (int y = 0; y < height; ++y) {
+ const uint8_t* srcRow =
+ static_cast<const uint8_t*>(buffer) + y * rowBytes;
+ pdfium::span<uint8_t> dst_row = dst8_pixels.subspan(y * width);
+ for (int x = 0; x < width; ++x)
+ dst_row[x] = srcRow[x >> 3] & (1 << (~x & 0x07)) ? color1 : color0;
+ }
+ buffer = dst8_storage.data();
+ rowBytes = width;
}
- buffer = dst8_storage.data();
- rowBytes = width;
break;
}
case 8:
