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?

>>>  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

Reply via email to