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