[Mesa-dev] [PATCH 01/10] mesa: Also track a remapped version of the color logic op
From: Ian Romanick 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 --- 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] 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
Re: [Mesa-dev] [PATCH 01/10] mesa: Also track a remapped version of the color logic op
On January 12, 2018 14:56:26 "Ian Romanick" wrote: From: Ian Romanick 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 --- 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? 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
Re: [Mesa-dev] [PATCH 01/10] mesa: Also track a remapped version of the color logic op
On 01/12/2018 03:56 PM, Ian Romanick wrote: From: Ian Romanick 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 --- 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] 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 { A name such as gl_logicop_mode would be more in line with other mesa enum types. + 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; /*@}*/ For patches 1,3,4, Reviewed-by: Brian Paul ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 01/10] mesa: Also track a remapped version of the color logic op
On 01/13/2018 01:47 PM, Jason Ekstrand wrote: > On January 12, 2018 14:56:26 "Ian Romanick" wrote: > >> From: Ian Romanick >> >> 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 >> --- >> 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. >> 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
Re: [Mesa-dev] [PATCH 01/10] mesa: Also track a remapped version of the color logic op
On 01/15/2018 11:23 AM, Brian Paul wrote: > On 01/12/2018 03:56 PM, Ian Romanick wrote: >> From: Ian Romanick >> >> 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 >> --- >> 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] >> 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 { > > A name such as gl_logicop_mode would be more in line with other mesa > enum types. I thought about that, but I thought the "gl_" prefix could cause some confusion since these are the "not GL" enums. I can change it if you like. >> + 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; >> /*@}*/ >> > > For patches 1,3,4, Reviewed-by: Brian Paul > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 01/10] mesa: Also track a remapped version of the color logic op
On 01/16/2018 11:12 AM, Ian Romanick wrote: On 01/15/2018 11:23 AM, Brian Paul wrote: On 01/12/2018 03:56 PM, Ian Romanick wrote: From: Ian Romanick 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 --- 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] 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 { A name such as gl_logicop_mode would be more in line with other mesa enum types. I thought about that, but I thought the "gl_" prefix could cause some confusion since these are the "not GL" enums. I can change it if you like. I'd prefer you change it. If you look around, we have lots of other enums like that (ex: gl_fog_mode, gl_tex_env_mode, gl_tex_env_source, gl_vert_attrib, gl_varying_slot, etc). Maybe they should all be "mesa_foo" but it is what it is. Thanks. -Brian + 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; /*@}*/ For patches 1,3,4, Reviewed-by: Brian Paul ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 01/10] mesa: Also track a remapped version of the color logic op
On 01/16/2018 10:59 AM, Brian Paul wrote: > On 01/16/2018 11:12 AM, Ian Romanick wrote: >> On 01/15/2018 11:23 AM, Brian Paul wrote: >>> On 01/12/2018 03:56 PM, Ian Romanick wrote: From: Ian Romanick 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 --- 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] 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 { >>> >>> A name such as gl_logicop_mode would be more in line with other mesa >>> enum types. >> >> I thought about that, but I thought the "gl_" prefix could cause some >> confusion since these are the "not GL" enums. I can change it if you >> like. > > I'd prefer you change it. If you look around, we have lots of other > enums like that (ex: gl_fog_mode, gl_tex_env_mode, gl_tex_env_source, > gl_vert_attrib, gl_varying_slot, etc). Maybe they should all be > "mesa_foo" but it is what it is. That makes sense. I've changed it locally. > Thanks. > > -Brian > >> + 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; /*@}*/ >>> >>> For patches 1,3,4, Reviewed-by: Brian Paul ___ mesa-dev mai
Re: [Mesa-dev] [PATCH 01/10] mesa: Also track a remapped version of the color logic op
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" wrote: >> >>> From: Ian Romanick >>> >>> 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 >>> --- >>> 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 m
Re: [Mesa-dev] [PATCH 01/10] mesa: Also track a remapped version of the color logic op
On 12.01.2018 23:56, Ian Romanick wrote: From: Ian Romanick 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 --- 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] 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`). Fun fact #2: These values are simply a natural encoding of the operation as a table of bit values. The result of the logic op is: result_bit = (logic_op >> (2 * src_bit + dst_bit)) & 1 That OpenGL has them reversed is really quite odd. I'd love to hear the history behind it. (I think it might be useful to add this information in the comment, but I'll leave that decision up to you.) Patches 1, 3, & the core + radeon parts of 4: Reviewed-by: Nicolai Hähnle + */ +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; /*@}*/ -- 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
Re: [Mesa-dev] [PATCH 01/10] mesa: Also track a remapped version of the color logic op
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" wrote: From: Ian Romanick 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 --- 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 ht
Re: [Mesa-dev] [PATCH 01/10] mesa: Also track a remapped version of the color logic op
On 01/17/2018 03:53 AM, Nicolai Hähnle wrote: > On 12.01.2018 23:56, Ian Romanick wrote: >> From: Ian Romanick >> >> 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 >> --- >> 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] >> 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`). > > Fun fact #2: These values are simply a natural encoding of the operation > as a table of bit values. The result of the logic op is: > > result_bit = (logic_op >> (2 * src_bit + dst_bit)) & 1 Oh interesting. > That OpenGL has them reversed is really quite odd. I'd love to hear the > history behind it. Big endian vs little endian perhaps? > (I think it might be useful to add this information in the comment, but > I'll leave that decision up to you.) > > Patches 1, 3, & the core + radeon parts of 4: > > Reviewed-by: Nicolai Hähnle > > >> + */ >> +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; >> /*@}*/ >> ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 01/10] mesa: Also track a remapped version of the color logic op
On 01/17/2018 08:42 AM, Ian Romanick wrote: > On 01/17/2018 03:53 AM, Nicolai Hähnle wrote: >> Fun fact #2: These values are simply a natural encoding of the operation >> as a table of bit values. The result of the logic op is: >> >> result_bit = (logic_op >> (2 * src_bit + dst_bit)) & 1 > > Oh interesting. > >> That OpenGL has them reversed is really quite odd. I'd love to hear the >> history behind it. > > Big endian vs little endian perhaps? Actually... it's not even that. It's just a different equation for the same thing: result_bit = logic_op & (1 << (2 * src_bit + dst_bit)) Everybody learns something new today. :) >> (I think it might be useful to add this information in the comment, but >> I'll leave that decision up to you.) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev