From 1a1d7648d3e338b756e464cebb2ae1a815359afa Mon Sep 17 00:00:00 2001 From: tsepez Date: Tue, 6 Dec 2016 06:29:28 -0800 Subject: Return unique_ptrs from CFX_DIBitmap::Clone(). Because that's what clone does. Perform immediate release in some spots to avoid disrupting too much at once. Review-Url: https://codereview.chromium.org/2534953004 --- core/fxge/dib/fx_dib_convert.cpp | 9 +- core/fxge/dib/fx_dib_main.cpp | 172 +++++++++++++------------------------ core/fxge/dib/fx_dib_transform.cpp | 25 +++--- 3 files changed, 81 insertions(+), 125 deletions(-) (limited to 'core/fxge/dib') diff --git a/core/fxge/dib/fx_dib_convert.cpp b/core/fxge/dib/fx_dib_convert.cpp index f5ae563d57..b1362462c3 100644 --- a/core/fxge/dib/fx_dib_convert.cpp +++ b/core/fxge/dib/fx_dib_convert.cpp @@ -9,6 +9,7 @@ #include "core/fxcodec/fx_codec.h" #include "core/fxge/fx_dib.h" +#include "third_party/base/ptr_util.h" class CFX_Palette { public: @@ -781,11 +782,12 @@ bool ConvertBuffer(FXDIB_Format dest_format, } } -CFX_DIBitmap* CFX_DIBSource::CloneConvert(FXDIB_Format dest_format) const { +std::unique_ptr CFX_DIBSource::CloneConvert( + FXDIB_Format dest_format) const { if (dest_format == GetFormat()) return Clone(nullptr); - std::unique_ptr pClone(new CFX_DIBitmap); + std::unique_ptr pClone = pdfium::MakeUnique(); if (!pClone->Create(m_Width, m_Height, dest_format)) return nullptr; @@ -819,7 +821,8 @@ CFX_DIBitmap* CFX_DIBSource::CloneConvert(FXDIB_Format dest_format) const { } if (pal_8bpp) pClone->CopyPalette(pal_8bpp.get()); - return pClone.release(); + + return pClone; } bool CFX_DIBitmap::ConvertFormat(FXDIB_Format dest_format) { diff --git a/core/fxge/dib/fx_dib_main.cpp b/core/fxge/dib/fx_dib_main.cpp index 83899553a2..310c62fcc5 100644 --- a/core/fxge/dib/fx_dib_main.cpp +++ b/core/fxge/dib/fx_dib_main.cpp @@ -13,6 +13,7 @@ #include #include "core/fxcodec/fx_codec.h" +#include "core/fxcrt/cfx_maybe_owned.h" #include "core/fxge/cfx_gemodule.h" #include "core/fxge/dib/dib_int.h" #include "core/fxge/ge/cfx_cliprgn.h" @@ -174,15 +175,14 @@ void CFX_DIBitmap::TakeOver(CFX_DIBitmap* pSrcBitmap) { m_Pitch = pSrcBitmap->m_Pitch; } -CFX_DIBitmap* 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()) { + if (rect.IsEmpty()) return nullptr; - } } - std::unique_ptr pNewBitmap(new CFX_DIBitmap); + auto pNewBitmap = pdfium::MakeUnique(); if (!pNewBitmap->Create(rect.Width(), rect.Height(), GetFormat())) return nullptr; @@ -202,22 +202,22 @@ CFX_DIBitmap* CFX_DIBSource::Clone(const FX_RECT* pClip) const { } } else { int copy_len = (pNewBitmap->GetWidth() * pNewBitmap->GetBPP() + 7) / 8; - if (m_Pitch < (uint32_t)copy_len) { + if (m_Pitch < (uint32_t)copy_len) copy_len = m_Pitch; - } + for (int row = rect.top; row < rect.bottom; row++) { const uint8_t* src_scan = GetScanline(row) + rect.left * m_bpp / 8; uint8_t* dest_scan = (uint8_t*)pNewBitmap->GetScanline(row - rect.top); FXSYS_memcpy(dest_scan, src_scan, copy_len); } } - return pNewBitmap.release(); + return pNewBitmap; } void CFX_DIBSource::BuildPalette() { - if (m_pPalette) { + if (m_pPalette) return; - } + if (GetBPP() == 1) { m_pPalette.reset(FX_Alloc(uint32_t, 2)); if (IsCmykImage()) { @@ -230,13 +230,11 @@ void CFX_DIBSource::BuildPalette() { } else if (GetBPP() == 8) { m_pPalette.reset(FX_Alloc(uint32_t, 256)); if (IsCmykImage()) { - for (int i = 0; i < 256; i++) { + for (int i = 0; i < 256; i++) m_pPalette.get()[i] = 0xff - i; - } } else { - for (int i = 0; i < 256; i++) { + for (int i = 0; i < 256; i++) m_pPalette.get()[i] = 0xff000000 | (i * 0x10101); - } } } } @@ -663,127 +661,92 @@ bool CFX_DIBSource::CopyAlphaMask(const CFX_DIBSource* pAlphaMask, const int g_ChannelOffset[] = {0, 2, 1, 0, 0, 1, 2, 3, 3}; bool CFX_DIBitmap::LoadChannel(FXDIB_Channel destChannel, - const CFX_DIBSource* pSrcBitmap, + CFX_DIBSource* pSrcBitmap, FXDIB_Channel srcChannel) { - if (!m_pBuffer) { + if (!m_pBuffer) return false; - } - CFX_DIBSource* pSrcClone = (CFX_DIBSource*)pSrcBitmap; - CFX_DIBitmap* pDst = this; - int destOffset, srcOffset; + + CFX_MaybeOwned pSrcClone(pSrcBitmap); + int srcOffset; if (srcChannel == FXDIB_Alpha) { - if (!pSrcBitmap->HasAlpha() && !pSrcBitmap->IsAlphaMask()) { + if (!pSrcBitmap->HasAlpha() && !pSrcBitmap->IsAlphaMask()) return false; - } + if (pSrcBitmap->GetBPP() == 1) { pSrcClone = pSrcBitmap->CloneConvert(FXDIB_8bppMask); - if (!pSrcClone) { + if (!pSrcClone) return false; - } - } - if (pSrcBitmap->GetFormat() == FXDIB_Argb) { - srcOffset = 3; - } else { - srcOffset = 0; } + srcOffset = pSrcBitmap->GetFormat() == FXDIB_Argb ? 3 : 0; } else { - if (pSrcBitmap->IsAlphaMask()) { + if (pSrcBitmap->IsAlphaMask()) return false; - } + if (pSrcBitmap->GetBPP() < 24) { if (pSrcBitmap->IsCmykImage()) { - pSrcClone = pSrcBitmap->CloneConvert( - (FXDIB_Format)((pSrcBitmap->GetFormat() & 0xff00) | 0x20)); + pSrcClone = pSrcBitmap->CloneConvert(static_cast( + (pSrcBitmap->GetFormat() & 0xff00) | 0x20)); } else { - pSrcClone = pSrcBitmap->CloneConvert( - (FXDIB_Format)((pSrcBitmap->GetFormat() & 0xff00) | 0x18)); + pSrcClone = pSrcBitmap->CloneConvert(static_cast( + (pSrcBitmap->GetFormat() & 0xff00) | 0x18)); } - if (!pSrcClone) { + if (!pSrcClone) return false; - } } srcOffset = g_ChannelOffset[srcChannel]; } + int destOffset = 0; if (destChannel == FXDIB_Alpha) { if (IsAlphaMask()) { - if (!ConvertFormat(FXDIB_8bppMask)) { - if (pSrcClone != pSrcBitmap) { - delete pSrcClone; - } + if (!ConvertFormat(FXDIB_8bppMask)) return false; - } - destOffset = 0; } else { - destOffset = 0; - if (!ConvertFormat(IsCmykImage() ? FXDIB_Cmyka : FXDIB_Argb)) { - if (pSrcClone != pSrcBitmap) { - delete pSrcClone; - } + if (!ConvertFormat(IsCmykImage() ? FXDIB_Cmyka : FXDIB_Argb)) return false; - } - if (GetFormat() == FXDIB_Argb) { + + if (GetFormat() == FXDIB_Argb) destOffset = 3; - } } } else { - if (IsAlphaMask()) { - if (pSrcClone != pSrcBitmap) { - delete pSrcClone; - } + if (IsAlphaMask()) return false; - } + if (GetBPP() < 24) { if (HasAlpha()) { - if (!ConvertFormat(IsCmykImage() ? FXDIB_Cmyka : FXDIB_Argb)) { - if (pSrcClone != pSrcBitmap) { - delete pSrcClone; - } + if (!ConvertFormat(IsCmykImage() ? FXDIB_Cmyka : FXDIB_Argb)) return false; - } #if _FXM_PLATFORM_ == _FXM_PLATFORM_APPLE_ } else if (!ConvertFormat(IsCmykImage() ? FXDIB_Cmyk : FXDIB_Rgb32)) { #else } else if (!ConvertFormat(IsCmykImage() ? FXDIB_Cmyk : FXDIB_Rgb)) { #endif - if (pSrcClone != pSrcBitmap) { - delete pSrcClone; - } return false; } } destOffset = g_ChannelOffset[destChannel]; } if (srcChannel == FXDIB_Alpha && pSrcClone->m_pAlphaMask) { - CFX_DIBitmap* pAlphaMask = pSrcClone->m_pAlphaMask; + CFX_MaybeOwned pAlphaMask(pSrcClone->m_pAlphaMask); if (pSrcClone->GetWidth() != m_Width || pSrcClone->GetHeight() != m_Height) { if (pAlphaMask) { pAlphaMask = pAlphaMask->StretchTo(m_Width, m_Height); - if (!pAlphaMask) { - if (pSrcClone != pSrcBitmap) { - delete pSrcClone; - } + if (!pAlphaMask) return false; - } } } - if (pSrcClone != pSrcBitmap) { - pSrcClone->m_pAlphaMask = nullptr; - delete pSrcClone; - } - pSrcClone = pAlphaMask; + pSrcClone = std::move(pAlphaMask); srcOffset = 0; } else if (pSrcClone->GetWidth() != m_Width || pSrcClone->GetHeight() != m_Height) { - CFX_DIBitmap* pSrcMatched = pSrcClone->StretchTo(m_Width, m_Height); - if (pSrcClone != pSrcBitmap) { - delete pSrcClone; - } - if (!pSrcMatched) { + std::unique_ptr pSrcMatched = + pSrcClone->StretchTo(m_Width, m_Height); + if (!pSrcMatched) return false; - } - pSrcClone = pSrcMatched; + + pSrcClone = std::move(pSrcMatched); } + CFX_DIBitmap* pDst = this; if (destChannel == FXDIB_Alpha && m_pAlphaMask) { pDst = m_pAlphaMask; destOffset = 0; @@ -799,9 +762,6 @@ bool CFX_DIBitmap::LoadChannel(FXDIB_Channel destChannel, src_pos += srcBytes; } } - if (pSrcClone != pSrcBitmap && pSrcClone != pSrcBitmap->m_pAlphaMask) { - delete pSrcClone; - } return true; } @@ -864,40 +824,36 @@ bool CFX_DIBitmap::LoadChannel(FXDIB_Channel destChannel, int value) { return true; } -bool CFX_DIBitmap::MultiplyAlpha(const CFX_DIBSource* pSrcBitmap) { - if (!m_pBuffer) { +bool CFX_DIBitmap::MultiplyAlpha(CFX_DIBSource* pSrcBitmap) { + if (!m_pBuffer) return false; - } + ASSERT(pSrcBitmap->IsAlphaMask()); - if (!pSrcBitmap->IsAlphaMask()) { + if (!pSrcBitmap->IsAlphaMask()) return false; - } - if (!IsAlphaMask() && !HasAlpha()) { + + if (!IsAlphaMask() && !HasAlpha()) return LoadChannel(FXDIB_Alpha, pSrcBitmap, FXDIB_Alpha); - } - CFX_DIBitmap* pSrcClone = (CFX_DIBitmap*)pSrcBitmap; + + CFX_MaybeOwned pSrcClone( + static_cast(pSrcBitmap)); if (pSrcBitmap->GetWidth() != m_Width || pSrcBitmap->GetHeight() != m_Height) { pSrcClone = pSrcBitmap->StretchTo(m_Width, m_Height); - if (!pSrcClone) { + if (!pSrcClone) return false; - } } if (IsAlphaMask()) { - if (!ConvertFormat(FXDIB_8bppMask)) { - if (pSrcClone != pSrcBitmap) { - delete pSrcClone; - } + if (!ConvertFormat(FXDIB_8bppMask)) return false; - } + for (int row = 0; row < m_Height; row++) { uint8_t* dest_scan = m_pBuffer + m_Pitch * row; uint8_t* src_scan = pSrcClone->m_pBuffer + pSrcClone->m_Pitch * row; if (pSrcClone->GetBPP() == 1) { for (int col = 0; col < m_Width; col++) { - if (!((1 << (7 - col % 8)) & src_scan[col / 8])) { + if (!((1 << (7 - col % 8)) & src_scan[col / 8])) dest_scan[col] = 0; - } } } else { for (int col = 0; col < m_Width; col++) { @@ -908,12 +864,9 @@ bool CFX_DIBitmap::MultiplyAlpha(const CFX_DIBSource* pSrcBitmap) { } } else { if (GetFormat() == FXDIB_Argb) { - if (pSrcClone->GetBPP() == 1) { - if (pSrcClone != pSrcBitmap) { - delete pSrcClone; - } + if (pSrcClone->GetBPP() == 1) return false; - } + for (int row = 0; row < m_Height; row++) { uint8_t* dest_scan = m_pBuffer + m_Pitch * row + 3; uint8_t* src_scan = pSrcClone->m_pBuffer + pSrcClone->m_Pitch * row; @@ -923,12 +876,9 @@ bool CFX_DIBitmap::MultiplyAlpha(const CFX_DIBSource* pSrcBitmap) { } } } else { - m_pAlphaMask->MultiplyAlpha(pSrcClone); + m_pAlphaMask->MultiplyAlpha(pSrcClone.Get()); } } - if (pSrcClone != pSrcBitmap) { - delete pSrcClone; - } return true; } @@ -1476,7 +1426,7 @@ CFX_DIBExtractor::CFX_DIBExtractor(const CFX_DIBSource* pSrc) { m_pBitmap->CopyPalette(pSrc->GetPalette()); m_pBitmap->CopyAlphaMask(pSrc->m_pAlphaMask); } else { - m_pBitmap.reset(pSrc->Clone()); + m_pBitmap = pSrc->Clone(); } } diff --git a/core/fxge/dib/fx_dib_transform.cpp b/core/fxge/dib/fx_dib_transform.cpp index b938d648a8..dacc0aa393 100644 --- a/core/fxge/dib/fx_dib_transform.cpp +++ b/core/fxge/dib/fx_dib_transform.cpp @@ -301,23 +301,25 @@ FX_RECT FXDIB_SwapClipBox(FX_RECT& clip, return rect; } -CFX_DIBitmap* CFX_DIBSource::TransformTo(const CFX_Matrix* pDestMatrix, - int& result_left, - int& result_top, - uint32_t flags, - const FX_RECT* pDestClip) const { +std::unique_ptr CFX_DIBSource::TransformTo( + const CFX_Matrix* pDestMatrix, + int& result_left, + int& result_top, + uint32_t flags, + const FX_RECT* pDestClip) const { CFX_ImageTransformer transformer(this, pDestMatrix, flags, pDestClip); transformer.Start(); transformer.Continue(nullptr); result_left = transformer.result().left; result_top = transformer.result().top; - return transformer.DetachBitmap().release(); + return transformer.DetachBitmap(); } -CFX_DIBitmap* CFX_DIBSource::StretchTo(int dest_width, - int dest_height, - uint32_t flags, - const FX_RECT* pClip) const { +std::unique_ptr CFX_DIBSource::StretchTo( + int dest_width, + int dest_height, + uint32_t flags, + const FX_RECT* pClip) const { FX_RECT clip_rect(0, 0, FXSYS_abs(dest_width), FXSYS_abs(dest_height)); if (pClip) clip_rect.Intersect(*pClip); @@ -333,7 +335,8 @@ CFX_DIBitmap* CFX_DIBSource::StretchTo(int dest_width, clip_rect, flags); if (stretcher.Start()) stretcher.Continue(nullptr); - return storer.Detach().release(); + + return storer.Detach(); } CFX_ImageTransformer::CFX_ImageTransformer(const CFX_DIBSource* pSrc, -- cgit v1.2.3