diff options
author | Nicolas Pena <npm@chromium.org> | 2017-07-12 11:04:12 -0400 |
---|---|---|
committer | Chromium commit bot <commit-bot@chromium.org> | 2017-07-12 18:26:43 +0000 |
commit | 58bf3de718e510648ee136c887be5b2d577644a7 (patch) | |
tree | fceba4f29df5fc6ecf4da1cf41d6bae0cdda05d2 | |
parent | 071f5eda0399d6f260cc1b67beffcdaa315a9af7 (diff) | |
download | pdfium-58bf3de718e510648ee136c887be5b2d577644a7.tar.xz |
Cleanup ICC code part 1
This CL switches void* to CLcmsCmm*, simplifies the ownership and
destruction of CLcmsCmm, and reduces unnecessary function calling in
fx_codec_icc.cpp.
Change-Id: Ifdbf59dcdaede497d1684b161dd066726cf08ee3
Reviewed-on: https://pdfium-review.googlesource.com/7590
Reviewed-by: dsinclair <dsinclair@chromium.org>
Commit-Queue: Nicolás Peña <npm@chromium.org>
-rw-r--r-- | core/fpdfapi/page/cpdf_iccprofile.cpp | 10 | ||||
-rw-r--r-- | core/fpdfapi/page/cpdf_iccprofile.h | 9 | ||||
-rw-r--r-- | core/fxcodec/codec/ccodec_iccmodule.h | 34 | ||||
-rw-r--r-- | core/fxcodec/codec/fx_codec_icc.cpp | 200 | ||||
-rw-r--r-- | testing/libfuzzer/pdf_codec_icc_fuzzer.cc | 6 |
5 files changed, 96 insertions, 163 deletions
diff --git a/core/fpdfapi/page/cpdf_iccprofile.cpp b/core/fpdfapi/page/cpdf_iccprofile.cpp index afb505a7e6..0fa4be4b5a 100644 --- a/core/fpdfapi/page/cpdf_iccprofile.cpp +++ b/core/fpdfapi/page/cpdf_iccprofile.cpp @@ -8,7 +8,6 @@ #include "core/fpdfapi/cpdf_modulemgr.h" #include "core/fpdfapi/parser/cpdf_stream.h" -#include "core/fxcodec/codec/ccodec_iccmodule.h" namespace { @@ -29,12 +28,9 @@ CPDF_IccProfile::CPDF_IccProfile(CPDF_Stream* pStream, uint32_t nSrcComps = 0; auto* pIccModule = CPDF_ModuleMgr::Get()->GetIccModule(); - m_pTransform = pIccModule->CreateTransform_sRGB(pData, dwSize, nSrcComps); - if (m_pTransform) + m_Transform = pIccModule->CreateTransform_sRGB(pData, dwSize, &nSrcComps); + if (m_Transform) m_nSrcComponents = nSrcComps; } -CPDF_IccProfile::~CPDF_IccProfile() { - if (m_pTransform) - CPDF_ModuleMgr::Get()->GetIccModule()->DestroyTransform(m_pTransform); -} +CPDF_IccProfile::~CPDF_IccProfile() {} diff --git a/core/fpdfapi/page/cpdf_iccprofile.h b/core/fpdfapi/page/cpdf_iccprofile.h index a0cf744b9c..156fb595dc 100644 --- a/core/fpdfapi/page/cpdf_iccprofile.h +++ b/core/fpdfapi/page/cpdf_iccprofile.h @@ -7,6 +7,9 @@ #ifndef CORE_FPDFAPI_PAGE_CPDF_ICCPROFILE_H_ #define CORE_FPDFAPI_PAGE_CPDF_ICCPROFILE_H_ +#include <memory> + +#include "core/fxcodec/codec/ccodec_iccmodule.h" #include "core/fxcrt/cfx_retain_ptr.h" #include "core/fxcrt/cfx_unowned_ptr.h" @@ -20,8 +23,8 @@ class CPDF_IccProfile : public CFX_Retainable { CPDF_Stream* GetStream() const { return m_pStream.Get(); } bool IsValid() const { return IsSRGB() || IsSupported(); } bool IsSRGB() const { return m_bsRGB; } - bool IsSupported() const { return !!m_pTransform; } - void* transform() { return m_pTransform; } + bool IsSupported() const { return !!m_Transform; } + CLcmsCmm* transform() { return m_Transform.get(); } uint32_t GetComponents() const { return m_nSrcComponents; } private: @@ -30,7 +33,7 @@ class CPDF_IccProfile : public CFX_Retainable { const bool m_bsRGB; CFX_UnownedPtr<CPDF_Stream> const m_pStream; - void* m_pTransform = nullptr; + std::unique_ptr<CLcmsCmm> m_Transform; uint32_t m_nSrcComponents = 0; }; diff --git a/core/fxcodec/codec/ccodec_iccmodule.h b/core/fxcodec/codec/ccodec_iccmodule.h index 08878acc98..e775475249 100644 --- a/core/fxcodec/codec/ccodec_iccmodule.h +++ b/core/fxcodec/codec/ccodec_iccmodule.h @@ -7,23 +7,41 @@ #ifndef CORE_FXCODEC_CODEC_CCODEC_ICCMODULE_H_ #define CORE_FXCODEC_CODEC_CCODEC_ICCMODULE_H_ +#include <memory> + #include "core/fxcodec/fx_codec_def.h" #include "core/fxcrt/fx_string.h" #include "core/fxcrt/fx_system.h" +#include "third_party/base/ptr_util.h" + +#if defined(USE_SYSTEM_LCMS2) +#include <lcms2.h> +#else +#include "third_party/lcms2-2.6/include/lcms2.h" +#endif + +class CLcmsCmm { + public: + CLcmsCmm(int srcComponents, cmsHTRANSFORM transform, bool isLab); + ~CLcmsCmm(); + + cmsHTRANSFORM m_hTransform; + int m_nSrcComponents; + bool m_bLab; +}; class CCodec_IccModule { public: CCodec_IccModule(); ~CCodec_IccModule(); - void* CreateTransform_sRGB(const uint8_t* pProfileData, - uint32_t dwProfileSize, - uint32_t& nComponents, - int32_t intent = 0, - uint32_t dwSrcFormat = Icc_FORMAT_DEFAULT); - void DestroyTransform(void* pTransform); - void Translate(void* pTransform, const float* pSrcValues, float* pDestValues); - void TranslateScanline(void* pTransform, + std::unique_ptr<CLcmsCmm> CreateTransform_sRGB(const uint8_t* pProfileData, + uint32_t dwProfileSize, + uint32_t* nComponents); + void Translate(CLcmsCmm* pTransform, + const float* pSrcValues, + float* pDestValues); + void TranslateScanline(CLcmsCmm* pTransform, uint8_t* pDest, const uint8_t* pSrc, int pixels); diff --git a/core/fxcodec/codec/fx_codec_icc.cpp b/core/fxcodec/codec/fx_codec_icc.cpp index ee562f0e92..3c08843544 100644 --- a/core/fxcodec/codec/fx_codec_icc.cpp +++ b/core/fxcodec/codec/fx_codec_icc.cpp @@ -4,6 +4,8 @@ // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com +#include <memory> + #include "core/fxcodec/codec/codec_int.h" #include "core/fxcodec/fx_codec.h" @@ -13,70 +15,51 @@ #include "third_party/lcms2-2.6/include/lcms2.h" #endif -struct CLcmsCmm { - cmsHTRANSFORM m_hTransform; - int m_nSrcComponents; - int m_nDstComponents; - bool m_bLab; -}; -bool CheckComponents(cmsColorSpaceSignature cs, int nComponents, bool bDst) { - if (nComponents <= 0 || nComponents > 15) { - return false; - } +namespace { + +bool Check3Components(cmsColorSpaceSignature cs, bool bDst) { switch (cs) { - case cmsSigLabData: - if (nComponents < 3) { - return false; - } - break; case cmsSigGrayData: - if (bDst && nComponents != 1) { - return false; - } - if (!bDst && nComponents > 2) { - return false; - } - break; - case cmsSigRgbData: - if (bDst && nComponents != 3) { - return false; - } - break; + return false; case cmsSigCmykData: - if (bDst && nComponents != 4) { + if (bDst) return false; - } break; + case cmsSigLabData: + case cmsSigRgbData: default: - if (nComponents != 3) { - return false; - } break; } return true; } -void* IccLib_CreateTransform(const unsigned char* pSrcProfileData, - uint32_t dwSrcProfileSize, - uint32_t& nSrcComponents, - const unsigned char* pDstProfileData, - uint32_t dwDstProfileSize, - int32_t nDstComponents, - int intent, - uint32_t dwSrcFormat = Icc_FORMAT_DEFAULT, - uint32_t dwDstFormat = Icc_FORMAT_DEFAULT) { - nSrcComponents = 0; +} // namespace + +CLcmsCmm::CLcmsCmm(int srcComponents, cmsHTRANSFORM hTransform, bool isLab) + : m_hTransform(hTransform), + m_nSrcComponents(srcComponents), + m_bLab(isLab) {} + +CLcmsCmm::~CLcmsCmm() { + cmsDeleteTransform(m_hTransform); +} + +CCodec_IccModule::CCodec_IccModule() : m_nComponents(0) {} + +CCodec_IccModule::~CCodec_IccModule() {} + +std::unique_ptr<CLcmsCmm> CCodec_IccModule::CreateTransform_sRGB( + const unsigned char* pSrcProfileData, + uint32_t dwSrcProfileSize, + uint32_t* nSrcComponents) { + *nSrcComponents = 0; cmsHPROFILE srcProfile = cmsOpenProfileFromMem(pSrcProfileData, dwSrcProfileSize); if (!srcProfile) return nullptr; cmsHPROFILE dstProfile; - if (!pDstProfileData && dwDstProfileSize == 0 && nDstComponents == 3) { - dstProfile = cmsCreate_sRGBProfile(); - } else { - dstProfile = cmsOpenProfileFromMem(pDstProfileData, dwDstProfileSize); - } + dstProfile = cmsCreate_sRGBProfile(); if (!dstProfile) { cmsCloseProfile(srcProfile); return nullptr; @@ -85,9 +68,9 @@ void* IccLib_CreateTransform(const unsigned char* pSrcProfileData, bool bLab = false; cmsColorSpaceSignature srcCS = cmsGetColorSpace(srcProfile); - nSrcComponents = cmsChannelsOf(srcCS); + *nSrcComponents = cmsChannelsOf(srcCS); // According to PDF spec, number of components must be 1, 3, or 4. - if (nSrcComponents != 1 && nSrcComponents != 3 && nSrcComponents != 4) { + if (*nSrcComponents != 1 && *nSrcComponents != 3 && *nSrcComponents != 4) { cmsCloseProfile(srcProfile); cmsCloseProfile(dstProfile); return nullptr; @@ -95,23 +78,24 @@ void* IccLib_CreateTransform(const unsigned char* pSrcProfileData, if (srcCS == cmsSigLabData) { srcFormat = - COLORSPACE_SH(PT_Lab) | CHANNELS_SH(nSrcComponents) | BYTES_SH(0); + COLORSPACE_SH(PT_Lab) | CHANNELS_SH(*nSrcComponents) | BYTES_SH(0); bLab = true; } else { srcFormat = - COLORSPACE_SH(PT_ANY) | CHANNELS_SH(nSrcComponents) | BYTES_SH(1); - if (srcCS == cmsSigRgbData && T_DOSWAP(dwSrcFormat)) { + COLORSPACE_SH(PT_ANY) | CHANNELS_SH(*nSrcComponents) | BYTES_SH(1); + if (srcCS == cmsSigRgbData && T_DOSWAP(Icc_FORMAT_DEFAULT)) { srcFormat |= DOSWAP_SH(1); } } cmsColorSpaceSignature dstCS = cmsGetColorSpace(dstProfile); - if (!CheckComponents(dstCS, nDstComponents, true)) { + if (!Check3Components(dstCS, true)) { cmsCloseProfile(srcProfile); cmsCloseProfile(dstProfile); return nullptr; } cmsHTRANSFORM hTransform = nullptr; + int intent = 0; switch (dstCS) { case cmsSigGrayData: hTransform = cmsCreateTransform(srcProfile, srcFormat, dstProfile, @@ -124,7 +108,7 @@ void* IccLib_CreateTransform(const unsigned char* pSrcProfileData, case cmsSigCmykData: hTransform = cmsCreateTransform( srcProfile, srcFormat, dstProfile, - T_DOSWAP(dwDstFormat) ? TYPE_KYMC_8 : TYPE_CMYK_8, intent, 0); + T_DOSWAP(Icc_FORMAT_DEFAULT) ? TYPE_KYMC_8 : TYPE_CMYK_8, intent, 0); break; default: break; @@ -134,114 +118,46 @@ void* IccLib_CreateTransform(const unsigned char* pSrcProfileData, cmsCloseProfile(dstProfile); return nullptr; } - CLcmsCmm* pCmm = new CLcmsCmm; - pCmm->m_nSrcComponents = nSrcComponents; - pCmm->m_nDstComponents = nDstComponents; - pCmm->m_hTransform = hTransform; - pCmm->m_bLab = bLab; + auto pCmm = pdfium::MakeUnique<CLcmsCmm>(*nSrcComponents, hTransform, bLab); cmsCloseProfile(srcProfile); cmsCloseProfile(dstProfile); return pCmm; } -void* IccLib_CreateTransform_sRGB(const unsigned char* pProfileData, - uint32_t dwProfileSize, - uint32_t& nComponents, - int32_t intent, - uint32_t dwSrcFormat) { - return IccLib_CreateTransform(pProfileData, dwProfileSize, nComponents, - nullptr, 0, 3, intent, dwSrcFormat); -} -void IccLib_DestroyTransform(void* pTransform) { - if (!pTransform) { - return; - } - cmsDeleteTransform(((CLcmsCmm*)pTransform)->m_hTransform); - delete (CLcmsCmm*)pTransform; -} -void IccLib_Translate(void* pTransform, - uint32_t nSrcComponents, - const float* pSrcValues, - float* pDestValues) { +void CCodec_IccModule::Translate(CLcmsCmm* pTransform, + const float* pSrcValues, + float* pDestValues) { if (!pTransform) return; - CLcmsCmm* p = (CLcmsCmm*)pTransform; + uint32_t nSrcComponents = m_nComponents; uint8_t output[4]; - if (p->m_bLab) { + if (pTransform->m_bLab) { CFX_FixedBufGrow<double, 16> inputs(nSrcComponents); double* input = inputs; - for (uint32_t i = 0; i < nSrcComponents; i++) + for (uint32_t i = 0; i < nSrcComponents; ++i) input[i] = pSrcValues[i]; - cmsDoTransform(p->m_hTransform, input, output, 1); + cmsDoTransform(pTransform->m_hTransform, input, output, 1); } else { CFX_FixedBufGrow<uint8_t, 16> inputs(nSrcComponents); uint8_t* input = inputs; - for (uint32_t i = 0; i < nSrcComponents; i++) { - if (pSrcValues[i] > 1.0f) - input[i] = 255; - else if (pSrcValues[i] < 0) - input[i] = 0; - else - input[i] = static_cast<int>(pSrcValues[i] * 255.0f); + for (uint32_t i = 0; i < nSrcComponents; ++i) { + input[i] = + pdfium::clamp(static_cast<int>(pSrcValues[i] * 255.0f), 0, 255); } - cmsDoTransform(p->m_hTransform, input, output, 1); + cmsDoTransform(pTransform->m_hTransform, input, output, 1); } - switch (p->m_nDstComponents) { - case 1: - pDestValues[0] = output[0] / 255.0f; - break; - case 3: - pDestValues[0] = output[2] / 255.0f; - pDestValues[1] = output[1] / 255.0f; - pDestValues[2] = output[0] / 255.0f; - break; - case 4: - pDestValues[0] = output[0] / 255.0f; - pDestValues[1] = output[1] / 255.0f; - pDestValues[2] = output[2] / 255.0f; - pDestValues[3] = output[3] / 255.0f; - break; - } -} - -void IccLib_TranslateImage(void* pTransform, - unsigned char* pDest, - const unsigned char* pSrc, - int32_t pixels) { - if (!pTransform) - return; - cmsDoTransform(((CLcmsCmm*)pTransform)->m_hTransform, pSrc, pDest, pixels); -} - -CCodec_IccModule::CCodec_IccModule() : m_nComponents(0) {} - -CCodec_IccModule::~CCodec_IccModule() { -} -void* CCodec_IccModule::CreateTransform_sRGB(const uint8_t* pProfileData, - uint32_t dwProfileSize, - uint32_t& nComponents, - int32_t intent, - uint32_t dwSrcFormat) { - return IccLib_CreateTransform_sRGB(pProfileData, dwProfileSize, nComponents, - intent, dwSrcFormat); -} - -void CCodec_IccModule::DestroyTransform(void* pTransform) { - IccLib_DestroyTransform(pTransform); -} - -void CCodec_IccModule::Translate(void* pTransform, - const float* pSrcValues, - float* pDestValues) { - IccLib_Translate(pTransform, m_nComponents, pSrcValues, pDestValues); + pDestValues[0] = output[2] / 255.0f; + pDestValues[1] = output[1] / 255.0f; + pDestValues[2] = output[0] / 255.0f; } -void CCodec_IccModule::TranslateScanline(void* pTransform, - uint8_t* pDest, - const uint8_t* pSrc, +void CCodec_IccModule::TranslateScanline(CLcmsCmm* pTransform, + unsigned char* pDest, + const unsigned char* pSrc, int32_t pixels) { - IccLib_TranslateImage(pTransform, pDest, pSrc, pixels); + if (pTransform) + cmsDoTransform(pTransform->m_hTransform, pSrc, pDest, pixels); } const uint8_t g_CMYKSamples[81 * 81 * 3] = { diff --git a/testing/libfuzzer/pdf_codec_icc_fuzzer.cc b/testing/libfuzzer/pdf_codec_icc_fuzzer.cc index c7abdd69a4..7021017953 100644 --- a/testing/libfuzzer/pdf_codec_icc_fuzzer.cc +++ b/testing/libfuzzer/pdf_codec_icc_fuzzer.cc @@ -9,7 +9,8 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { CCodec_IccModule icc_module; uint32_t nComponent = 0; - void* transform = icc_module.CreateTransform_sRGB(data, size, nComponent); + std::unique_ptr<CLcmsCmm> transform = + icc_module.CreateTransform_sRGB(data, size, &nComponent); if (transform) { float src[4]; @@ -17,8 +18,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { for (int i = 0; i < 4; i++) src[i] = 0.5f; icc_module.SetComponents(nComponent); - icc_module.Translate(transform, src, dst); - icc_module.DestroyTransform(transform); + icc_module.Translate(transform.get(), src, dst); } return 0; |