From 31b08d4cdaa17d7a03f35e087096a77036af98ec Mon Sep 17 00:00:00 2001 From: dsinclair Date: Tue, 28 Mar 2017 15:47:47 +0000 Subject: Revert "Refcount all CFX_DIBSources (and subclasses) all the time." This reverts commit 0004f29bf6ee3c6060a272c79f14993e92e053c7. Reason for revert: Breaks build with skia_paths enabled (which will break the chrome roll). ../../third_party/pdfium/core/fxge/skia/fx_skia_device.cpp:1858:38: error: no member named 'get' in 'CFX_RetainPtr' ../../third_party/pdfium/core/fxge/skia/fx_skia_device.cpp:1861:42: error: no member named 'get' in 'CFX_RetainPtr' ../../third_party/pdfium/core/fxge/skia/fx_skia_device.cpp:2987:15: error: no viable overloaded '=' ../../third_party/pdfium/core/fxge/skia/fx_skia_device.cpp:2991:18: error: no viable overloaded '=' ../../third_party/pdfium/core/fxge/skia/fx_skia_device.cpp:2999:17: error: no viable overloaded '=' ../../third_party/pdfium/core/fxge/skia/fx_skia_device.cpp:3001:43: error: no member named 'GetObject' in 'CFX_RetainPtr' Original change's description: > Refcount all CFX_DIBSources (and subclasses) all the time. > > There are currently several ownership models for these objects, > including ad-hoc logic for sharing and deletion, and the > now-redundant CFX_DIBitmapRef externally-counted handle to the DIBs. > > Replace them all with the internal refcount scheme. > > Change-Id: I2db399dfc19219eda384f94cc989353b78ce2872 > Reviewed-on: https://pdfium-review.googlesource.com/3166 > Reviewed-by: dsinclair > Commit-Queue: dsinclair > TBR=thestig@chromium.org,tsepez@chromium.org,dsinclair@chromium.org,pdfium-reviews@googlegroups.com NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true Change-Id: I678b1fbc5e666cf7a19372ebaff3270fb115ba5e Reviewed-on: https://pdfium-review.googlesource.com/3243 Reviewed-by: dsinclair Commit-Queue: dsinclair --- core/fxge/dib/fx_dib_main.cpp | 170 +++++++++++++++++++++++------------------- 1 file changed, 95 insertions(+), 75 deletions(-) (limited to 'core/fxge/dib/fx_dib_main.cpp') diff --git a/core/fxge/dib/fx_dib_main.cpp b/core/fxge/dib/fx_dib_main.cpp index 8a97d342b5..96cae9d4e7 100644 --- a/core/fxge/dib/fx_dib_main.cpp +++ b/core/fxge/dib/fx_dib_main.cpp @@ -44,9 +44,16 @@ uint32_t ArgbEncode(int a, FX_COLORREF rgb) { } CFX_DIBSource::CFX_DIBSource() - : m_Width(0), m_Height(0), m_bpp(0), m_AlphaFlag(0), m_Pitch(0) {} - -CFX_DIBSource::~CFX_DIBSource() {} + : m_pAlphaMask(nullptr), + m_Width(0), + m_Height(0), + m_bpp(0), + m_AlphaFlag(0), + m_Pitch(0) {} + +CFX_DIBSource::~CFX_DIBSource() { + delete m_pAlphaMask; +} uint8_t* CFX_DIBSource::GetBuffer() const { return nullptr; @@ -120,7 +127,7 @@ bool CFX_DIBitmap::Create(int width, return true; } -bool CFX_DIBitmap::Copy(const CFX_RetainPtr& pSrc) { +bool CFX_DIBitmap::Copy(const CFX_DIBSource* pSrc) { if (m_pBuffer) return false; @@ -150,10 +157,11 @@ const uint8_t* CFX_DIBitmap::GetScanline(int line) const { return m_pBuffer ? m_pBuffer + line * m_Pitch : nullptr; } -void CFX_DIBitmap::TakeOver(CFX_RetainPtr&& pSrcBitmap) { +void CFX_DIBitmap::TakeOver(CFX_DIBitmap* pSrcBitmap) { if (!m_bExtBuf) FX_Free(m_pBuffer); + delete m_pAlphaMask; m_pBuffer = pSrcBitmap->m_pBuffer; m_pPalette = std::move(pSrcBitmap->m_pPalette); m_pAlphaMask = pSrcBitmap->m_pAlphaMask; @@ -167,14 +175,14 @@ void CFX_DIBitmap::TakeOver(CFX_RetainPtr&& pSrcBitmap) { m_Pitch = pSrcBitmap->m_Pitch; } -CFX_RetainPtr CFX_DIBSource::Clone(const FX_RECT* pClip) const { +std::unique_ptr CFX_DIBSource::Clone(const FX_RECT* pClip) const { FX_RECT rect(0, 0, m_Width, m_Height); if (pClip) { rect.Intersect(*pClip); if (rect.IsEmpty()) return nullptr; } - auto pNewBitmap = pdfium::MakeRetain(); + auto pNewBitmap = pdfium::MakeUnique(); if (!pNewBitmap->Create(rect.Width(), rect.Height(), GetFormat())) return nullptr; @@ -232,11 +240,12 @@ void CFX_DIBSource::BuildPalette() { } bool CFX_DIBSource::BuildAlphaMask() { - if (m_pAlphaMask) + if (m_pAlphaMask) { return true; - - m_pAlphaMask = pdfium::MakeRetain(); + } + m_pAlphaMask = new CFX_DIBitmap; if (!m_pAlphaMask->Create(m_Width, m_Height, FXDIB_8bppMask)) { + delete m_pAlphaMask; m_pAlphaMask = nullptr; return false; } @@ -392,14 +401,13 @@ void CFX_DIBSource::GetOverlapRect(int& dest_left, height = dest_rect.bottom - dest_rect.top; } -bool CFX_DIBitmap::TransferBitmap( - int dest_left, - int dest_top, - int width, - int height, - const CFX_RetainPtr& pSrcBitmap, - int src_left, - int src_top) { +bool CFX_DIBitmap::TransferBitmap(int dest_left, + int dest_top, + int width, + int height, + const CFX_DIBSource* pSrcBitmap, + int src_left, + int src_top) { if (!m_pBuffer) return false; @@ -458,7 +466,7 @@ bool CFX_DIBitmap::TransferMask(int dest_left, int dest_top, int width, int height, - const CFX_RetainPtr& pMask, + const CFX_DIBSource* pMask, uint32_t color, int src_left, int src_top, @@ -597,7 +605,7 @@ void CFX_DIBSource::GetPalette(uint32_t* pal, int alpha) const { } } -CFX_RetainPtr CFX_DIBSource::CloneAlphaMask( +std::unique_ptr CFX_DIBSource::CloneAlphaMask( const FX_RECT* pClip) const { ASSERT(GetFormat() == FXDIB_Argb); FX_RECT rect(0, 0, m_Width, m_Height); @@ -606,7 +614,7 @@ CFX_RetainPtr CFX_DIBSource::CloneAlphaMask( if (rect.IsEmpty()) return nullptr; } - auto pMask = pdfium::MakeRetain(); + auto pMask = pdfium::MakeUnique(); if (!pMask->Create(rect.Width(), rect.Height(), FXDIB_8bppMask)) return nullptr; @@ -622,7 +630,7 @@ CFX_RetainPtr CFX_DIBSource::CloneAlphaMask( return pMask; } -bool CFX_DIBSource::SetAlphaMask(const CFX_RetainPtr& pAlphaMask, +bool CFX_DIBSource::SetAlphaMask(const CFX_DIBSource* pAlphaMask, const FX_RECT* pClip) { if (!HasAlpha() || GetFormat() == FXDIB_Argb) return false; @@ -652,12 +660,12 @@ bool CFX_DIBSource::SetAlphaMask(const CFX_RetainPtr& pAlphaMask, const int g_ChannelOffset[] = {0, 2, 1, 0, 0, 1, 2, 3, 3}; bool CFX_DIBitmap::LoadChannel(FXDIB_Channel destChannel, - const CFX_RetainPtr& pSrcBitmap, + CFX_DIBSource* pSrcBitmap, FXDIB_Channel srcChannel) { if (!m_pBuffer) return false; - CFX_RetainPtr pSrcClone = pSrcBitmap; + CFX_MaybeOwned pSrcClone(pSrcBitmap); int srcOffset; if (srcChannel == FXDIB_Alpha) { if (!pSrcBitmap->HasAlpha() && !pSrcBitmap->IsAlphaMask()) @@ -717,7 +725,7 @@ bool CFX_DIBitmap::LoadChannel(FXDIB_Channel destChannel, destOffset = g_ChannelOffset[destChannel]; } if (srcChannel == FXDIB_Alpha && pSrcClone->m_pAlphaMask) { - CFX_RetainPtr pAlphaMask = pSrcClone->m_pAlphaMask; + CFX_MaybeOwned pAlphaMask(pSrcClone->m_pAlphaMask); if (pSrcClone->GetWidth() != m_Width || pSrcClone->GetHeight() != m_Height) { if (pAlphaMask) { @@ -730,14 +738,14 @@ bool CFX_DIBitmap::LoadChannel(FXDIB_Channel destChannel, srcOffset = 0; } else if (pSrcClone->GetWidth() != m_Width || pSrcClone->GetHeight() != m_Height) { - CFX_RetainPtr pSrcMatched = + std::unique_ptr pSrcMatched = pSrcClone->StretchTo(m_Width, m_Height); if (!pSrcMatched) return false; pSrcClone = std::move(pSrcMatched); } - CFX_RetainPtr pDst(this); + CFX_DIBitmap* pDst = this; if (destChannel == FXDIB_Alpha && m_pAlphaMask) { pDst = m_pAlphaMask; destOffset = 0; @@ -815,8 +823,7 @@ bool CFX_DIBitmap::LoadChannel(FXDIB_Channel destChannel, int value) { return true; } -bool CFX_DIBitmap::MultiplyAlpha( - const CFX_RetainPtr& pSrcBitmap) { +bool CFX_DIBitmap::MultiplyAlpha(CFX_DIBSource* pSrcBitmap) { if (!m_pBuffer) return false; @@ -827,7 +834,8 @@ bool CFX_DIBitmap::MultiplyAlpha( if (!IsAlphaMask() && !HasAlpha()) return LoadChannel(FXDIB_Alpha, pSrcBitmap, FXDIB_Alpha); - CFX_RetainPtr pSrcClone = pSrcBitmap.As(); + CFX_MaybeOwned pSrcClone( + static_cast(pSrcBitmap)); if (pSrcBitmap->GetWidth() != m_Width || pSrcBitmap->GetHeight() != m_Height) { pSrcClone = pSrcBitmap->StretchTo(m_Width, m_Height); @@ -867,7 +875,7 @@ bool CFX_DIBitmap::MultiplyAlpha( } } } else { - m_pAlphaMask->MultiplyAlpha(pSrcClone); + m_pAlphaMask->MultiplyAlpha(pSrcClone.Get()); } } return true; @@ -879,9 +887,9 @@ bool CFX_DIBitmap::GetGrayData(void* pIccTransform) { } switch (GetFormat()) { case FXDIB_1bppRgb: { - if (!m_pPalette) + if (!m_pPalette) { return false; - + } uint8_t gray[2]; for (int i = 0; i < 2; i++) { int r = static_cast(m_pPalette.get()[i] >> 16); @@ -889,10 +897,11 @@ bool CFX_DIBitmap::GetGrayData(void* pIccTransform) { int b = static_cast(m_pPalette.get()[i]); gray[i] = static_cast(FXRGB2GRAY(r, g, b)); } - auto pMask = pdfium::MakeRetain(); - if (!pMask->Create(m_Width, m_Height, FXDIB_8bppMask)) + CFX_DIBitmap* pMask = new CFX_DIBitmap; + if (!pMask->Create(m_Width, m_Height, FXDIB_8bppMask)) { + delete pMask; return false; - + } FXSYS_memset(pMask->GetBuffer(), gray[0], pMask->GetPitch() * m_Height); for (int row = 0; row < m_Height; row++) { uint8_t* src_pos = m_pBuffer + row * m_Pitch; @@ -904,13 +913,14 @@ bool CFX_DIBitmap::GetGrayData(void* pIccTransform) { dest_pos++; } } - TakeOver(std::move(pMask)); + TakeOver(pMask); + delete pMask; break; } case FXDIB_8bppRgb: { - if (!m_pPalette) + if (!m_pPalette) { return false; - + } uint8_t gray[256]; for (int i = 0; i < 256; i++) { int r = static_cast(m_pPalette.get()[i] >> 16); @@ -918,10 +928,11 @@ bool CFX_DIBitmap::GetGrayData(void* pIccTransform) { int b = static_cast(m_pPalette.get()[i]); gray[i] = static_cast(FXRGB2GRAY(r, g, b)); } - auto pMask = pdfium::MakeRetain(); - if (!pMask->Create(m_Width, m_Height, FXDIB_8bppMask)) + CFX_DIBitmap* pMask = new CFX_DIBitmap; + if (!pMask->Create(m_Width, m_Height, FXDIB_8bppMask)) { + delete pMask; return false; - + } for (int row = 0; row < m_Height; row++) { uint8_t* dest_pos = pMask->GetBuffer() + row * pMask->GetPitch(); uint8_t* src_pos = m_pBuffer + row * m_Pitch; @@ -929,14 +940,16 @@ bool CFX_DIBitmap::GetGrayData(void* pIccTransform) { *dest_pos++ = gray[*src_pos++]; } } - TakeOver(std::move(pMask)); + TakeOver(pMask); + delete pMask; break; } case FXDIB_Rgb: { - auto pMask = pdfium::MakeRetain(); - if (!pMask->Create(m_Width, m_Height, FXDIB_8bppMask)) + CFX_DIBitmap* pMask = new CFX_DIBitmap; + if (!pMask->Create(m_Width, m_Height, FXDIB_8bppMask)) { + delete pMask; return false; - + } for (int row = 0; row < m_Height; row++) { uint8_t* src_pos = m_pBuffer + row * m_Pitch; uint8_t* dest_pos = pMask->GetBuffer() + row * pMask->GetPitch(); @@ -945,14 +958,16 @@ bool CFX_DIBitmap::GetGrayData(void* pIccTransform) { src_pos += 3; } } - TakeOver(std::move(pMask)); + TakeOver(pMask); + delete pMask; break; } case FXDIB_Rgb32: { - auto pMask = pdfium::MakeRetain(); - if (!pMask->Create(m_Width, m_Height, FXDIB_8bppMask)) + CFX_DIBitmap* pMask = new CFX_DIBitmap; + if (!pMask->Create(m_Width, m_Height, FXDIB_8bppMask)) { + delete pMask; return false; - + } for (int row = 0; row < m_Height; row++) { uint8_t* src_pos = m_pBuffer + row * m_Pitch; uint8_t* dest_pos = pMask->GetBuffer() + row * pMask->GetPitch(); @@ -961,7 +976,8 @@ bool CFX_DIBitmap::GetGrayData(void* pIccTransform) { src_pos += 4; } } - TakeOver(std::move(pMask)); + TakeOver(pMask); + delete pMask; break; } default: @@ -1326,9 +1342,9 @@ bool CFX_DIBitmap::ConvertColorScale(uint32_t forecolor, uint32_t backcolor) { return true; } -CFX_RetainPtr CFX_DIBSource::FlipImage(bool bXFlip, - bool bYFlip) const { - auto pFlipped = pdfium::MakeRetain(); +std::unique_ptr CFX_DIBSource::FlipImage(bool bXFlip, + bool bYFlip) const { + auto pFlipped = pdfium::MakeUnique(); if (!pFlipped->Create(m_Width, m_Height, GetFormat())) return nullptr; @@ -1398,12 +1414,12 @@ CFX_RetainPtr CFX_DIBSource::FlipImage(bool bXFlip, return pFlipped; } -CFX_DIBExtractor::CFX_DIBExtractor(const CFX_RetainPtr& pSrc) { +CFX_DIBExtractor::CFX_DIBExtractor(const CFX_DIBSource* pSrc) { if (pSrc->GetBuffer()) { - m_pBitmap = pdfium::MakeRetain(); + m_pBitmap = pdfium::MakeUnique(); if (!m_pBitmap->Create(pSrc->GetWidth(), pSrc->GetHeight(), pSrc->GetFormat(), pSrc->GetBuffer())) { - m_pBitmap.Reset(); + m_pBitmap.reset(); return; } m_pBitmap->SetPalette(pSrc->GetPalette()); @@ -1415,12 +1431,17 @@ CFX_DIBExtractor::CFX_DIBExtractor(const CFX_RetainPtr& pSrc) { CFX_DIBExtractor::~CFX_DIBExtractor() {} -CFX_FilteredDIB::CFX_FilteredDIB() {} +CFX_FilteredDIB::CFX_FilteredDIB() : m_pSrc(nullptr) {} -CFX_FilteredDIB::~CFX_FilteredDIB() {} +CFX_FilteredDIB::~CFX_FilteredDIB() { + if (m_bAutoDropSrc) { + delete m_pSrc; + } +} -void CFX_FilteredDIB::LoadSrc(const CFX_RetainPtr& pSrc) { +void CFX_FilteredDIB::LoadSrc(const CFX_DIBSource* pSrc, bool bAutoDropSrc) { m_pSrc = pSrc; + m_bAutoDropSrc = bAutoDropSrc; m_Width = pSrc->GetWidth(); m_Height = pSrc->GetHeight(); FXDIB_Format format = GetDestFormat(); @@ -1457,9 +1478,9 @@ CFX_ImageRenderer::CFX_ImageRenderer() { CFX_ImageRenderer::~CFX_ImageRenderer() {} -bool CFX_ImageRenderer::Start(const CFX_RetainPtr& pDevice, +bool CFX_ImageRenderer::Start(CFX_DIBitmap* pDevice, const CFX_ClipRgn* pClipRgn, - const CFX_RetainPtr& pSource, + const CFX_DIBSource* pSource, int bitmap_alpha, uint32_t mask_color, const CFX_Matrix* pMatrix, @@ -1548,7 +1569,7 @@ bool CFX_ImageRenderer::Continue(IFX_Pause* pPause) { if (m_pTransformer->Continue(pPause)) return true; - CFX_RetainPtr pBitmap = m_pTransformer->DetachBitmap(); + std::unique_ptr pBitmap(m_pTransformer->DetachBitmap()); if (!pBitmap || !pBitmap->GetBuffer()) return false; @@ -1564,16 +1585,16 @@ bool CFX_ImageRenderer::Continue(IFX_Pause* pPause) { } m_pDevice->CompositeMask( m_pTransformer->result().left, m_pTransformer->result().top, - pBitmap->GetWidth(), pBitmap->GetHeight(), pBitmap, m_MaskColor, 0, 0, - m_BlendType, m_pClipRgn, m_bRgbByteOrder, m_AlphaFlag, + pBitmap->GetWidth(), pBitmap->GetHeight(), pBitmap.get(), m_MaskColor, + 0, 0, m_BlendType, m_pClipRgn, m_bRgbByteOrder, m_AlphaFlag, m_pIccTransform); } else { if (m_BitmapAlpha != 255) pBitmap->MultiplyAlpha(m_BitmapAlpha); m_pDevice->CompositeBitmap( m_pTransformer->result().left, m_pTransformer->result().top, - pBitmap->GetWidth(), pBitmap->GetHeight(), pBitmap, 0, 0, m_BlendType, - m_pClipRgn, m_bRgbByteOrder, m_pIccTransform); + pBitmap->GetWidth(), pBitmap->GetHeight(), pBitmap.get(), 0, 0, + m_BlendType, m_pClipRgn, m_bRgbByteOrder, m_pIccTransform); } return false; } @@ -1586,11 +1607,11 @@ CFX_BitmapStorer::CFX_BitmapStorer() { CFX_BitmapStorer::~CFX_BitmapStorer() { } -CFX_RetainPtr CFX_BitmapStorer::Detach() { +std::unique_ptr CFX_BitmapStorer::Detach() { return std::move(m_pBitmap); } -void CFX_BitmapStorer::Replace(CFX_RetainPtr&& pBitmap) { +void CFX_BitmapStorer::Replace(std::unique_ptr pBitmap) { m_pBitmap = std::move(pBitmap); } @@ -1615,13 +1636,12 @@ bool CFX_BitmapStorer::SetInfo(int width, int height, FXDIB_Format src_format, uint32_t* pSrcPalette) { - auto pBitmap = pdfium::MakeRetain(); - if (!pBitmap->Create(width, height, src_format)) + m_pBitmap = pdfium::MakeUnique(); + if (!m_pBitmap->Create(width, height, src_format)) { + m_pBitmap.reset(); return false; - + } if (pSrcPalette) - pBitmap->SetPalette(pSrcPalette); - - m_pBitmap = std::move(pBitmap); + m_pBitmap->SetPalette(pSrcPalette); return true; } -- cgit v1.2.3