Remove UNSAFE_TODO() from CFX_DIBitmap::Clear()
Switch to spans. Then it is not so hard to fill every scanline if
needed. Instead of filling the first scanline and copying it to the
other scanlines.
To make the changes build, change fxcrt::Fill() to take a rvalue.
Otherwise fxcrt::Fill(GetWritableSpan(), val) does not compile.
To make it slightly easier to manage color component variables, add
ArgbToBGRStruct() and use that in place of ArgbDecode().
Bug: 42271176
Change-Id: Idf7a682bbc82d045922a23b2c2feb3090a58a630
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/121450
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@google.com>
diff --git a/core/fxcrt/stl_util.h b/core/fxcrt/stl_util.h
index ced0bb1..3b6e7e5 100644
--- a/core/fxcrt/stl_util.h
+++ b/core/fxcrt/stl_util.h
@@ -47,7 +47,7 @@
// Equivalent of C++20 std::ranges::fill().
template <typename T, typename V>
-void Fill(T& container, const V& value) {
+void Fill(T&& container, const V& value) {
std::fill(std::begin(container), std::end(container), value);
}
diff --git a/core/fxge/dib/cfx_dibitmap.cpp b/core/fxge/dib/cfx_dibitmap.cpp
index 2da6974..563bb36 100644
--- a/core/fxge/dib/cfx_dibitmap.cpp
+++ b/core/fxge/dib/cfx_dibitmap.cpp
@@ -22,6 +22,7 @@
#include "core/fxcrt/numerics/safe_conversions.h"
#include "core/fxcrt/span.h"
#include "core/fxcrt/span_util.h"
+#include "core/fxcrt/stl_util.h"
#include "core/fxge/calculate_pitch.h"
#include "core/fxge/cfx_cliprgn.h"
#include "core/fxge/cfx_defaultrenderdevice.h"
@@ -131,70 +132,50 @@
}
void CFX_DIBitmap::Clear(uint32_t color) {
- if (!m_pBuffer)
+ auto buffer = GetWritableBuffer();
+ if (buffer.empty()) {
return;
+ }
- uint8_t* pBuffer = m_pBuffer.Get();
- UNSAFE_TODO({
- switch (GetFormat()) {
- case FXDIB_Format::kInvalid:
- break;
- case FXDIB_Format::k1bppMask:
- FXSYS_memset(pBuffer, (color & 0xff000000) ? 0xff : 0,
- m_Pitch * m_Height);
- break;
- case FXDIB_Format::k1bppRgb: {
- int index = FindPalette(color);
- FXSYS_memset(pBuffer, index ? 0xff : 0, m_Pitch * m_Height);
- break;
- }
- case FXDIB_Format::k8bppMask:
- FXSYS_memset(pBuffer, color >> 24, m_Pitch * m_Height);
- break;
- case FXDIB_Format::k8bppRgb: {
- int index = FindPalette(color);
- FXSYS_memset(pBuffer, index, m_Pitch * m_Height);
- break;
- }
- case FXDIB_Format::kRgb: {
- int a;
- int r;
- int g;
- int b;
- std::tie(a, r, g, b) = ArgbDecode(color);
- if (r == g && g == b) {
- FXSYS_memset(pBuffer, r, m_Pitch * m_Height);
- } else {
- int byte_pos = 0;
- for (int col = 0; col < m_Width; col++) {
- pBuffer[byte_pos++] = b;
- pBuffer[byte_pos++] = g;
- pBuffer[byte_pos++] = r;
- }
- for (int row = 1; row < m_Height; row++) {
- FXSYS_memcpy(pBuffer + row * m_Pitch, pBuffer, m_Pitch);
- }
+ switch (GetFormat()) {
+ case FXDIB_Format::kInvalid:
+ break;
+ case FXDIB_Format::k1bppMask:
+ fxcrt::Fill(buffer, (color & 0xff000000) ? 0xff : 0);
+ break;
+ case FXDIB_Format::k1bppRgb:
+ fxcrt::Fill(buffer, FindPalette(color) ? 0xff : 0);
+ break;
+ case FXDIB_Format::k8bppMask:
+ fxcrt::Fill(buffer, color >> 24);
+ break;
+ case FXDIB_Format::k8bppRgb:
+ fxcrt::Fill(buffer, FindPalette(color));
+ break;
+ case FXDIB_Format::kRgb: {
+ const FX_BGR_STRUCT<uint8_t> bgr = ArgbToBGRStruct(color);
+ if (bgr.red == bgr.green && bgr.green == bgr.blue) {
+ fxcrt::Fill(buffer, bgr.red);
+ } else {
+ for (int row = 0; row < m_Height; row++) {
+ fxcrt::Fill(GetWritableScanlineAs<FX_BGR_STRUCT<uint8_t>>(row), bgr);
}
- break;
}
- case FXDIB_Format::kRgb32:
- case FXDIB_Format::kArgb: {
- if (CFX_DefaultRenderDevice::UseSkiaRenderer() &&
- FXDIB_Format::kRgb32 == GetFormat()) {
- // TODO(crbug.com/pdfium/2016): This is not reliable because alpha may
- // be modified outside of this operation.
- color |= 0xFF000000;
- }
- for (int i = 0; i < m_Width; i++) {
- reinterpret_cast<uint32_t*>(pBuffer)[i] = color;
- }
- for (int row = 1; row < m_Height; row++) {
- FXSYS_memcpy(pBuffer + row * m_Pitch, pBuffer, m_Pitch);
- }
- break;
- }
+ break;
}
- });
+ case FXDIB_Format::kRgb32:
+ if (CFX_DefaultRenderDevice::UseSkiaRenderer()) {
+ // TODO(crbug.com/pdfium/2016): This is not reliable because alpha may
+ // be modified outside of this operation.
+ color |= 0xFF000000;
+ }
+ [[fallthrough]];
+ case FXDIB_Format::kArgb:
+ for (int row = 0; row < m_Height; row++) {
+ fxcrt::Fill(GetWritableScanlineAs<uint32_t>(row), color);
+ }
+ break;
+ }
}
bool CFX_DIBitmap::TransferBitmap(int dest_left,
diff --git a/core/fxge/dib/fx_dib.cpp b/core/fxge/dib/fx_dib.cpp
index b4867d6..1cc5e85 100644
--- a/core/fxge/dib/fx_dib.cpp
+++ b/core/fxge/dib/fx_dib.cpp
@@ -60,6 +60,10 @@
return bInterpolateBilinear || bHalftone || bNoSmoothing || bLossy;
}
+FX_BGR_STRUCT<uint8_t> ArgbToBGRStruct(FX_ARGB argb) {
+ return {FXARGB_B(argb), FXARGB_G(argb), FXARGB_R(argb)};
+}
+
std::tuple<int, int, int, int> ArgbDecode(FX_ARGB argb) {
return std::make_tuple(FXARGB_A(argb), FXARGB_R(argb), FXARGB_G(argb),
FXARGB_B(argb));
diff --git a/core/fxge/dib/fx_dib.h b/core/fxge/dib/fx_dib.h
index 758210b..83da668 100644
--- a/core/fxge/dib/fx_dib.h
+++ b/core/fxge/dib/fx_dib.h
@@ -162,6 +162,9 @@
FXDIB_Format MakeRGBFormat(int bpp);
+// Ignores alpha.
+FX_BGR_STRUCT<uint8_t> ArgbToBGRStruct(FX_ARGB argb);
+
// Returns (a, r, g, b)
std::tuple<int, int, int, int> ArgbDecode(FX_ARGB argb);
