From cc3a3ee3ebcc1baabdfa7ffca5876dbbfa3980c1 Mon Sep 17 00:00:00 2001 From: Ryan Harrison Date: Tue, 26 Sep 2017 16:59:12 -0400 Subject: Make names of GIF types less opaque A lot of the structs in the GIF code use acronyms that are non-obvious unless you are familar with the GIF spec, and even then lead to lots of looking back. Making the naming more explicit. Also converted GifImage to be a struct, since it didn't have any methods defined for it. BUG=pdfium:903 Change-Id: I97ca2adfdc4ada8bcaef5f8f317c1dea7365d70d Reviewed-on: https://pdfium-review.googlesource.com/14836 Reviewed-by: dsinclair Commit-Queue: Ryan Harrison --- core/fxcodec/lgif/cfx_lzwdecoder.h | 6 +-- core/fxcodec/lgif/cgifcontext.cpp | 83 +++++++++++++++++++------------------- core/fxcodec/lgif/cgifcontext.h | 2 +- core/fxcodec/lgif/fx_gif.cpp | 22 +++------- core/fxcodec/lgif/fx_gif.h | 51 +++++++++++------------ 5 files changed, 74 insertions(+), 90 deletions(-) diff --git a/core/fxcodec/lgif/cfx_lzwdecoder.h b/core/fxcodec/lgif/cfx_lzwdecoder.h index 913df670c6..9dbf15d9b7 100644 --- a/core/fxcodec/lgif/cfx_lzwdecoder.h +++ b/core/fxcodec/lgif/cfx_lzwdecoder.h @@ -14,10 +14,10 @@ class CFX_LZWDecoder { public: - struct tag_Table { + typedef struct { uint16_t prefix; uint8_t suffix; - }; + } CodeEntry; // Returns nullptr on error static std::unique_ptr Create(uint8_t color_exp, @@ -44,7 +44,7 @@ class CFX_LZWDecoder { uint8_t code_first_; uint8_t stack_[GIF_MAX_LZW_CODE]; size_t stack_size_; - tag_Table code_table_[GIF_MAX_LZW_CODE]; + CodeEntry code_table_[GIF_MAX_LZW_CODE]; uint16_t code_old_; uint8_t* next_in_; uint32_t avail_in_; diff --git a/core/fxcodec/lgif/cgifcontext.cpp b/core/fxcodec/lgif/cgifcontext.cpp index 21e896405f..d1b9b44583 100644 --- a/core/fxcodec/lgif/cgifcontext.cpp +++ b/core/fxcodec/lgif/cgifcontext.cpp @@ -84,30 +84,28 @@ GifDecodeStatus CGifContext::ReadHeader() { AddError("Not A Gif Image"); return GifDecodeStatus::Error; } - GifLSD* gif_lsd_ptr = nullptr; + GifLocalScreenDescriptor* gif_lsd_ptr = nullptr; if (!ReadData(reinterpret_cast(&gif_lsd_ptr), 7)) { skip_size = skip_size_org; return GifDecodeStatus::Unfinished; } - if (reinterpret_cast(&gif_lsd_ptr->global_flag)->global_pal) { - global_pal_exp = - reinterpret_cast(&gif_lsd_ptr->global_flag)->pal_bits; - int32_t global_pal_size = (2 << global_pal_exp) * 3; + if (gif_lsd_ptr->global_flags.global_pal) { + global_pal_exp = gif_lsd_ptr->global_flags.pal_bits; + uint32_t global_pal_size = unsigned(2 << global_pal_exp) * 3u; uint8_t* global_pal_ptr = nullptr; if (!ReadData(&global_pal_ptr, global_pal_size)) { skip_size = skip_size_org; return GifDecodeStatus::Unfinished; } - global_sort_flag = ((GifGF*)&gif_lsd_ptr->global_flag)->sort_flag; - global_color_resolution = - ((GifGF*)&gif_lsd_ptr->global_flag)->color_resolution; + global_sort_flag = gif_lsd_ptr->global_flags.sort_flag; + global_color_resolution = gif_lsd_ptr->global_flags.color_resolution; m_GlobalPalette.resize(global_pal_size / 3); memcpy(m_GlobalPalette.data(), global_pal_ptr, global_pal_size); } - width = - (int)GetWord_LSBFirst(reinterpret_cast(&gif_lsd_ptr->width)); - height = - (int)GetWord_LSBFirst(reinterpret_cast(&gif_lsd_ptr->height)); + width = static_cast( + GetWord_LSBFirst(reinterpret_cast(&gif_lsd_ptr->width))); + height = static_cast( + GetWord_LSBFirst(reinterpret_cast(&gif_lsd_ptr->height))); bc_index = gif_lsd_ptr->bc_index; pixel_aspect = gif_lsd_ptr->pixel_aspect; return GifDecodeStatus::Success; @@ -222,10 +220,11 @@ GifDecodeStatus CGifContext::LoadFrame(int32_t frame_num) { } if (decode_status == GIF_D_STATUS_TAIL) { gif_image_ptr->m_ImageRowBuf.resize(gif_img_row_bytes); - GifGCE* gif_img_gce_ptr = gif_image_ptr->m_ImageGCE.get(); + GifGraphicControlExtension* gif_img_gce_ptr = + gif_image_ptr->m_ImageGCE.get(); int32_t loc_pal_num = - ((GifLF*)&gif_image_ptr->m_ImageInfo.local_flag)->local_pal - ? (2 << ((GifLF*)&gif_image_ptr->m_ImageInfo.local_flag)->pal_bits) + gif_image_ptr->m_ImageInfo.local_flags.local_pal + ? (2 << gif_image_ptr->m_ImageInfo.local_flags.pal_bits) : 0; avail_in = 0; GifPalette* pLocalPalette = gif_image_ptr->m_LocalPalettes.empty() @@ -236,8 +235,7 @@ GifDecodeStatus CGifContext::LoadFrame(int32_t frame_num) { gif_image_ptr->image_data_pos, gif_image_ptr->m_ImageInfo.left, gif_image_ptr->m_ImageInfo.top, gif_image_ptr->m_ImageInfo.width, gif_image_ptr->m_ImageInfo.height, loc_pal_num, pLocalPalette, 0, 0, - -1, 0, - (bool)((GifLF*)&gif_image_ptr->m_ImageInfo.local_flag)->interlace); + -1, 0, gif_image_ptr->m_ImageInfo.local_flags.interlace); if (!bRes) { gif_image_ptr->m_ImageRowBuf.clear(); AddError("Error Read Record Position Data"); @@ -248,14 +246,14 @@ GifDecodeStatus CGifContext::LoadFrame(int32_t frame_num) { gif_image_ptr->image_data_pos, gif_image_ptr->m_ImageInfo.left, gif_image_ptr->m_ImageInfo.top, gif_image_ptr->m_ImageInfo.width, gif_image_ptr->m_ImageInfo.height, loc_pal_num, pLocalPalette, - (int32_t)gif_image_ptr->m_ImageGCE->delay_time, - (bool)((GifCEF*)&gif_image_ptr->m_ImageGCE->gce_flag)->user_input, - ((GifCEF*)&gif_image_ptr->m_ImageGCE->gce_flag)->transparency - ? (int32_t)gif_image_ptr->m_ImageGCE->trans_index + static_cast(gif_image_ptr->m_ImageGCE->delay_time), + gif_image_ptr->m_ImageGCE->gce_flags.user_input, + gif_image_ptr->m_ImageGCE->gce_flags.transparency + ? static_cast(gif_image_ptr->m_ImageGCE->trans_index) : -1, - (int32_t)((GifCEF*)&gif_image_ptr->m_ImageGCE->gce_flag) - ->disposal_method, - (bool)((GifLF*)&gif_image_ptr->m_ImageInfo.local_flag)->interlace); + static_cast( + gif_image_ptr->m_ImageGCE->gce_flags.disposal_method), + gif_image_ptr->m_ImageInfo.local_flags.interlace); if (!bRes) { gif_image_ptr->m_ImageRowBuf.clear(); AddError("Error Read Record Position Data"); @@ -341,12 +339,12 @@ GifDecodeStatus CGifContext::LoadFrame(int32_t frame_num) { } } if (ret == GifDecodeStatus::InsufficientDestSize) { - if (((GifLF*)&gif_image_ptr->m_ImageInfo.local_flag)->interlace) { + if (gif_image_ptr->m_ImageInfo.local_flags.interlace) { ReadScanline(gif_image_ptr->image_row_num, gif_image_ptr->m_ImageRowBuf.data()); gif_image_ptr->image_row_num += s_gif_interlace_step[img_pass_num]; if (gif_image_ptr->image_row_num >= - (int32_t)gif_image_ptr->m_ImageInfo.height) { + static_cast(gif_image_ptr->m_ImageInfo.height)) { img_pass_num++; if (img_pass_num == FX_ArraySize(s_gif_interlace_step)) { DecodingFailureAtTailCleanup(gif_image_ptr); @@ -438,11 +436,11 @@ GifDecodeStatus CGifContext::DecodeExtension() { break; } case GIF_D_STATUS_EXT_PTE: { - GifPTE* gif_pte = nullptr; + GifPlainTextExtension* gif_pte = nullptr; if (!ReadData(reinterpret_cast(&gif_pte), 13)) return GifDecodeStatus::Unfinished; - m_GifGCE = nullptr; + m_GraphicControlExtension = nullptr; if (!ReadData(&data_size_ptr, 1)) { skip_size = skip_size_org; return GifDecodeStatus::Unfinished; @@ -457,22 +455,23 @@ GifDecodeStatus CGifContext::DecodeExtension() { break; } case GIF_D_STATUS_EXT_GCE: { - GifGCE* gif_gce_ptr = nullptr; + GifGraphicControlExtension* gif_gce_ptr = nullptr; if (!ReadData(reinterpret_cast(&gif_gce_ptr), 6)) return GifDecodeStatus::Unfinished; - if (!m_GifGCE.get()) - m_GifGCE = pdfium::MakeUnique(); - m_GifGCE->block_size = gif_gce_ptr->block_size; - m_GifGCE->gce_flag = gif_gce_ptr->gce_flag; - m_GifGCE->delay_time = GetWord_LSBFirst( + if (!m_GraphicControlExtension.get()) + m_GraphicControlExtension = + pdfium::MakeUnique(); + m_GraphicControlExtension->block_size = gif_gce_ptr->block_size; + m_GraphicControlExtension->gce_flags = gif_gce_ptr->gce_flags; + m_GraphicControlExtension->delay_time = GetWord_LSBFirst( reinterpret_cast(&gif_gce_ptr->delay_time)); - m_GifGCE->trans_index = gif_gce_ptr->trans_index; + m_GraphicControlExtension->trans_index = gif_gce_ptr->trans_index; break; } default: { if (decode_status == GIF_D_STATUS_EXT_PTE) - m_GifGCE = nullptr; + m_GraphicControlExtension = nullptr; if (!ReadData(&data_size_ptr, 1)) return GifDecodeStatus::Unfinished; @@ -508,16 +507,16 @@ GifDecodeStatus CGifContext::DecodeImageInfo() { GetWord_LSBFirst(reinterpret_cast(&gif_img_info_ptr->width)); gif_image->m_ImageInfo.height = GetWord_LSBFirst(reinterpret_cast(&gif_img_info_ptr->height)); - gif_image->m_ImageInfo.local_flag = gif_img_info_ptr->local_flag; + gif_image->m_ImageInfo.local_flags = gif_img_info_ptr->local_flags; if (gif_image->m_ImageInfo.left + gif_image->m_ImageInfo.width > width || gif_image->m_ImageInfo.top + gif_image->m_ImageInfo.height > height) { AddError("Image Data Out Of LSD, The File May Be Corrupt"); return GifDecodeStatus::Error; } - GifLF* gif_img_info_lf_ptr = (GifLF*)&gif_img_info_ptr->local_flag; + GifLocalFlags* gif_img_info_lf_ptr = &gif_img_info_ptr->local_flags; if (gif_img_info_lf_ptr->local_pal) { gif_image->local_pallette_exp = gif_img_info_lf_ptr->pal_bits; - int32_t loc_pal_size = (2 << gif_img_info_lf_ptr->pal_bits) * 3; + uint32_t loc_pal_size = unsigned(2 << gif_img_info_lf_ptr->pal_bits) * 3u; uint8_t* loc_pal_ptr = nullptr; if (!ReadData(&loc_pal_ptr, loc_pal_size)) { skip_size = skip_size_org; @@ -536,9 +535,9 @@ GifDecodeStatus CGifContext::DecodeImageInfo() { RecordCurrentPosition(&gif_image->image_data_pos); gif_image->image_data_pos += skip_size; gif_image->m_ImageGCE = nullptr; - if (m_GifGCE.get()) { - gif_image->m_ImageGCE = std::move(m_GifGCE); - m_GifGCE = nullptr; + if (m_GraphicControlExtension.get()) { + gif_image->m_ImageGCE = std::move(m_GraphicControlExtension); + m_GraphicControlExtension = nullptr; } m_Images.push_back(std::move(gif_image)); SaveDecodingStatus(GIF_D_STATUS_IMG_DATA); diff --git a/core/fxcodec/lgif/cgifcontext.h b/core/fxcodec/lgif/cgifcontext.h index 87f38f0694..c0a8a434bc 100644 --- a/core/fxcodec/lgif/cgifcontext.h +++ b/core/fxcodec/lgif/cgifcontext.h @@ -54,7 +54,7 @@ class CGifContext : public CCodec_GifModule::Context { int32_t decode_status; uint32_t skip_size; ByteString cmt_data; - std::unique_ptr m_GifGCE; + std::unique_ptr m_GraphicControlExtension; uint8_t* next_in; std::vector> m_Images; std::unique_ptr m_ImgDecoder; diff --git a/core/fxcodec/lgif/fx_gif.cpp b/core/fxcodec/lgif/fx_gif.cpp index db0744a6bb..b8e5cb8994 100644 --- a/core/fxcodec/lgif/fx_gif.cpp +++ b/core/fxcodec/lgif/fx_gif.cpp @@ -6,23 +6,13 @@ #include "core/fxcodec/lgif/fx_gif.h" -#include -#include - -#include "core/fxcodec/lbmp/fx_bmp.h" -#include "core/fxcodec/lgif/cgifcontext.h" -#include "third_party/base/logging.h" -#include "third_party/base/ptr_util.h" -#include "third_party/base/stl_util.h" - static_assert(sizeof(GifImageInfo) == 9, "GifImageInfo should have a size of 9"); static_assert(sizeof(GifPalette) == 3, "GifPalette should have a size of 3"); -static_assert(sizeof(GifPTE) == 13, "GifPTE should have a size of 13"); -static_assert(sizeof(GifGCE) == 5, "GifGCE should have a size of 5"); +static_assert(sizeof(GifPlainTextExtension) == 13, + "GifPlainTextExtension should have a size of 13"); +static_assert(sizeof(GifGraphicControlExtension) == 5, + "GifGraphicControlExtension should have a size of 5"); static_assert(sizeof(GifHeader) == 6, "GifHeader should have a size of 6"); -static_assert(sizeof(GifLSD) == 7, "GifLSD should have a size of 7"); - -GifImage::GifImage() {} - -GifImage::~GifImage() {} +static_assert(sizeof(GifLocalScreenDescriptor) == 7, + "GifLocalScreenDescriptor should have a size of 7"); diff --git a/core/fxcodec/lgif/fx_gif.h b/core/fxcodec/lgif/fx_gif.h index bbf847de39..08a64c8326 100644 --- a/core/fxcodec/lgif/fx_gif.h +++ b/core/fxcodec/lgif/fx_gif.h @@ -37,58 +37,57 @@ class CGifContext; #define GIF_D_STATUS_IMG_DATA 0x0A #pragma pack(1) -typedef struct tagGifGF { +typedef struct { uint8_t pal_bits : 3; uint8_t sort_flag : 1; uint8_t color_resolution : 3; uint8_t global_pal : 1; -} GifGF; +} GifGlobalFlags; -typedef struct tagGifLF { +typedef struct { uint8_t pal_bits : 3; uint8_t reserved : 2; uint8_t sort_flag : 1; uint8_t interlace : 1; uint8_t local_pal : 1; -} GifLF; +} GifLocalFlags; -typedef struct tagGifHeader { +typedef struct { char signature[3]; char version[3]; } GifHeader; -typedef struct tagGifLSD { +typedef struct { uint16_t width; uint16_t height; - uint8_t global_flag; + GifGlobalFlags global_flags; uint8_t bc_index; uint8_t pixel_aspect; -} GifLSD; +} GifLocalScreenDescriptor; -typedef struct tagGifImageInfo { +typedef struct { uint16_t left; uint16_t top; uint16_t width; uint16_t height; - - uint8_t local_flag; + GifLocalFlags local_flags; } GifImageInfo; -typedef struct tagGifCEF { +typedef struct { uint8_t transparency : 1; uint8_t user_input : 1; uint8_t disposal_method : 3; uint8_t reserved : 3; -} GifCEF; +} GifControlExtensionFlags; -typedef struct tagGifGCE { +typedef struct { uint8_t block_size; - uint8_t gce_flag; + GifControlExtensionFlags gce_flags; uint16_t delay_time; uint8_t trans_index; -} GifGCE; +} GifGraphicControlExtension; -typedef struct tagGifPTE { +typedef struct { uint8_t block_size; uint16_t grid_left; uint16_t grid_top; @@ -100,15 +99,15 @@ typedef struct tagGifPTE { uint8_t fc_index; uint8_t bc_index; -} GifPTE; +} GifPlainTextExtension; -typedef struct tagGifAE { +typedef struct { uint8_t block_size; uint8_t app_identify[8]; uint8_t app_authentication[3]; -} GifAE; +} GifApplicationExtension; -typedef struct tagGifPalette { uint8_t r, g, b; } GifPalette; +typedef struct { uint8_t r, g, b; } GifPalette; #pragma pack() enum class GifDecodeStatus { @@ -118,12 +117,8 @@ enum class GifDecodeStatus { InsufficientDestSize, // Only used internally by CGifLZWDecoder::Decode() }; -class GifImage { - public: - GifImage(); - ~GifImage(); - - std::unique_ptr m_ImageGCE; +typedef struct { + std::unique_ptr m_ImageGCE; std::vector m_LocalPalettes; std::vector m_ImageRowBuf; GifImageInfo m_ImageInfo; @@ -131,6 +126,6 @@ class GifImage { uint8_t image_code_exp; uint32_t image_data_pos; int32_t image_row_num; -}; +} GifImage; #endif // CORE_FXCODEC_LGIF_FX_GIF_H_ -- cgit v1.2.3