On 17.01.2018 02:12, Ian Romanick wrote:
On 01/16/2018 10:11 AM, Ian Romanick wrote:
On 01/13/2018 01:47 PM, Jason Ekstrand wrote:
On January 12, 2018 14:56:26 "Ian Romanick" <i...@freedesktop.org> wrote:

From: Ian Romanick <ian.d.roman...@intel.com>

With the exception of NVIDIA hardware, these are is the values that all
hardware and Gallium want.  The remapping is currently implemented in at
least 6 places.  This starts the process of consolidating to a single
place.

Signed-off-by: Ian Romanick <ian.d.roman...@intel.com>
---
  src/mesa/main/blend.c  | 22 ++++++++++++++++++++++
  src/mesa/main/mtypes.h | 29 +++++++++++++++++++++++++++++
  2 files changed, 51 insertions(+)

diff --git a/src/mesa/main/blend.c b/src/mesa/main/blend.c
index 01721ab..f47b102 100644
--- a/src/mesa/main/blend.c
+++ b/src/mesa/main/blend.c
@@ -849,6 +849,26 @@ _mesa_AlphaFunc( GLenum func, GLclampf ref )
     }
  }

+static const enum color_logic_ops color_logicop_mapping[16] = {
+   COLOR_LOGICOP_CLEAR,
+   COLOR_LOGICOP_AND,
+   COLOR_LOGICOP_AND_REVERSE,
+   COLOR_LOGICOP_COPY,
+   COLOR_LOGICOP_AND_INVERTED,
+   COLOR_LOGICOP_NOOP,
+   COLOR_LOGICOP_XOR,
+   COLOR_LOGICOP_OR,
+   COLOR_LOGICOP_NOR,
+   COLOR_LOGICOP_EQUIV,
+   COLOR_LOGICOP_INVERT,
+   COLOR_LOGICOP_OR_REVERSE,
+   COLOR_LOGICOP_COPY_INVERTED,
+   COLOR_LOGICOP_OR_INVERTED,
+   COLOR_LOGICOP_NAND,
+   COLOR_LOGICOP_SET
+};
+
+#define GLenum_to_color_logicop(x) color_logicop_mapping[x & 0x0f]

Would it be better to make this a static inline so we can add a little
range check assert?

When I was experimenting with bit-reversal to implement this, I did
those implementations as functions.  I mostly did it as an #define for
compactness, but I'm not married to it.  There's only one user, and just
outside the diff context that user checks that the value is valid.
Since all 16 values after masking are valid, array accesses will always
be in bounds.

Now that I think about it... the only reason I did it as a #define was
so that I could easily transition to a different function or something.
After monkeying with it for... more time than I'll admit on the record,
I don't plan to pursue that.  The best option is probably to fold this
into the one user.

Thoughts?

Sounds reasonable to me.

Cheers,
Nicolai



  static ALWAYS_INLINE void
  logic_op(struct gl_context *ctx, GLenum opcode, bool no_error)
@@ -884,6 +904,7 @@ logic_op(struct gl_context *ctx, GLenum opcode,
bool no_error)
     FLUSH_VERTICES(ctx, ctx->DriverFlags.NewLogicOp ? 0 : _NEW_COLOR);
     ctx->NewDriverState |= ctx->DriverFlags.NewLogicOp;
     ctx->Color.LogicOp = opcode;
+   ctx->Color._LogicOp = GLenum_to_color_logicop(opcode);

     if (ctx->Driver.LogicOpcode)
        ctx->Driver.LogicOpcode(ctx, opcode);
@@ -1189,6 +1210,7 @@ void _mesa_init_color( struct gl_context * ctx )
     ctx->Color.IndexLogicOpEnabled = GL_FALSE;
     ctx->Color.ColorLogicOpEnabled = GL_FALSE;
     ctx->Color.LogicOp = GL_COPY;
+   ctx->Color._LogicOp = COLOR_LOGICOP_COPY;
     ctx->Color.DitherFlag = GL_TRUE;

     /* GL_FRONT is not possible on GLES. Instead GL_BACK will render
to either
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 226eb94..2fbfd27 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -418,6 +418,34 @@ union gl_color_union
     GLuint ui[4];
  };

+/**
+ * Remapped color logical operations
+ *
+ * With the exception of NVIDIA hardware, which consumes the OpenGL
enumerants
+ * directly, everything wants this mapping of color logical operations.
+ *
+ * Fun fact: These values are just the bit-reverse of the low-nibble
of the GL
+ * enumerant values (i.e., `GL_NOOP & 0x0f` is `b0101' while
+ * \c COLOR_LOGICOP_NOOP is `b1010`).
+ */
+enum PACKED color_logic_ops {
+   COLOR_LOGICOP_CLEAR = 0,
+   COLOR_LOGICOP_NOR = 1,
+   COLOR_LOGICOP_AND_INVERTED = 2,
+   COLOR_LOGICOP_COPY_INVERTED = 3,
+   COLOR_LOGICOP_AND_REVERSE = 4,
+   COLOR_LOGICOP_INVERT = 5,
+   COLOR_LOGICOP_XOR = 6,
+   COLOR_LOGICOP_NAND = 7,
+   COLOR_LOGICOP_AND = 8,
+   COLOR_LOGICOP_EQUIV = 9,
+   COLOR_LOGICOP_NOOP = 10,
+   COLOR_LOGICOP_OR_INVERTED = 11,
+   COLOR_LOGICOP_COPY = 12,
+   COLOR_LOGICOP_OR_REVERSE = 13,
+   COLOR_LOGICOP_OR = 14,
+   COLOR_LOGICOP_SET = 15
+};

  /**
   * Color buffer attribute group (GL_COLOR_BUFFER_BIT).
@@ -493,6 +521,7 @@ struct gl_colorbuffer_attrib
     GLboolean IndexLogicOpEnabled;    /**< Color index logic op
enabled flag */
     GLboolean ColorLogicOpEnabled;    /**< RGBA logic op enabled flag */
     GLenum LogicOp;            /**< Logic operator */
+   enum color_logic_ops _LogicOp;

     /*@}*/

--
2.9.5

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev



--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to