Use flat enum for PrimitiveMode.

scripts/run_code_generation_hashes.json

@@ -92,7 +92,7 @@
   "packed enum:src/common/packed_egl_enums.json":
     "5f591d220ee53b6e54a27d1523a3ab79",
   "packed enum:src/common/packed_gl_enums.json":
-    "7338a2a11c679aeb8efe2b415dbd4810",
+    "b54346c106ab4a7b0acb69eb65123c1a",
   "proc table:src/libGLESv2/gen_proc_table.py":
     "027bfd5a8a8dffe91f492bf199029cde",
   "proc table:src/libGLESv2/proc_table_data.json":

src/common/PackedEnums.h

@@ -74,6 +74,22 @@ class PackedEnumMap
     using Storage        = std::array<T, MaxSize>;
 
   public:
+    using InitPair = std::pair<E, T>;
+
+    constexpr PackedEnumMap() = default;
+
+    constexpr PackedEnumMap(std::initializer_list<InitPair> init) : mPrivateData{}
+    {
+        // We use a for loop instead of range-for to work around a limitation in MSVC.
+        for (const InitPair *it = init.begin(); it != init.end(); ++it)
+        {
+            // This horrible const_cast pattern is necessary to work around a constexpr limitation.
+            // See https://stackoverflow.com/q/34199774/ . Note that it should be fixed with C++17.
+            const_cast<T &>(const_cast<const Storage &>(
+                mPrivateData)[static_cast<UnderlyingType>(it->first)]) = it->second;
+        }
+    }
+
     // types:
     using value_type      = T;
     using pointer         = T *;
@@ -133,8 +149,8 @@ class PackedEnumMap
     T *data() noexcept { return mPrivateData.data(); }
     const T *data() const noexcept { return mPrivateData.data(); }
 
-    // Do not access this variable directly. It unfortunately must be public to use aggregate init.
-    /* private: */ Storage mPrivateData;
+  private:
+    Storage mPrivateData;
 };
 
 // PackedEnumBitSetE> is like an std::bitset<E::EnumCount> but is indexed with enum values. It
@@ -199,6 +215,59 @@ TextureType SamplerTypeToTextureType(GLenum samplerType);
 
 bool IsMultisampled(gl::TextureType type);
 
+enum class PrimitiveMode : uint8_t
+{
+    Points                 = 0x0,
+    Lines                  = 0x1,
+    LineLoop               = 0x2,
+    LineStrip              = 0x3,
+    Triangles              = 0x4,
+    TriangleStrip          = 0x5,
+    TriangleFan            = 0x6,
+    Unused1                = 0x7,
+    Unused2                = 0x8,
+    Unused3                = 0x9,
+    LinesAdjacency         = 0xA,
+    LineStripAdjacency     = 0xB,
+    TrianglesAdjacency     = 0xC,
+    TriangleStripAdjacency = 0xD,
+
+    InvalidEnum = 0xE,
+    EnumCount   = 0xE,
+};
+
+template <>
+constexpr PrimitiveMode FromGLenum<PrimitiveMode>(GLenum from)
+{
+    if (from >= static_cast<GLenum>(PrimitiveMode::EnumCount))
+    {
+        return PrimitiveMode::InvalidEnum;
+    }
+
+    return static_cast<PrimitiveMode>(from);
+}
+
+constexpr GLenum ToGLenum(PrimitiveMode from)
+{
+    return static_cast<GLenum>(from);
+}
+
+static_assert(ToGLenum(PrimitiveMode::Points) == GL_POINTS, "PrimitiveMode violation");
+static_assert(ToGLenum(PrimitiveMode::Lines) == GL_LINES, "PrimitiveMode violation");
+static_assert(ToGLenum(PrimitiveMode::LineLoop) == GL_LINE_LOOP, "PrimitiveMode violation");
+static_assert(ToGLenum(PrimitiveMode::LineStrip) == GL_LINE_STRIP, "PrimitiveMode violation");
+static_assert(ToGLenum(PrimitiveMode::Triangles) == GL_TRIANGLES, "PrimitiveMode violation");
+static_assert(ToGLenum(PrimitiveMode::TriangleStrip) == GL_TRIANGLE_STRIP,
+              "PrimitiveMode violation");
+static_assert(ToGLenum(PrimitiveMode::TriangleFan) == GL_TRIANGLE_FAN, "PrimitiveMode violation");
+static_assert(ToGLenum(PrimitiveMode::LinesAdjacency) == GL_LINES_ADJACENCY,
+              "PrimitiveMode violation");
+static_assert(ToGLenum(PrimitiveMode::LineStripAdjacency) == GL_LINE_STRIP_ADJACENCY,
+              "PrimitiveMode violation");
+static_assert(ToGLenum(PrimitiveMode::TrianglesAdjacency) == GL_TRIANGLES_ADJACENCY,
+              "PrimitiveMode violation");
+static_assert(ToGLenum(PrimitiveMode::TriangleStripAdjacency) == GL_TRIANGLE_STRIP_ADJACENCY,
+              "PrimitiveMode violation");
 }  // namespace gl
 
 namespace egl

src/common/PackedGLEnums_autogen.cpp

@@ -628,70 +628,6 @@ GLenum ToGLenum(PointParameter from)
 }
 
 template <>
-PrimitiveMode FromGLenum<PrimitiveMode>(GLenum from)
-{
-    switch (from)
-    {
-        case GL_POINTS:
-            return PrimitiveMode::Points;
-        case GL_LINES:
-            return PrimitiveMode::Lines;
-        case GL_LINE_LOOP:
-            return PrimitiveMode::LineLoop;
-        case GL_LINE_STRIP:
-            return PrimitiveMode::LineStrip;
-        case GL_TRIANGLES:
-            return PrimitiveMode::Triangles;
-        case GL_TRIANGLE_STRIP:
-            return PrimitiveMode::TriangleStrip;
-        case GL_TRIANGLE_FAN:
-            return PrimitiveMode::TriangleFan;
-        case GL_LINES_ADJACENCY_EXT:
-            return PrimitiveMode::LinesAdjacency;
-        case GL_LINE_STRIP_ADJACENCY_EXT:
-            return PrimitiveMode::LineStripAdjacency;
-        case GL_TRIANGLES_ADJACENCY_EXT:
-            return PrimitiveMode::TrianglesAdjacency;
-        case GL_TRIANGLE_STRIP_ADJACENCY_EXT:
-            return PrimitiveMode::TriangleStripAdjacency;
-        default:
-            return PrimitiveMode::InvalidEnum;
-    }
-}
-
-GLenum ToGLenum(PrimitiveMode from)
-{
-    switch (from)
-    {
-        case PrimitiveMode::Points:
-            return GL_POINTS;
-        case PrimitiveMode::Lines:
-            return GL_LINES;
-        case PrimitiveMode::LineLoop:
-            return GL_LINE_LOOP;
-        case PrimitiveMode::LineStrip:
-            return GL_LINE_STRIP;
-        case PrimitiveMode::Triangles:
-            return GL_TRIANGLES;
-        case PrimitiveMode::TriangleStrip:
-            return GL_TRIANGLE_STRIP;
-        case PrimitiveMode::TriangleFan:
-            return GL_TRIANGLE_FAN;
-        case PrimitiveMode::LinesAdjacency:
-            return GL_LINES_ADJACENCY_EXT;
-        case PrimitiveMode::LineStripAdjacency:
-            return GL_LINE_STRIP_ADJACENCY_EXT;
-        case PrimitiveMode::TrianglesAdjacency:
-            return GL_TRIANGLES_ADJACENCY_EXT;
-        case PrimitiveMode::TriangleStripAdjacency:
-            return GL_TRIANGLE_STRIP_ADJACENCY_EXT;
-        default:
-            UNREACHABLE();
-            return 0;
-    }
-}
-
-template <>
 QueryType FromGLenum<QueryType>(GLenum from)
 {
     switch (from)

src/common/PackedGLEnums_autogen.h

@@ -255,28 +255,6 @@ template <>
 PointParameter FromGLenum<PointParameter>(GLenum from);
 GLenum ToGLenum(PointParameter from);
 
-enum class PrimitiveMode : uint8_t
-{
-    Points                 = 0,
-    Lines                  = 1,
-    LineLoop               = 2,
-    LineStrip              = 3,
-    Triangles              = 4,
-    TriangleStrip          = 5,
-    TriangleFan            = 6,
-    LinesAdjacency         = 7,
-    LineStripAdjacency     = 8,
-    TrianglesAdjacency     = 9,
-    TriangleStripAdjacency = 10,
-
-    InvalidEnum = 11,
-    EnumCount   = 11,
-};
-
-template <>
-PrimitiveMode FromGLenum<PrimitiveMode>(GLenum from);
-GLenum ToGLenum(PrimitiveMode from);
-
 enum class QueryType : uint8_t
 {
     AnySamples                         = 0,

src/common/packed_gl_enums.json

@@ -229,20 +229,6 @@
         "SpotDirection": "GL_SPOT_DIRECTION",
         "SpotExponent": "GL_SPOT_EXPONENT"
     },
-    "PrimitiveMode":
-    {
-        "Points": "GL_POINTS",
-        "Lines": "GL_LINES",
-        "LineLoop": "GL_LINE_LOOP",
-        "LineStrip": "GL_LINE_STRIP",
-        "Triangles": "GL_TRIANGLES",
-        "TriangleStrip": "GL_TRIANGLE_STRIP",
-        "TriangleFan": "GL_TRIANGLE_FAN",
-        "LinesAdjacency": "GL_LINES_ADJACENCY_EXT",
-        "LineStripAdjacency": "GL_LINE_STRIP_ADJACENCY_EXT",
-        "TrianglesAdjacency": "GL_TRIANGLES_ADJACENCY_EXT",
-        "TriangleStripAdjacency": "GL_TRIANGLE_STRIP_ADJACENCY_EXT"
-    },
     "PointParameter":
     {
         "PointSizeMin" : "GL_POINT_SIZE_MIN",

src/libANGLE/State.cpp

@@ -50,26 +50,22 @@ bool GetAlternativeQueryType(QueryType type, QueryType *alternativeType)
 
 // Mapping from a buffer binding type to a dirty bit type.
 constexpr angle::PackedEnumMap<BufferBinding, size_t> kBufferBindingDirtyBits = {{
-    0,                                                 /* Array */
-    State::DIRTY_BIT_ATOMIC_COUNTER_BUFFER_BINDING,    /* AtomicCounter */
-    0,                                                 /* CopyRead */
-    0,                                                 /* CopyWrite */
-    State::DIRTY_BIT_DISPATCH_INDIRECT_BUFFER_BINDING, /* DispatchIndirect */
-    State::DIRTY_BIT_DRAW_INDIRECT_BUFFER_BINDING,     /* DrawIndirect */
-    0,                                                 /* ElementArray */
-    State::DIRTY_BIT_PACK_BUFFER_BINDING,              /* PixelPack */
-    State::DIRTY_BIT_UNPACK_BUFFER_BINDING,            /* PixelUnpack */
-    State::DIRTY_BIT_SHADER_STORAGE_BUFFER_BINDING,    /* ShaderStorage */
-    0,                                                 /* TransformFeedback */
-    State::DIRTY_BIT_UNIFORM_BUFFER_BINDINGS,          /* Uniform */
+    {BufferBinding::AtomicCounter, State::DIRTY_BIT_ATOMIC_COUNTER_BUFFER_BINDING},
+    {BufferBinding::DispatchIndirect, State::DIRTY_BIT_DISPATCH_INDIRECT_BUFFER_BINDING},
+    {BufferBinding::DrawIndirect, State::DIRTY_BIT_DRAW_INDIRECT_BUFFER_BINDING},
+    {BufferBinding::PixelPack, State::DIRTY_BIT_PACK_BUFFER_BINDING},
+    {BufferBinding::PixelUnpack, State::DIRTY_BIT_UNPACK_BUFFER_BINDING},
+    {BufferBinding::ShaderStorage, State::DIRTY_BIT_SHADER_STORAGE_BUFFER_BINDING},
+    {BufferBinding::Uniform, State::DIRTY_BIT_UNIFORM_BUFFER_BINDINGS},
 }};
 
 // Returns a buffer binding function depending on if a dirty bit is set.
 template <BufferBinding Target>
-constexpr State::BufferBindingSetter GetBufferBindingSetter()
+constexpr std::pair<BufferBinding, State::BufferBindingSetter> GetBufferBindingSetter()
 {
-    return kBufferBindingDirtyBits[Target] != 0 ? &State::setGenericBufferBindingWithBit<Target>
-                                                : &State::setGenericBufferBinding<Target>;
+    return std::make_pair(Target, kBufferBindingDirtyBits[Target] != 0
+                                      ? &State::setGenericBufferBindingWithBit<Target>
+                                      : &State::setGenericBufferBinding<Target>);
 }
 }  // namespace
 

src/tests/gl_tests/SimpleOperationTest.cpp

@@ -1070,6 +1070,36 @@ TEST_P(SimpleOperationTest, RenderbufferAttachment)
     EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
 }
 
+// Tests that using desktop GL_QUADS/GL_POLYGONS enums generate the correct error.
+TEST_P(SimpleOperationTest, PrimitiveModeNegativeTest)
+{
+    // Draw a correct quad.
+    ANGLE_GL_PROGRAM(program, kBasicVertexShader, kGreenFragmentShader);
+    glUseProgram(program);
+
+    GLint positionLocation = glGetAttribLocation(program, "position");
+    ASSERT_NE(-1, positionLocation);
+
+    setupQuadVertexBuffer(0.5f, 1.0f);
+    glVertexAttribPointer(positionLocation, 3, GL_FLOAT, GL_FALSE, 0, 0);
+    glEnableVertexAttribArray(positionLocation);
+
+    // Tests that TRIANGLES works.
+    glDrawArrays(GL_TRIANGLES, 0, 6);
+    ASSERT_GL_NO_ERROR();
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
+
+    // Tests that specific invalid enums don't work.
+    glDrawArrays(static_cast<GLenum>(7), 0, 6);
+    EXPECT_GL_ERROR(GL_INVALID_ENUM);
+
+    glDrawArrays(static_cast<GLenum>(8), 0, 6);
+    EXPECT_GL_ERROR(GL_INVALID_ENUM);
+
+    glDrawArrays(static_cast<GLenum>(9), 0, 6);
+    EXPECT_GL_ERROR(GL_INVALID_ENUM);
+}
+
 // Use this to select which configurations (e.g. which renderer, which GLES major version) these
 // tests should be run against.
 ANGLE_INSTANTIATE_TEST(SimpleOperationTest,