From 9f810e7216c2dbb1a1ab090f1bee4207ecd871c0 Mon Sep 17 00:00:00 2001 From: Bo Xu Date: Sun, 31 Aug 2014 15:23:46 -0700 Subject: Remove the GetValidBpc check in application callers and move it to where m_bpc is assigned. The problem of using GetValidBpc() in each function call is it could result in mismatch as seen in this case: in ContinueToLoadMask(), m_bpc is re-assigned to 1 if m_bImageMask==1 regardless of the value from GetValidBpc(). This will result in mismatch if another function use the value from GetValidBpc(). The solution could be checking m_bImageMask in another function to make sure m_bpc is consistent, but that makes the code too cumbersome. Also, we have to bring and are bringing in more and more GetValidBpc check, and this will continue with other buggy documents. So better to fix it now. The original rational to use GetValidBpc() in where m_bpc is used is to respect the "raw" data from parsing. However, if it will be ignored anyway and using value from GetValidBpc(), we'd better correct it at the very beginning. BUG=408541 R=tsepez@chromium.org Review URL: https://codereview.chromium.org/518443002 --- .../fpdfapi/fpdf_render/fpdf_render_loadimage.cpp | 138 ++++++++++----------- core/src/fpdfapi/fpdf_render/render_int.h | 4 +- 2 files changed, 67 insertions(+), 75 deletions(-) diff --git a/core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp b/core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp index 5f0abde2be..f3a1996ab4 100644 --- a/core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp +++ b/core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp @@ -253,20 +253,19 @@ FX_BOOL CPDF_DIBSource::Load(CPDF_Document* pDoc, const CPDF_Stream* pStream, CP } int CPDF_DIBSource::ContinueToLoadMask() { - FX_DWORD bpc = GetValidBpc(); if (m_bImageMask) { m_bpp = 1; - bpc = 1; + m_bpc = 1; m_nComponents = 1; m_AlphaFlag = 1; - } else if (bpc * m_nComponents == 1) { + } else if (m_bpc * m_nComponents == 1) { m_bpp = 1; - } else if (bpc * m_nComponents <= 8) { + } else if (m_bpc * m_nComponents <= 8) { m_bpp = 8; } else { m_bpp = 24; } - if (!bpc || !m_nComponents) { + if (!m_bpc || !m_nComponents) { return 0; } FX_SAFE_DWORD pitch = m_Width; @@ -451,6 +450,8 @@ int CPDF_DIBSource::ContinueLoadDIBSource(IFX_Pause* pPause) } FX_BOOL CPDF_DIBSource::LoadColorInfo(CPDF_Dictionary* pFormResources, CPDF_Dictionary* pPageResources) { + m_bpc_orig = m_pDict->GetInteger(FX_BSTRC("BitsPerComponent")); + ValidateBpc(); if (m_pDict->GetInteger("ImageMask")) { m_bImageMask = TRUE; } @@ -492,7 +493,6 @@ FX_BOOL CPDF_DIBSource::LoadColorInfo(CPDF_Dictionary* pFormResources, CPDF_Dict if (m_pColorSpace == NULL) { return FALSE; } - m_bpc = m_pDict->GetInteger(FX_BSTRC("BitsPerComponent")); m_Family = m_pColorSpace->GetFamily(); m_nComponents = m_pColorSpace->CountComponents(); if (m_Family == PDFCS_ICCBASED && pCSObj->GetType() == PDFOBJ_NAME) { @@ -565,8 +565,7 @@ int CPDF_DIBSource::CreateDecoder() if (decoder.IsEmpty()) { return 1; } - FX_DWORD bpc = GetValidBpc(); - if (bpc == 0) { + if (m_bpc == 0) { return 0; } FX_LPCBYTE src_data = m_pStreamAcc->GetData(); @@ -579,17 +578,17 @@ int CPDF_DIBSource::CreateDecoder() m_nComponents, pParams ? pParams->GetInteger(FX_BSTR("ColorTransform"), 1) : 1); if (NULL == m_pDecoder) { FX_BOOL bTransform = FALSE; - int comps, tmp_bpc; + int comps, bpc; ICodec_JpegModule* pJpegModule = CPDF_ModuleMgr::Get()->GetJpegModule(); - if (pJpegModule->LoadInfo(src_data, src_size, m_Width, m_Height, comps, tmp_bpc, bTransform)) { + if (pJpegModule->LoadInfo(src_data, src_size, m_Width, m_Height, comps, bpc, bTransform)) { m_nComponents = comps; - m_bpc = bpc = tmp_bpc; + m_bpc = bpc; m_pDecoder = CPDF_ModuleMgr::Get()->GetJpegModule()->CreateDecoder(src_data, src_size, m_Width, m_Height, m_nComponents, bTransform); } } } else if (decoder == FX_BSTRC("FlateDecode")) { - m_pDecoder = FPDFAPI_CreateFlateDecoder(src_data, src_size, m_Width, m_Height, m_nComponents, bpc, pParams); + m_pDecoder = FPDFAPI_CreateFlateDecoder(src_data, src_size, m_Width, m_Height, m_nComponents, m_bpc, pParams); } else if (decoder == FX_BSTRC("JPXDecode")) { LoadJpxBitmap(); return m_pCachedBitmap != NULL ? 1 : 0; @@ -603,10 +602,10 @@ int CPDF_DIBSource::CreateDecoder() m_Status = 1; return 2; } else if (decoder == FX_BSTRC("RunLengthDecode")) { - m_pDecoder = CPDF_ModuleMgr::Get()->GetCodecModule()->GetBasicModule()->CreateRunLengthDecoder(src_data, src_size, m_Width, m_Height, m_nComponents, bpc); + m_pDecoder = CPDF_ModuleMgr::Get()->GetCodecModule()->GetBasicModule()->CreateRunLengthDecoder(src_data, src_size, m_Width, m_Height, m_nComponents, m_bpc); } if (m_pDecoder) { - FX_SAFE_DWORD requested_pitch = bpc; + FX_SAFE_DWORD requested_pitch = m_bpc; requested_pitch *= m_nComponents; requested_pitch *= m_Width; requested_pitch += 7; @@ -855,17 +854,16 @@ int CPDF_DIBSource::StartLoadMaskDIB() } void CPDF_DIBSource::LoadPalette() { - FX_DWORD bpc = GetValidBpc(); - if (bpc == 0) { + if (m_bpc == 0) { return; } - if (bpc * m_nComponents > 8) { + if (m_bpc * m_nComponents > 8) { return; } if (m_pColorSpace == NULL) { return; } - if (bpc * m_nComponents == 1) { + if (m_bpc * m_nComponents == 1) { if (m_bDefaultDecode && (m_Family == PDFCS_DEVICEGRAY || m_Family == PDFCS_DEVICERGB)) { return; } @@ -889,16 +887,16 @@ void CPDF_DIBSource::LoadPalette() } return; } - if (m_pColorSpace == CPDF_ColorSpace::GetStockCS(PDFCS_DEVICEGRAY) && bpc == 8 && m_bDefaultDecode) { + if (m_pColorSpace == CPDF_ColorSpace::GetStockCS(PDFCS_DEVICEGRAY) && m_bpc == 8 && m_bDefaultDecode) { } else { - int palette_count = 1 << (bpc * m_nComponents); + int palette_count = 1 << (m_bpc * m_nComponents); CFX_FixedBufGrow color_values(m_nComponents); FX_FLOAT* color_value = color_values; for (int i = 0; i < palette_count; i ++) { int color_data = i; for (FX_DWORD j = 0; j < m_nComponents; j ++) { - int encoded_component = color_data % (1 << bpc); - color_data /= 1 << bpc; + int encoded_component = color_data % (1 << m_bpc); + color_data /= 1 << m_bpc; color_value[j] = m_pCompData[j].m_DecodeMin + m_pCompData[j].m_DecodeStep * encoded_component; } FX_FLOAT R = 0, G = 0, B = 0; @@ -917,49 +915,46 @@ void CPDF_DIBSource::LoadPalette() } } } -FX_DWORD CPDF_DIBSource::GetValidBpc() const +void CPDF_DIBSource::ValidateBpc() { - FX_DWORD bpc = m_bpc; + m_bpc = m_bpc_orig; CPDF_Object * pFilter = m_pDict ? m_pDict->GetElementValue(FX_BSTRC("Filter")) : NULL; if (pFilter) { if (pFilter->GetType() == PDFOBJ_NAME) { CFX_ByteString filter = pFilter->GetString(); if (filter == FX_BSTRC("CCITTFaxDecode") || filter == FX_BSTRC("JBIG2Decode")) { - bpc = 1; + m_bpc = 1; } if (filter == FX_BSTRC("RunLengthDecode") || filter == FX_BSTRC("DCTDecode")) { - bpc = 8; + m_bpc = 8; } } else if (pFilter->GetType() == PDFOBJ_ARRAY) { CPDF_Array *pArray = (CPDF_Array *)pFilter; if (pArray->GetString(pArray->GetCount() - 1) == FX_BSTRC("CCITTFacDecode") || pArray->GetString(pArray->GetCount() - 1) == FX_BSTRC("JBIG2Decode")) { - bpc = 1; + m_bpc = 1; } if (pArray->GetString(pArray->GetCount() - 1) == FX_BSTRC("RunLengthDecode") || pArray->GetString(pArray->GetCount() - 1) == FX_BSTRC("DCTDecode")) { - bpc = 8; + m_bpc = 8; } } } - if (bpc != 1 && bpc != 2 && bpc != 4 && bpc != 8 && bpc != 16) { - bpc = 0; + if (m_bpc != 1 && m_bpc != 2 && m_bpc != 4 && m_bpc != 8 && m_bpc != 16) { + m_bpc = 0; } - - return bpc; } #define NORMALCOLOR_MAX(color, max) (color) > (max) ? (max) : (color) < 0 ? 0 : (color); void CPDF_DIBSource::TranslateScanline24bpp(FX_LPBYTE dest_scan, FX_LPCBYTE src_scan) const { - FX_DWORD bpc = GetValidBpc(); - if (bpc == 0) { + if (m_bpc == 0) { return; } - int max_data = (1 << bpc) - 1; + int max_data = (1 << m_bpc) - 1; if (m_bDefaultDecode) { if (m_Family == PDFCS_DEVICERGB || m_Family == PDFCS_CALRGB) { FX_LPCBYTE src_pos = src_scan; - switch (bpc) { + switch (m_bpc) { case 16: for (int col = 0; col < m_Width; col ++) { *dest_scan++ = src_pos[4]; @@ -980,12 +975,12 @@ void CPDF_DIBSource::TranslateScanline24bpp(FX_LPBYTE dest_scan, FX_LPCBYTE src_ int src_bit_pos = 0; int dest_byte_pos = 0; for (int column = 0; column < m_Width; column ++) { - int R = _GetBits8(src_scan, src_bit_pos, bpc); - src_bit_pos += bpc; - int G = _GetBits8(src_scan, src_bit_pos, bpc); - src_bit_pos += bpc; - int B = _GetBits8(src_scan, src_bit_pos, bpc); - src_bit_pos += bpc; + int R = _GetBits8(src_scan, src_bit_pos, m_bpc); + src_bit_pos += m_bpc; + int G = _GetBits8(src_scan, src_bit_pos, m_bpc); + src_bit_pos += m_bpc; + int B = _GetBits8(src_scan, src_bit_pos, m_bpc); + src_bit_pos += m_bpc; R = NORMALCOLOR_MAX(R, max_data); G = NORMALCOLOR_MAX(G, max_data); B = NORMALCOLOR_MAX(B, max_data); @@ -997,7 +992,7 @@ void CPDF_DIBSource::TranslateScanline24bpp(FX_LPBYTE dest_scan, FX_LPCBYTE src_ break; } return; - } else if (bpc == 8) { + } else if (m_bpc == 8) { if (m_nComponents == m_pColorSpace->CountComponents()) m_pColorSpace->TranslateImageLine(dest_scan, src_scan, m_Width, m_Width, m_Height, m_bLoadMask && m_GroupFamily == PDFCS_DEVICECMYK && m_Family == PDFCS_DEVICECMYK); @@ -1007,7 +1002,7 @@ void CPDF_DIBSource::TranslateScanline24bpp(FX_LPBYTE dest_scan, FX_LPCBYTE src_ CFX_FixedBufGrow color_values1(m_nComponents); FX_FLOAT* color_values = color_values1; FX_FLOAT R = 0.0f, G = 0.0f, B = 0.0f; - if (bpc == 8) { + if (m_bpc == 8) { int src_byte_pos = 0; int dest_byte_pos = 0; for (int column = 0; column < m_Width; column ++) { @@ -1035,13 +1030,12 @@ void CPDF_DIBSource::TranslateScanline24bpp(FX_LPBYTE dest_scan, FX_LPCBYTE src_ } else { int src_bit_pos = 0; int dest_byte_pos = 0; - FX_DWORD bpc = GetValidBpc(); for (int column = 0; column < m_Width; column ++) { for (FX_DWORD color = 0; color < m_nComponents; color ++) { - int data = _GetBits8(src_scan, src_bit_pos, bpc); + int data = _GetBits8(src_scan, src_bit_pos, m_bpc); color_values[color] = m_pCompData[color].m_DecodeMin + m_pCompData[color].m_DecodeStep * data; - src_bit_pos += bpc; + src_bit_pos += m_bpc; } if (m_bLoadMask && m_GroupFamily == PDFCS_DEVICECMYK && m_Family == PDFCS_DEVICECMYK) { FX_FLOAT k = 1.0f - color_values[3]; @@ -1070,12 +1064,11 @@ FX_LPBYTE CPDF_DIBSource::GetBuffer() const } FX_LPCBYTE CPDF_DIBSource::GetScanline(int line) const { - FX_DWORD bpc = GetValidBpc(); - if (bpc == 0) { + if (m_bpc == 0) { return NULL; } FX_SAFE_DWORD src_pitch = m_Width; - src_pitch *= bpc; + src_pitch *= m_bpc; src_pitch *= m_nComponents; src_pitch += 7; src_pitch /= 8; @@ -1100,7 +1093,7 @@ FX_LPCBYTE CPDF_DIBSource::GetScanline(int line) const FXSYS_memset8(pLineBuf, 0xff, m_Pitch); return pLineBuf; } - if (bpc * m_nComponents == 1) { + if (m_bpc * m_nComponents == 1) { if (m_bImageMask && m_bDefaultDecode) { for (FX_DWORD i = 0; i < src_pitch_value; i++) { m_pLineBuf[i] = ~pSrcLine[i]; @@ -1132,17 +1125,17 @@ FX_LPCBYTE CPDF_DIBSource::GetScanline(int line) const } return m_pLineBuf; } - if (bpc * m_nComponents <= 8) { - if (bpc == 8) { + if (m_bpc * m_nComponents <= 8) { + if (m_bpc == 8) { FXSYS_memcpy32(m_pLineBuf, pSrcLine, src_pitch_value); } else { int src_bit_pos = 0; for (int col = 0; col < m_Width; col ++) { int color_index = 0; for (FX_DWORD color = 0; color < m_nComponents; color ++) { - int data = _GetBits8(pSrcLine, src_bit_pos, bpc); - color_index |= data << (color * bpc); - src_bit_pos += bpc; + int data = _GetBits8(pSrcLine, src_bit_pos, m_bpc); + color_index |= data << (color * m_bpc); + src_bit_pos += m_bpc; } m_pLineBuf[col] = color_index; } @@ -1169,7 +1162,7 @@ FX_LPCBYTE CPDF_DIBSource::GetScanline(int line) const return m_pLineBuf; } if (m_bColorKey) { - if (m_nComponents == 3 && bpc == 8) { + if (m_nComponents == 3 && m_bpc == 8) { FX_LPBYTE alpha_channel = m_pMaskedLine + 3; for (int col = 0; col < m_Width; col ++) { FX_LPCBYTE pPixel = pSrcLine + col * 3; @@ -1214,10 +1207,9 @@ void CPDF_DIBSource::DownSampleScanline(int line, FX_LPBYTE dest_scan, int dest_ return; } - FX_DWORD bpc = GetValidBpc(); FX_DWORD src_width = m_Width; FX_SAFE_DWORD pitch = src_width; - pitch *= bpc; + pitch *= m_bpc; pitch *= m_nComponents; pitch += 7; pitch /= 8; @@ -1241,7 +1233,7 @@ void CPDF_DIBSource::DownSampleScanline(int line, FX_LPBYTE dest_scan, int dest_ pSrcLine = m_pStreamAcc->GetData() + line * src_pitch; } } - int orig_Bpp = bpc * m_nComponents / 8; + int orig_Bpp = m_bpc * m_nComponents / 8; int dest_Bpp = dest_bpp / 8; if (pSrcLine == NULL) { FXSYS_memset32(dest_scan, 0xff, dest_Bpp * clip_width); @@ -1257,7 +1249,7 @@ void CPDF_DIBSource::DownSampleScanline(int line, FX_LPBYTE dest_scan, int dest_ } CFX_FixedBufGrow temp(orig_Bpp); - if (bpc * m_nComponents == 1) { + if (m_bpc * m_nComponents == 1) { FX_DWORD set_argb = (FX_DWORD) - 1, reset_argb = 0; if (m_bImageMask) { if (m_bDefaultDecode) { @@ -1325,15 +1317,15 @@ void CPDF_DIBSource::DownSampleScanline(int line, FX_LPBYTE dest_scan, int dest_ } } return; - } else if (bpc * m_nComponents <= 8) { - if (bpc < 8) { + } else if (m_bpc * m_nComponents <= 8) { + if (m_bpc < 8) { int src_bit_pos = 0; for (FX_DWORD col = 0; col < src_width; col ++) { int color_index = 0; for (FX_DWORD color = 0; color < m_nComponents; color ++) { - int data = _GetBits8(pSrcLine, src_bit_pos, bpc); - color_index |= data << (color * bpc); - src_bit_pos += bpc; + int data = _GetBits8(pSrcLine, src_bit_pos, m_bpc); + color_index |= data << (color * m_bpc); + src_bit_pos += m_bpc; } m_pLineBuf[col] = color_index; } @@ -1382,14 +1374,14 @@ void CPDF_DIBSource::DownSampleScanline(int line, FX_LPBYTE dest_scan, int dest_ } else { int last_src_x = -1; FX_ARGB last_argb; - FX_FLOAT orig_Not8Bpp = (FX_FLOAT)bpc * (FX_FLOAT)m_nComponents / 8.0f; - FX_FLOAT unit_To8Bpc = 255.0f / ((1 << bpc) - 1); + FX_FLOAT orig_Not8Bpp = (FX_FLOAT)m_bpc * (FX_FLOAT)m_nComponents / 8.0f; + FX_FLOAT unit_To8Bpc = 255.0f / ((1 << m_bpc) - 1); for (int i = 0; i < clip_width; i ++) { int dest_x = clip_left + i; FX_DWORD src_x = (bFlipX ? (dest_width - dest_x - 1) : dest_x) * (FX_INT64)src_width / dest_width; src_x %= src_width; FX_LPCBYTE pSrcPixel = NULL; - if (bpc % 8 == 0) { + if (m_bpc % 8 == 0) { pSrcPixel = pSrcLine + src_x * orig_Bpp; } else { pSrcPixel = pSrcLine + (int)(src_x * orig_Not8Bpp); @@ -1408,14 +1400,14 @@ void CPDF_DIBSource::DownSampleScanline(int line, FX_LPBYTE dest_scan, int dest_ } m_pColorSpace->TranslateImageLine(color, temp, 1, 0, 0, m_bLoadMask && m_GroupFamily == PDFCS_DEVICECMYK && m_Family == PDFCS_DEVICECMYK); } else { - if (bpc < 8) { + if (m_bpc < 8) { int src_bit_pos = 0; if (src_x % 2) { src_bit_pos = 4; } for (FX_DWORD i = 0; i < m_nComponents; i ++) { - temp[i] = (FX_BYTE)(_GetBits8(pSrcPixel, src_bit_pos, bpc) * unit_To8Bpc); - src_bit_pos += bpc; + temp[i] = (FX_BYTE)(_GetBits8(pSrcPixel, src_bit_pos, m_bpc) * unit_To8Bpc); + src_bit_pos += m_bpc; } m_pColorSpace->TranslateImageLine(color, temp, 1, 0, 0, m_bLoadMask && m_GroupFamily == PDFCS_DEVICECMYK && m_Family == PDFCS_DEVICECMYK); } else { @@ -1428,7 +1420,7 @@ void CPDF_DIBSource::DownSampleScanline(int line, FX_LPBYTE dest_scan, int dest_ } if (m_bColorKey) { int alpha = 0xff; - if (m_nComponents == 3 && bpc == 8) { + if (m_nComponents == 3 && m_bpc == 8) { alpha = (pSrcPixel[0] < m_pCompData[0].m_ColorKeyMin || pSrcPixel[0] > m_pCompData[0].m_ColorKeyMax || pSrcPixel[1] < m_pCompData[1].m_ColorKeyMin || diff --git a/core/src/fpdfapi/fpdf_render/render_int.h b/core/src/fpdfapi/fpdf_render/render_int.h index fda3666871..af0e74e5e4 100644 --- a/core/src/fpdfapi/fpdf_render/render_int.h +++ b/core/src/fpdfapi/fpdf_render/render_int.h @@ -419,13 +419,13 @@ protected: void LoadPalette(); FX_BOOL CreateDecoder(); void TranslateScanline24bpp(FX_LPBYTE dest_scan, FX_LPCBYTE src_scan) const; - FX_DWORD GetValidBpc() const; + void ValidateBpc(); CPDF_Document* m_pDocument; const CPDF_Stream* m_pStream; CPDF_StreamAcc* m_pStreamAcc; const CPDF_Dictionary* m_pDict; CPDF_ColorSpace* m_pColorSpace; - FX_DWORD m_Family, m_bpc, m_nComponents, m_GroupFamily; + FX_DWORD m_Family, m_bpc, m_bpc_orig, m_nComponents, m_GroupFamily; FX_BOOL m_bLoadMask; FX_BOOL m_bDefaultDecode, m_bImageMask, m_bColorKey; DIB_COMP_DATA* m_pCompData; -- cgit v1.2.3