Give names to the shading types

core/src/fpdfapi/fpdf_page/fpdf_page_pattern.cpp

 // Copyright 2014 PDFium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
 #include "../../../include/fpdfapi/fpdf_page.h"
 #include "pageint.h"
 
 CPDF_Pattern::CPDF_Pattern(const CFX_AffineMatrix* pParentMatrix)
@@ skipping 60 matching lines @@
     CPDF_Dictionary* pDict = m_pPatternObj->GetDict();
     ASSERT(pDict != NULL);
     m_Pattern2Form = pDict->GetMatrix(FX_BSTRC("Matrix"));
     m_pShadingObj = pDict->GetElementValue(FX_BSTRC("Shading"));
     if (parentMatrix) {
       m_Pattern2Form.Concat(*parentMatrix);
     }
   } else {
     m_pShadingObj = pPatternObj;
   }
-  m_ShadingType = 0;
+  m_ShadingType = kInvalidShading;
   m_pCS = NULL;
   m_nFuncs = 0;
   for (int i = 0; i < 4; i++) {
     m_pFunctions[i] = NULL;
   }
   m_pCountedCS = NULL;
 }
 CPDF_ShadingPattern::~CPDF_ShadingPattern() {
   Clear();
 }
 void CPDF_ShadingPattern::Clear() {
   for (int i = 0; i < m_nFuncs; i++) {
     delete m_pFunctions[i];
     m_pFunctions[i] = NULL;
   }
   CPDF_ColorSpace* pCS = m_pCountedCS ? m_pCountedCS->get() : NULL;
   if (pCS && m_pDocument) {
     m_pDocument->GetPageData()->ReleaseColorSpace(pCS->GetArray());
   }
-  m_ShadingType = 0;
+  m_ShadingType = kInvalidShading;
   m_pCS = NULL;
   m_pCountedCS = NULL;
   m_nFuncs = 0;
 }
+
+static ShadingType ToShadingType(int type) {
+  ShadingType st = static_cast<ShadingType>(type);
+  ASSERT(st > kInvalidShading && st < kMaxShading);

[Tom Sepez, 2015/10/27 16:20:16]

We need something stronger than an assert here, since the value in question
comes directly from the document, and we'd expect to see arbitrary values in the
wild. And < comparisions with ints is probably cleaner than with enums (e.g.
we'll never cast an out-of-range value to the enum type) so maybe:

return (type > static_cast<int>(kInvalidShading) &&
        type < static_cast<int>(kMaxShading)) ?
       static_cast<ShadingType>(type) : kInvalidShading;

[dsinclair, 2015/10/27 17:00:37]

Done.

+  return st;
+}
+
 FX_BOOL CPDF_ShadingPattern::Load() {
-  if (m_ShadingType != 0) {
+  if (m_ShadingType != kInvalidShading)
     return TRUE;
-  }
+
   CPDF_Dictionary* pShadingDict =
       m_pShadingObj ? m_pShadingObj->GetDict() : NULL;
   if (pShadingDict == NULL) {
     return FALSE;
   }
   if (m_nFuncs) {
     for (int i = 0; i < m_nFuncs; i++)
       delete m_pFunctions[i];
     m_nFuncs = 0;
   }
@@ skipping 12 matching lines @@
   }
   CPDF_Object* pCSObj = pShadingDict->GetElementValue(FX_BSTRC("ColorSpace"));
   if (pCSObj == NULL) {
     return FALSE;
   }
   CPDF_DocPageData* pDocPageData = m_pDocument->GetPageData();
   m_pCS = pDocPageData->GetColorSpace(pCSObj, NULL);
   if (m_pCS) {
     m_pCountedCS = pDocPageData->FindColorSpacePtr(m_pCS->GetArray());
   }
-  m_ShadingType = pShadingDict->GetInteger(FX_BSTRC("ShadingType"));
+  m_ShadingType =
+      ToShadingType(pShadingDict->GetInteger(FX_BSTRC("ShadingType")));
+
   return TRUE;
 }
 FX_BOOL CPDF_ShadingPattern::Reload() {
   Clear();
   return Load();
 }
 FX_BOOL CPDF_MeshStream::Load(CPDF_Stream* pShadingStream,
                               CPDF_Function** pFuncs,
                               int nFuncs,
                               CPDF_ColorSpace* pCS) {
@@ skipping 88 matching lines @@
     if (m_BitStream.IsEOF()) {
       return FALSE;
     }
     GetCoords(vertex[i].x, vertex[i].y);
     pObject2Bitmap->Transform(vertex[i].x, vertex[i].y);
     GetColor(vertex[i].r, vertex[i].g, vertex[i].b);
     m_BitStream.ByteAlign();
   }
   return TRUE;
 }
-CFX_FloatRect _GetShadingBBox(CPDF_Stream* pStream,
-                              int type,
-                              const CFX_AffineMatrix* pMatrix,
-                              CPDF_Function** pFuncs,
-                              int nFuncs,
-                              CPDF_ColorSpace* pCS) {
+CFX_FloatRect GetShadingBBox(CPDF_Stream* pStream,
+                             ShadingType type,
+                             const CFX_AffineMatrix* pMatrix,
+                             CPDF_Function** pFuncs,
+                             int nFuncs,
+                             CPDF_ColorSpace* pCS) {
   if (!pStream || !pStream->IsStream() || !pFuncs || !pCS)
     return CFX_FloatRect(0, 0, 0, 0);
 
   CPDF_MeshStream stream;
   if (!stream.Load(pStream, pFuncs, nFuncs, pCS))
     return CFX_FloatRect(0, 0, 0, 0);
 
   CFX_FloatRect rect;
   FX_BOOL bStarted = FALSE;
-  FX_BOOL bGouraud = type == 4 || type == 5;
-  int full_point_count = type == 7 ? 16 : (type == 6 ? 12 : 1);
-  int full_color_count = (type == 6 || type == 7) ? 4 : 1;
+  FX_BOOL bGouraud = type == kFreeFormGouraudTriangleMeshShading ||

[Tom Sepez, 2015/10/27 16:20:16]

nit: did clang-format do this?  sigh.  Maybe helper functions above with
switches e.g.

int PointCount(ShadingType type) {
  switch(type) {
    case kMumble:
      return 16;
    case kOther:
      return 12;
    default:
      return 1;
  }
}

[dsinclair, 2015/10/27 17:00:37]

How's this? I named the magic numbers and turned it into if()'s which I think
reads easier.

[Tom Sepez, 2015/10/27 17:04:25]

swell.

+                     type == kLatticeFormGouraudTriangleMeshShading;
+  int full_point_count = type == kTensorProductPatchMeshShading
+                             ? 16
+                             : (type == kCoonsPatchMeshShading ? 12 : 1);
+  int full_color_count =
+      (type == kCoonsPatchMeshShading || type == kTensorProductPatchMeshShading)
+          ? 4
+          : 1;
   while (!stream.m_BitStream.IsEOF()) {
     FX_DWORD flag = 0;
-    if (type != 5) {
+    if (type != kLatticeFormGouraudTriangleMeshShading)
       flag = stream.GetFlag();
-    }
+
     int point_count = full_point_count, color_count = full_color_count;

[Tom Sepez, 2015/10/27 16:20:16]

nit: do we need these locals?

[dsinclair, 2015/10/27 17:00:37]

Nope, did the math on the full_* versions and used those below.

     if (!bGouraud && flag) {
       point_count -= 4;
       color_count -= 2;
     }
     for (int i = 0; i < point_count; i++) {
       FX_FLOAT x, y;
       stream.GetCoords(x, y);
       if (bStarted) {
         rect.UpdateRect(x, y);
       } else {
         rect.InitRect(x, y);
         bStarted = TRUE;
       }
     }
     stream.m_BitStream.SkipBits(stream.m_nComps * stream.m_nCompBits *
                                 color_count);
     if (bGouraud)
       stream.m_BitStream.ByteAlign();
   }
   rect.Transform(pMatrix);
   return rect;
 }