Re: [Mesa-dev] [PATCH 1/6] gallium: use enums in p_defines.h

2016-04-17 Thread Marek Olšák
On Sun, Apr 17, 2016 at 1:22 AM, Emil Velikov  wrote:
> On 16 April 2016 at 22:04, Marek Olšák  wrote:
>> On Sat, Apr 16, 2016 at 9:40 PM, Rob Clark  wrote:
>>> On Sat, Apr 16, 2016 at 8:50 AM, Marek Olšák  wrote:
 From: Marek Olšák 

 and remove number assignments which are consecutive
 ---
  src/gallium/include/pipe/p_defines.h | 378 
 +++
  1 file changed, 205 insertions(+), 173 deletions(-)

 diff --git a/src/gallium/include/pipe/p_defines.h 
 b/src/gallium/include/pipe/p_defines.h
 index 1aef21d..6bb180d 100644
 --- a/src/gallium/include/pipe/p_defines.h
 +++ b/src/gallium/include/pipe/p_defines.h
 @@ -51,49 +51,56 @@ enum pipe_error
 /* TODO */
  };

 +enum {
>>>
>>> so, I would kinda like to use named enums, and then update the state
>>> structs to use 'em (since it would make gcc and gdb grok them
>>> better).. ofc it is a big change and doesn't have to be done in all
>>> one go, but why not give the enum's names in this first step?
>>
>> Enum type names can't be used everywhere. A lot of states are packed
>> (unsigned x:n). Not sure how enums work with that. In any case, let's
>> add the names later if needed.
>>
> Please name the new enums. All of the ones in gallium/include have names.

If somebody wants to name them, it can be done in another patch. Right
now, no names are needed to compile it.

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


Re: [Mesa-dev] [PATCH 1/6] gallium: use enums in p_defines.h

2016-04-17 Thread Jose Fonseca

On 16/04/16 22:51, Rob Clark wrote:

On Sat, Apr 16, 2016 at 5:49 PM, Rob Clark  wrote:

On Sat, Apr 16, 2016 at 5:04 PM, Marek Olšák  wrote:

On Sat, Apr 16, 2016 at 9:40 PM, Rob Clark  wrote:

On Sat, Apr 16, 2016 at 8:50 AM, Marek Olšák  wrote:

From: Marek Olšák 

and remove number assignments which are consecutive
---
  src/gallium/include/pipe/p_defines.h | 378 +++
  1 file changed, 205 insertions(+), 173 deletions(-)

diff --git a/src/gallium/include/pipe/p_defines.h 
b/src/gallium/include/pipe/p_defines.h
index 1aef21d..6bb180d 100644
--- a/src/gallium/include/pipe/p_defines.h
+++ b/src/gallium/include/pipe/p_defines.h
@@ -51,49 +51,56 @@ enum pipe_error
 /* TODO */
  };

+enum {


so, I would kinda like to use named enums, and then update the state
structs to use 'em (since it would make gcc and gdb grok them
better).. ofc it is a big change and doesn't have to be done in all
one go, but why not give the enum's names in this first step?


Enum type names can't be used everywhere. A lot of states are packed
(unsigned x:n). Not sure how enums work with that. In any case, let's
add the names later if needed.


I *believe* 'enum foo name : n' works.. it has been used in other
places, but I didn't verify that the structure layout was the same
(and I'm not 100% sure about msvc, etc)..


btw, to be clear, I wasn't proposing to change all the state structs
as well in the same patch.. I don't believe it would be a problem to
give enum names now and leave "unsigned foo : n" for now..


Right.  Changing the state members might not be possible, but having 
enum names so we can use in intermediate variables would already be useful.


Eitherway, thanks for doing this Marek.  It's a nice cleanup.

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


Re: [Mesa-dev] [PATCH 1/6] gallium: use enums in p_defines.h

2016-04-16 Thread Emil Velikov
On 16 April 2016 at 22:04, Marek Olšák  wrote:
> On Sat, Apr 16, 2016 at 9:40 PM, Rob Clark  wrote:
>> On Sat, Apr 16, 2016 at 8:50 AM, Marek Olšák  wrote:
>>> From: Marek Olšák 
>>>
>>> and remove number assignments which are consecutive
>>> ---
>>>  src/gallium/include/pipe/p_defines.h | 378 
>>> +++
>>>  1 file changed, 205 insertions(+), 173 deletions(-)
>>>
>>> diff --git a/src/gallium/include/pipe/p_defines.h 
>>> b/src/gallium/include/pipe/p_defines.h
>>> index 1aef21d..6bb180d 100644
>>> --- a/src/gallium/include/pipe/p_defines.h
>>> +++ b/src/gallium/include/pipe/p_defines.h
>>> @@ -51,49 +51,56 @@ enum pipe_error
>>> /* TODO */
>>>  };
>>>
>>> +enum {
>>
>> so, I would kinda like to use named enums, and then update the state
>> structs to use 'em (since it would make gcc and gdb grok them
>> better).. ofc it is a big change and doesn't have to be done in all
>> one go, but why not give the enum's names in this first step?
>
> Enum type names can't be used everywhere. A lot of states are packed
> (unsigned x:n). Not sure how enums work with that. In any case, let's
> add the names later if needed.
>
Please name the new enums. All of the ones in gallium/include have names.

On the topic of using the enums in the structs (and passing them
around in the functions) yes that is a very good idea imho. Although
could/should happen in the long term. Because a) one has to explicitly
pack them and b) one has to be wary of buggy GCC [1] [2].

Note: i965 recently started using packed enums, so we might want to
check exactly when that happened and bump the requirement to GCC
4.3.6+ as per the second bug report.

-Emil

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=14124
[2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=39219
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/6] gallium: use enums in p_defines.h

2016-04-16 Thread Roland Scheidegger
Makes me wonder why we didn't use enums in the first place.
But I can't think of any disadvantages.

Reviewed-by: Roland Scheidegger 

Am 16.04.2016 um 14:50 schrieb Marek Olšák:
> From: Marek Olšák 
> 
> and remove number assignments which are consecutive
> ---
>  src/gallium/include/pipe/p_defines.h | 378 
> +++
>  1 file changed, 205 insertions(+), 173 deletions(-)
> 
> diff --git a/src/gallium/include/pipe/p_defines.h 
> b/src/gallium/include/pipe/p_defines.h
> index 1aef21d..6bb180d 100644
> --- a/src/gallium/include/pipe/p_defines.h
> +++ b/src/gallium/include/pipe/p_defines.h
> @@ -51,49 +51,56 @@ enum pipe_error
> /* TODO */
>  };
>  
> +enum {
> +   PIPE_BLENDFACTOR_ONE = 1,
> +   PIPE_BLENDFACTOR_SRC_COLOR,
> +   PIPE_BLENDFACTOR_SRC_ALPHA,
> +   PIPE_BLENDFACTOR_DST_ALPHA,
> +   PIPE_BLENDFACTOR_DST_COLOR,
> +   PIPE_BLENDFACTOR_SRC_ALPHA_SATURATE,
> +   PIPE_BLENDFACTOR_CONST_COLOR,
> +   PIPE_BLENDFACTOR_CONST_ALPHA,
> +   PIPE_BLENDFACTOR_SRC1_COLOR,
> +   PIPE_BLENDFACTOR_SRC1_ALPHA,
> +
> +   PIPE_BLENDFACTOR_ZERO = 0x11,
> +   PIPE_BLENDFACTOR_INV_SRC_COLOR,
> +   PIPE_BLENDFACTOR_INV_SRC_ALPHA,
> +   PIPE_BLENDFACTOR_INV_DST_ALPHA,
> +   PIPE_BLENDFACTOR_INV_DST_COLOR,
> +
> +   PIPE_BLENDFACTOR_INV_CONST_COLOR = 0x17,
> +   PIPE_BLENDFACTOR_INV_CONST_ALPHA,
> +   PIPE_BLENDFACTOR_INV_SRC1_COLOR,
> +   PIPE_BLENDFACTOR_INV_SRC1_ALPHA,
> +};
> +
> +enum {
> +   PIPE_BLEND_ADD,
> +   PIPE_BLEND_SUBTRACT,
> +   PIPE_BLEND_REVERSE_SUBTRACT,
> +   PIPE_BLEND_MIN,
> +   PIPE_BLEND_MAX,
> +};
>  
> -#define PIPE_BLENDFACTOR_ONE 0x1
> -#define PIPE_BLENDFACTOR_SRC_COLOR   0x2
> -#define PIPE_BLENDFACTOR_SRC_ALPHA   0x3
> -#define PIPE_BLENDFACTOR_DST_ALPHA   0x4
> -#define PIPE_BLENDFACTOR_DST_COLOR   0x5
> -#define PIPE_BLENDFACTOR_SRC_ALPHA_SATURATE  0x6
> -#define PIPE_BLENDFACTOR_CONST_COLOR 0x7
> -#define PIPE_BLENDFACTOR_CONST_ALPHA 0x8
> -#define PIPE_BLENDFACTOR_SRC1_COLOR  0x9
> -#define PIPE_BLENDFACTOR_SRC1_ALPHA  0x0A
> -#define PIPE_BLENDFACTOR_ZERO0x11
> -#define PIPE_BLENDFACTOR_INV_SRC_COLOR   0x12
> -#define PIPE_BLENDFACTOR_INV_SRC_ALPHA   0x13
> -#define PIPE_BLENDFACTOR_INV_DST_ALPHA   0x14
> -#define PIPE_BLENDFACTOR_INV_DST_COLOR   0x15
> -#define PIPE_BLENDFACTOR_INV_CONST_COLOR 0x17
> -#define PIPE_BLENDFACTOR_INV_CONST_ALPHA 0x18
> -#define PIPE_BLENDFACTOR_INV_SRC1_COLOR  0x19
> -#define PIPE_BLENDFACTOR_INV_SRC1_ALPHA  0x1A
> -
> -#define PIPE_BLEND_ADD   0
> -#define PIPE_BLEND_SUBTRACT  1
> -#define PIPE_BLEND_REVERSE_SUBTRACT  2
> -#define PIPE_BLEND_MIN   3
> -#define PIPE_BLEND_MAX   4
> -
> -#define PIPE_LOGICOP_CLEAR0
> -#define PIPE_LOGICOP_NOR  1
> -#define PIPE_LOGICOP_AND_INVERTED 2
> -#define PIPE_LOGICOP_COPY_INVERTED3
> -#define PIPE_LOGICOP_AND_REVERSE  4
> -#define PIPE_LOGICOP_INVERT   5
> -#define PIPE_LOGICOP_XOR  6
> -#define PIPE_LOGICOP_NAND 7
> -#define PIPE_LOGICOP_AND  8
> -#define PIPE_LOGICOP_EQUIV9
> -#define PIPE_LOGICOP_NOOP 10
> -#define PIPE_LOGICOP_OR_INVERTED  11
> -#define PIPE_LOGICOP_COPY 12
> -#define PIPE_LOGICOP_OR_REVERSE   13
> -#define PIPE_LOGICOP_OR   14
> -#define PIPE_LOGICOP_SET  15  
> +enum {
> +   PIPE_LOGICOP_CLEAR,
> +   PIPE_LOGICOP_NOR,
> +   PIPE_LOGICOP_AND_INVERTED,
> +   PIPE_LOGICOP_COPY_INVERTED,
> +   PIPE_LOGICOP_AND_REVERSE,
> +   PIPE_LOGICOP_INVERT,
> +   PIPE_LOGICOP_XOR,
> +   PIPE_LOGICOP_NAND,
> +   PIPE_LOGICOP_AND,
> +   PIPE_LOGICOP_EQUIV,
> +   PIPE_LOGICOP_NOOP,
> +   PIPE_LOGICOP_OR_INVERTED,
> +   PIPE_LOGICOP_COPY,
> +   PIPE_LOGICOP_OR_REVERSE,
> +   PIPE_LOGICOP_OR,
> +   PIPE_LOGICOP_SET,
> +};
>  
>  #define PIPE_MASK_R  0x1
>  #define PIPE_MASK_G  0x2
> @@ -110,19 +117,23 @@ enum pipe_error
>   * Inequality functions.  Used for depth test, stencil compare, alpha
>   * test, shadow compare, etc.
>   */
> -#define PIPE_FUNC_NEVER0
> -#define PIPE_FUNC_LESS 1
> -#define PIPE_FUNC_EQUAL2
> -#define PIPE_FUNC_LEQUAL   3
> -#define PIPE_FUNC_GREATER  4
> -#define PIPE_FUNC_NOTEQUAL 5
> -#define PIPE_FUNC_GEQUAL   6
> -#define PIPE_FUNC_ALWAYS   7
> +enum {
> +   PIPE_FUNC_NEVER,
> +   PIPE_FUNC_LESS,
> +   PIPE_FUNC_EQUAL,
> +   PIPE_FUNC_LEQUAL,
> +   PIPE_FUNC_GREATER,
> +   PIPE_FUNC_NOTEQUAL,
> +   PIPE_FUNC_GEQUAL,
> +   PIPE_FUNC_ALWAYS,
> +};
>  
>  /** Polygon fill mode */
> -#define PIPE_POLYGON_MODE_FILL  0
> -#define PIPE_POLYGON_MODE_LINE  1
> -#define PIPE_POLYGON_MODE_POINT 2
> +enum {
> +   PIPE_POLYGON_MODE_FILL,
> +   PIPE_POLYGON_MODE_LINE,
> +   PIPE_POLYGON_MODE_POINT,
> +};
>  
>  /** Polygon face specification, eg for culling */
>  #define PIPE_FACE_NONE   0
> @@ -131,60 +142,72 @@ enum pipe_error
>  #de

Re: [Mesa-dev] [PATCH 1/6] gallium: use enums in p_defines.h

2016-04-16 Thread Rob Clark
On Sat, Apr 16, 2016 at 5:49 PM, Rob Clark  wrote:
> On Sat, Apr 16, 2016 at 5:04 PM, Marek Olšák  wrote:
>> On Sat, Apr 16, 2016 at 9:40 PM, Rob Clark  wrote:
>>> On Sat, Apr 16, 2016 at 8:50 AM, Marek Olšák  wrote:
 From: Marek Olšák 

 and remove number assignments which are consecutive
 ---
  src/gallium/include/pipe/p_defines.h | 378 
 +++
  1 file changed, 205 insertions(+), 173 deletions(-)

 diff --git a/src/gallium/include/pipe/p_defines.h 
 b/src/gallium/include/pipe/p_defines.h
 index 1aef21d..6bb180d 100644
 --- a/src/gallium/include/pipe/p_defines.h
 +++ b/src/gallium/include/pipe/p_defines.h
 @@ -51,49 +51,56 @@ enum pipe_error
 /* TODO */
  };

 +enum {
>>>
>>> so, I would kinda like to use named enums, and then update the state
>>> structs to use 'em (since it would make gcc and gdb grok them
>>> better).. ofc it is a big change and doesn't have to be done in all
>>> one go, but why not give the enum's names in this first step?
>>
>> Enum type names can't be used everywhere. A lot of states are packed
>> (unsigned x:n). Not sure how enums work with that. In any case, let's
>> add the names later if needed.
>
> I *believe* 'enum foo name : n' works.. it has been used in other
> places, but I didn't verify that the structure layout was the same
> (and I'm not 100% sure about msvc, etc)..

btw, to be clear, I wasn't proposing to change all the state structs
as well in the same patch.. I don't believe it would be a problem to
give enum names now and leave "unsigned foo : n" for now..

BR,
-R


> BR,
> -R
>
>
>>
>> Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/6] gallium: use enums in p_defines.h

2016-04-16 Thread Rob Clark
On Sat, Apr 16, 2016 at 5:04 PM, Marek Olšák  wrote:
> On Sat, Apr 16, 2016 at 9:40 PM, Rob Clark  wrote:
>> On Sat, Apr 16, 2016 at 8:50 AM, Marek Olšák  wrote:
>>> From: Marek Olšák 
>>>
>>> and remove number assignments which are consecutive
>>> ---
>>>  src/gallium/include/pipe/p_defines.h | 378 
>>> +++
>>>  1 file changed, 205 insertions(+), 173 deletions(-)
>>>
>>> diff --git a/src/gallium/include/pipe/p_defines.h 
>>> b/src/gallium/include/pipe/p_defines.h
>>> index 1aef21d..6bb180d 100644
>>> --- a/src/gallium/include/pipe/p_defines.h
>>> +++ b/src/gallium/include/pipe/p_defines.h
>>> @@ -51,49 +51,56 @@ enum pipe_error
>>> /* TODO */
>>>  };
>>>
>>> +enum {
>>
>> so, I would kinda like to use named enums, and then update the state
>> structs to use 'em (since it would make gcc and gdb grok them
>> better).. ofc it is a big change and doesn't have to be done in all
>> one go, but why not give the enum's names in this first step?
>
> Enum type names can't be used everywhere. A lot of states are packed
> (unsigned x:n). Not sure how enums work with that. In any case, let's
> add the names later if needed.

I *believe* 'enum foo name : n' works.. it has been used in other
places, but I didn't verify that the structure layout was the same
(and I'm not 100% sure about msvc, etc)..

BR,
-R


>
> Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/6] gallium: use enums in p_defines.h

2016-04-16 Thread Marek Olšák
On Sat, Apr 16, 2016 at 9:40 PM, Rob Clark  wrote:
> On Sat, Apr 16, 2016 at 8:50 AM, Marek Olšák  wrote:
>> From: Marek Olšák 
>>
>> and remove number assignments which are consecutive
>> ---
>>  src/gallium/include/pipe/p_defines.h | 378 
>> +++
>>  1 file changed, 205 insertions(+), 173 deletions(-)
>>
>> diff --git a/src/gallium/include/pipe/p_defines.h 
>> b/src/gallium/include/pipe/p_defines.h
>> index 1aef21d..6bb180d 100644
>> --- a/src/gallium/include/pipe/p_defines.h
>> +++ b/src/gallium/include/pipe/p_defines.h
>> @@ -51,49 +51,56 @@ enum pipe_error
>> /* TODO */
>>  };
>>
>> +enum {
>
> so, I would kinda like to use named enums, and then update the state
> structs to use 'em (since it would make gcc and gdb grok them
> better).. ofc it is a big change and doesn't have to be done in all
> one go, but why not give the enum's names in this first step?

Enum type names can't be used everywhere. A lot of states are packed
(unsigned x:n). Not sure how enums work with that. In any case, let's
add the names later if needed.

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


Re: [Mesa-dev] [PATCH 1/6] gallium: use enums in p_defines.h

2016-04-16 Thread Rob Clark
On Sat, Apr 16, 2016 at 8:50 AM, Marek Olšák  wrote:
> From: Marek Olšák 
>
> and remove number assignments which are consecutive
> ---
>  src/gallium/include/pipe/p_defines.h | 378 
> +++
>  1 file changed, 205 insertions(+), 173 deletions(-)
>
> diff --git a/src/gallium/include/pipe/p_defines.h 
> b/src/gallium/include/pipe/p_defines.h
> index 1aef21d..6bb180d 100644
> --- a/src/gallium/include/pipe/p_defines.h
> +++ b/src/gallium/include/pipe/p_defines.h
> @@ -51,49 +51,56 @@ enum pipe_error
> /* TODO */
>  };
>
> +enum {

so, I would kinda like to use named enums, and then update the state
structs to use 'em (since it would make gcc and gdb grok them
better).. ofc it is a big change and doesn't have to be done in all
one go, but why not give the enum's names in this first step?

BR,
-R


> +   PIPE_BLENDFACTOR_ONE = 1,
> +   PIPE_BLENDFACTOR_SRC_COLOR,
> +   PIPE_BLENDFACTOR_SRC_ALPHA,
> +   PIPE_BLENDFACTOR_DST_ALPHA,
> +   PIPE_BLENDFACTOR_DST_COLOR,
> +   PIPE_BLENDFACTOR_SRC_ALPHA_SATURATE,
> +   PIPE_BLENDFACTOR_CONST_COLOR,
> +   PIPE_BLENDFACTOR_CONST_ALPHA,
> +   PIPE_BLENDFACTOR_SRC1_COLOR,
> +   PIPE_BLENDFACTOR_SRC1_ALPHA,
> +
> +   PIPE_BLENDFACTOR_ZERO = 0x11,
> +   PIPE_BLENDFACTOR_INV_SRC_COLOR,
> +   PIPE_BLENDFACTOR_INV_SRC_ALPHA,
> +   PIPE_BLENDFACTOR_INV_DST_ALPHA,
> +   PIPE_BLENDFACTOR_INV_DST_COLOR,
> +
> +   PIPE_BLENDFACTOR_INV_CONST_COLOR = 0x17,
> +   PIPE_BLENDFACTOR_INV_CONST_ALPHA,
> +   PIPE_BLENDFACTOR_INV_SRC1_COLOR,
> +   PIPE_BLENDFACTOR_INV_SRC1_ALPHA,
> +};
> +
> +enum {
> +   PIPE_BLEND_ADD,
> +   PIPE_BLEND_SUBTRACT,
> +   PIPE_BLEND_REVERSE_SUBTRACT,
> +   PIPE_BLEND_MIN,
> +   PIPE_BLEND_MAX,
> +};
>
> -#define PIPE_BLENDFACTOR_ONE 0x1
> -#define PIPE_BLENDFACTOR_SRC_COLOR   0x2
> -#define PIPE_BLENDFACTOR_SRC_ALPHA   0x3
> -#define PIPE_BLENDFACTOR_DST_ALPHA   0x4
> -#define PIPE_BLENDFACTOR_DST_COLOR   0x5
> -#define PIPE_BLENDFACTOR_SRC_ALPHA_SATURATE  0x6
> -#define PIPE_BLENDFACTOR_CONST_COLOR 0x7
> -#define PIPE_BLENDFACTOR_CONST_ALPHA 0x8
> -#define PIPE_BLENDFACTOR_SRC1_COLOR  0x9
> -#define PIPE_BLENDFACTOR_SRC1_ALPHA  0x0A
> -#define PIPE_BLENDFACTOR_ZERO0x11
> -#define PIPE_BLENDFACTOR_INV_SRC_COLOR   0x12
> -#define PIPE_BLENDFACTOR_INV_SRC_ALPHA   0x13
> -#define PIPE_BLENDFACTOR_INV_DST_ALPHA   0x14
> -#define PIPE_BLENDFACTOR_INV_DST_COLOR   0x15
> -#define PIPE_BLENDFACTOR_INV_CONST_COLOR 0x17
> -#define PIPE_BLENDFACTOR_INV_CONST_ALPHA 0x18
> -#define PIPE_BLENDFACTOR_INV_SRC1_COLOR  0x19
> -#define PIPE_BLENDFACTOR_INV_SRC1_ALPHA  0x1A
> -
> -#define PIPE_BLEND_ADD   0
> -#define PIPE_BLEND_SUBTRACT  1
> -#define PIPE_BLEND_REVERSE_SUBTRACT  2
> -#define PIPE_BLEND_MIN   3
> -#define PIPE_BLEND_MAX   4
> -
> -#define PIPE_LOGICOP_CLEAR0
> -#define PIPE_LOGICOP_NOR  1
> -#define PIPE_LOGICOP_AND_INVERTED 2
> -#define PIPE_LOGICOP_COPY_INVERTED3
> -#define PIPE_LOGICOP_AND_REVERSE  4
> -#define PIPE_LOGICOP_INVERT   5
> -#define PIPE_LOGICOP_XOR  6
> -#define PIPE_LOGICOP_NAND 7
> -#define PIPE_LOGICOP_AND  8
> -#define PIPE_LOGICOP_EQUIV9
> -#define PIPE_LOGICOP_NOOP 10
> -#define PIPE_LOGICOP_OR_INVERTED  11
> -#define PIPE_LOGICOP_COPY 12
> -#define PIPE_LOGICOP_OR_REVERSE   13
> -#define PIPE_LOGICOP_OR   14
> -#define PIPE_LOGICOP_SET  15
> +enum {
> +   PIPE_LOGICOP_CLEAR,
> +   PIPE_LOGICOP_NOR,
> +   PIPE_LOGICOP_AND_INVERTED,
> +   PIPE_LOGICOP_COPY_INVERTED,
> +   PIPE_LOGICOP_AND_REVERSE,
> +   PIPE_LOGICOP_INVERT,
> +   PIPE_LOGICOP_XOR,
> +   PIPE_LOGICOP_NAND,
> +   PIPE_LOGICOP_AND,
> +   PIPE_LOGICOP_EQUIV,
> +   PIPE_LOGICOP_NOOP,
> +   PIPE_LOGICOP_OR_INVERTED,
> +   PIPE_LOGICOP_COPY,
> +   PIPE_LOGICOP_OR_REVERSE,
> +   PIPE_LOGICOP_OR,
> +   PIPE_LOGICOP_SET,
> +};
>
>  #define PIPE_MASK_R  0x1
>  #define PIPE_MASK_G  0x2
> @@ -110,19 +117,23 @@ enum pipe_error
>   * Inequality functions.  Used for depth test, stencil compare, alpha
>   * test, shadow compare, etc.
>   */
> -#define PIPE_FUNC_NEVER0
> -#define PIPE_FUNC_LESS 1
> -#define PIPE_FUNC_EQUAL2
> -#define PIPE_FUNC_LEQUAL   3
> -#define PIPE_FUNC_GREATER  4
> -#define PIPE_FUNC_NOTEQUAL 5
> -#define PIPE_FUNC_GEQUAL   6
> -#define PIPE_FUNC_ALWAYS   7
> +enum {
> +   PIPE_FUNC_NEVER,
> +   PIPE_FUNC_LESS,
> +   PIPE_FUNC_EQUAL,
> +   PIPE_FUNC_LEQUAL,
> +   PIPE_FUNC_GREATER,
> +   PIPE_FUNC_NOTEQUAL,
> +   PIPE_FUNC_GEQUAL,
> +   PIPE_FUNC_ALWAYS,
> +};
>
>  /** Polygon fill mode */
> -#define PIPE_POLYGON_MODE_FILL  0
> -#define PIPE_POLYGON_MODE_LINE  1
> -#define PIPE_POLYGON_MODE_POINT 2
> +enum {
> +   PIPE_POLYGON_MODE_FILL,
> +   PIPE_POLYGON_MODE_LINE,
> +   PIPE_POLYGON_MODE_POINT,
> +};
>
>  /*

Re: [Mesa-dev] [PATCH 1/6] gallium: use enums in p_defines.h

2016-04-16 Thread eocallaghan

Patches 1 & 2 are,

Reviewed-by: Edward O'Callaghan 

i`ll have to spend some time looking at the others tomorrow..

On 2016-04-16 22:50, Marek Olšák wrote:

From: Marek Olšák 

and remove number assignments which are consecutive
---
 src/gallium/include/pipe/p_defines.h | 378 
+++

 1 file changed, 205 insertions(+), 173 deletions(-)

diff --git a/src/gallium/include/pipe/p_defines.h
b/src/gallium/include/pipe/p_defines.h
index 1aef21d..6bb180d 100644
--- a/src/gallium/include/pipe/p_defines.h
+++ b/src/gallium/include/pipe/p_defines.h
@@ -51,49 +51,56 @@ enum pipe_error
/* TODO */
 };

+enum {
+   PIPE_BLENDFACTOR_ONE = 1,
+   PIPE_BLENDFACTOR_SRC_COLOR,
+   PIPE_BLENDFACTOR_SRC_ALPHA,
+   PIPE_BLENDFACTOR_DST_ALPHA,
+   PIPE_BLENDFACTOR_DST_COLOR,
+   PIPE_BLENDFACTOR_SRC_ALPHA_SATURATE,
+   PIPE_BLENDFACTOR_CONST_COLOR,
+   PIPE_BLENDFACTOR_CONST_ALPHA,
+   PIPE_BLENDFACTOR_SRC1_COLOR,
+   PIPE_BLENDFACTOR_SRC1_ALPHA,
+
+   PIPE_BLENDFACTOR_ZERO = 0x11,
+   PIPE_BLENDFACTOR_INV_SRC_COLOR,
+   PIPE_BLENDFACTOR_INV_SRC_ALPHA,
+   PIPE_BLENDFACTOR_INV_DST_ALPHA,
+   PIPE_BLENDFACTOR_INV_DST_COLOR,
+
+   PIPE_BLENDFACTOR_INV_CONST_COLOR = 0x17,
+   PIPE_BLENDFACTOR_INV_CONST_ALPHA,
+   PIPE_BLENDFACTOR_INV_SRC1_COLOR,
+   PIPE_BLENDFACTOR_INV_SRC1_ALPHA,
+};
+
+enum {
+   PIPE_BLEND_ADD,
+   PIPE_BLEND_SUBTRACT,
+   PIPE_BLEND_REVERSE_SUBTRACT,
+   PIPE_BLEND_MIN,
+   PIPE_BLEND_MAX,
+};

-#define PIPE_BLENDFACTOR_ONE 0x1
-#define PIPE_BLENDFACTOR_SRC_COLOR   0x2
-#define PIPE_BLENDFACTOR_SRC_ALPHA   0x3
-#define PIPE_BLENDFACTOR_DST_ALPHA   0x4
-#define PIPE_BLENDFACTOR_DST_COLOR   0x5
-#define PIPE_BLENDFACTOR_SRC_ALPHA_SATURATE  0x6
-#define PIPE_BLENDFACTOR_CONST_COLOR 0x7
-#define PIPE_BLENDFACTOR_CONST_ALPHA 0x8
-#define PIPE_BLENDFACTOR_SRC1_COLOR  0x9
-#define PIPE_BLENDFACTOR_SRC1_ALPHA  0x0A
-#define PIPE_BLENDFACTOR_ZERO0x11
-#define PIPE_BLENDFACTOR_INV_SRC_COLOR   0x12
-#define PIPE_BLENDFACTOR_INV_SRC_ALPHA   0x13
-#define PIPE_BLENDFACTOR_INV_DST_ALPHA   0x14
-#define PIPE_BLENDFACTOR_INV_DST_COLOR   0x15
-#define PIPE_BLENDFACTOR_INV_CONST_COLOR 0x17
-#define PIPE_BLENDFACTOR_INV_CONST_ALPHA 0x18
-#define PIPE_BLENDFACTOR_INV_SRC1_COLOR  0x19
-#define PIPE_BLENDFACTOR_INV_SRC1_ALPHA  0x1A
-
-#define PIPE_BLEND_ADD   0
-#define PIPE_BLEND_SUBTRACT  1
-#define PIPE_BLEND_REVERSE_SUBTRACT  2
-#define PIPE_BLEND_MIN   3
-#define PIPE_BLEND_MAX   4
-
-#define PIPE_LOGICOP_CLEAR0
-#define PIPE_LOGICOP_NOR  1
-#define PIPE_LOGICOP_AND_INVERTED 2
-#define PIPE_LOGICOP_COPY_INVERTED3
-#define PIPE_LOGICOP_AND_REVERSE  4
-#define PIPE_LOGICOP_INVERT   5
-#define PIPE_LOGICOP_XOR  6
-#define PIPE_LOGICOP_NAND 7
-#define PIPE_LOGICOP_AND  8
-#define PIPE_LOGICOP_EQUIV9
-#define PIPE_LOGICOP_NOOP 10
-#define PIPE_LOGICOP_OR_INVERTED  11
-#define PIPE_LOGICOP_COPY 12
-#define PIPE_LOGICOP_OR_REVERSE   13
-#define PIPE_LOGICOP_OR   14
-#define PIPE_LOGICOP_SET  15
+enum {
+   PIPE_LOGICOP_CLEAR,
+   PIPE_LOGICOP_NOR,
+   PIPE_LOGICOP_AND_INVERTED,
+   PIPE_LOGICOP_COPY_INVERTED,
+   PIPE_LOGICOP_AND_REVERSE,
+   PIPE_LOGICOP_INVERT,
+   PIPE_LOGICOP_XOR,
+   PIPE_LOGICOP_NAND,
+   PIPE_LOGICOP_AND,
+   PIPE_LOGICOP_EQUIV,
+   PIPE_LOGICOP_NOOP,
+   PIPE_LOGICOP_OR_INVERTED,
+   PIPE_LOGICOP_COPY,
+   PIPE_LOGICOP_OR_REVERSE,
+   PIPE_LOGICOP_OR,
+   PIPE_LOGICOP_SET,
+};

 #define PIPE_MASK_R  0x1
 #define PIPE_MASK_G  0x2
@@ -110,19 +117,23 @@ enum pipe_error
  * Inequality functions.  Used for depth test, stencil compare, alpha
  * test, shadow compare, etc.
  */
-#define PIPE_FUNC_NEVER0
-#define PIPE_FUNC_LESS 1
-#define PIPE_FUNC_EQUAL2
-#define PIPE_FUNC_LEQUAL   3
-#define PIPE_FUNC_GREATER  4
-#define PIPE_FUNC_NOTEQUAL 5
-#define PIPE_FUNC_GEQUAL   6
-#define PIPE_FUNC_ALWAYS   7
+enum {
+   PIPE_FUNC_NEVER,
+   PIPE_FUNC_LESS,
+   PIPE_FUNC_EQUAL,
+   PIPE_FUNC_LEQUAL,
+   PIPE_FUNC_GREATER,
+   PIPE_FUNC_NOTEQUAL,
+   PIPE_FUNC_GEQUAL,
+   PIPE_FUNC_ALWAYS,
+};

 /** Polygon fill mode */
-#define PIPE_POLYGON_MODE_FILL  0
-#define PIPE_POLYGON_MODE_LINE  1
-#define PIPE_POLYGON_MODE_POINT 2
+enum {
+   PIPE_POLYGON_MODE_FILL,
+   PIPE_POLYGON_MODE_LINE,
+   PIPE_POLYGON_MODE_POINT,
+};

 /** Polygon face specification, eg for culling */
 #define PIPE_FACE_NONE   0
@@ -131,60 +142,72 @@ enum pipe_error
 #define PIPE_FACE_FRONT_AND_BACK (PIPE_FACE_FRONT | PIPE_FACE_BACK)

 /** Stencil ops */
-#define PIPE_STENCIL_OP_KEEP   0
-#define PIPE_STENCIL_OP_ZERO   1
-#define PIPE_STENCIL_OP_REPLACE2
-#define PIPE_STENCIL_OP_INCR   3
-#define PIPE_STENCIL_OP_DECR   4
-#define PIPE_STENCIL_OP_INCR_WRAP  5
-#define PI

[Mesa-dev] [PATCH 1/6] gallium: use enums in p_defines.h

2016-04-16 Thread Marek Olšák
From: Marek Olšák 

and remove number assignments which are consecutive
---
 src/gallium/include/pipe/p_defines.h | 378 +++
 1 file changed, 205 insertions(+), 173 deletions(-)

diff --git a/src/gallium/include/pipe/p_defines.h 
b/src/gallium/include/pipe/p_defines.h
index 1aef21d..6bb180d 100644
--- a/src/gallium/include/pipe/p_defines.h
+++ b/src/gallium/include/pipe/p_defines.h
@@ -51,49 +51,56 @@ enum pipe_error
/* TODO */
 };
 
+enum {
+   PIPE_BLENDFACTOR_ONE = 1,
+   PIPE_BLENDFACTOR_SRC_COLOR,
+   PIPE_BLENDFACTOR_SRC_ALPHA,
+   PIPE_BLENDFACTOR_DST_ALPHA,
+   PIPE_BLENDFACTOR_DST_COLOR,
+   PIPE_BLENDFACTOR_SRC_ALPHA_SATURATE,
+   PIPE_BLENDFACTOR_CONST_COLOR,
+   PIPE_BLENDFACTOR_CONST_ALPHA,
+   PIPE_BLENDFACTOR_SRC1_COLOR,
+   PIPE_BLENDFACTOR_SRC1_ALPHA,
+
+   PIPE_BLENDFACTOR_ZERO = 0x11,
+   PIPE_BLENDFACTOR_INV_SRC_COLOR,
+   PIPE_BLENDFACTOR_INV_SRC_ALPHA,
+   PIPE_BLENDFACTOR_INV_DST_ALPHA,
+   PIPE_BLENDFACTOR_INV_DST_COLOR,
+
+   PIPE_BLENDFACTOR_INV_CONST_COLOR = 0x17,
+   PIPE_BLENDFACTOR_INV_CONST_ALPHA,
+   PIPE_BLENDFACTOR_INV_SRC1_COLOR,
+   PIPE_BLENDFACTOR_INV_SRC1_ALPHA,
+};
+
+enum {
+   PIPE_BLEND_ADD,
+   PIPE_BLEND_SUBTRACT,
+   PIPE_BLEND_REVERSE_SUBTRACT,
+   PIPE_BLEND_MIN,
+   PIPE_BLEND_MAX,
+};
 
-#define PIPE_BLENDFACTOR_ONE 0x1
-#define PIPE_BLENDFACTOR_SRC_COLOR   0x2
-#define PIPE_BLENDFACTOR_SRC_ALPHA   0x3
-#define PIPE_BLENDFACTOR_DST_ALPHA   0x4
-#define PIPE_BLENDFACTOR_DST_COLOR   0x5
-#define PIPE_BLENDFACTOR_SRC_ALPHA_SATURATE  0x6
-#define PIPE_BLENDFACTOR_CONST_COLOR 0x7
-#define PIPE_BLENDFACTOR_CONST_ALPHA 0x8
-#define PIPE_BLENDFACTOR_SRC1_COLOR  0x9
-#define PIPE_BLENDFACTOR_SRC1_ALPHA  0x0A
-#define PIPE_BLENDFACTOR_ZERO0x11
-#define PIPE_BLENDFACTOR_INV_SRC_COLOR   0x12
-#define PIPE_BLENDFACTOR_INV_SRC_ALPHA   0x13
-#define PIPE_BLENDFACTOR_INV_DST_ALPHA   0x14
-#define PIPE_BLENDFACTOR_INV_DST_COLOR   0x15
-#define PIPE_BLENDFACTOR_INV_CONST_COLOR 0x17
-#define PIPE_BLENDFACTOR_INV_CONST_ALPHA 0x18
-#define PIPE_BLENDFACTOR_INV_SRC1_COLOR  0x19
-#define PIPE_BLENDFACTOR_INV_SRC1_ALPHA  0x1A
-
-#define PIPE_BLEND_ADD   0
-#define PIPE_BLEND_SUBTRACT  1
-#define PIPE_BLEND_REVERSE_SUBTRACT  2
-#define PIPE_BLEND_MIN   3
-#define PIPE_BLEND_MAX   4
-
-#define PIPE_LOGICOP_CLEAR0
-#define PIPE_LOGICOP_NOR  1
-#define PIPE_LOGICOP_AND_INVERTED 2
-#define PIPE_LOGICOP_COPY_INVERTED3
-#define PIPE_LOGICOP_AND_REVERSE  4
-#define PIPE_LOGICOP_INVERT   5
-#define PIPE_LOGICOP_XOR  6
-#define PIPE_LOGICOP_NAND 7
-#define PIPE_LOGICOP_AND  8
-#define PIPE_LOGICOP_EQUIV9
-#define PIPE_LOGICOP_NOOP 10
-#define PIPE_LOGICOP_OR_INVERTED  11
-#define PIPE_LOGICOP_COPY 12
-#define PIPE_LOGICOP_OR_REVERSE   13
-#define PIPE_LOGICOP_OR   14
-#define PIPE_LOGICOP_SET  15  
+enum {
+   PIPE_LOGICOP_CLEAR,
+   PIPE_LOGICOP_NOR,
+   PIPE_LOGICOP_AND_INVERTED,
+   PIPE_LOGICOP_COPY_INVERTED,
+   PIPE_LOGICOP_AND_REVERSE,
+   PIPE_LOGICOP_INVERT,
+   PIPE_LOGICOP_XOR,
+   PIPE_LOGICOP_NAND,
+   PIPE_LOGICOP_AND,
+   PIPE_LOGICOP_EQUIV,
+   PIPE_LOGICOP_NOOP,
+   PIPE_LOGICOP_OR_INVERTED,
+   PIPE_LOGICOP_COPY,
+   PIPE_LOGICOP_OR_REVERSE,
+   PIPE_LOGICOP_OR,
+   PIPE_LOGICOP_SET,
+};
 
 #define PIPE_MASK_R  0x1
 #define PIPE_MASK_G  0x2
@@ -110,19 +117,23 @@ enum pipe_error
  * Inequality functions.  Used for depth test, stencil compare, alpha
  * test, shadow compare, etc.
  */
-#define PIPE_FUNC_NEVER0
-#define PIPE_FUNC_LESS 1
-#define PIPE_FUNC_EQUAL2
-#define PIPE_FUNC_LEQUAL   3
-#define PIPE_FUNC_GREATER  4
-#define PIPE_FUNC_NOTEQUAL 5
-#define PIPE_FUNC_GEQUAL   6
-#define PIPE_FUNC_ALWAYS   7
+enum {
+   PIPE_FUNC_NEVER,
+   PIPE_FUNC_LESS,
+   PIPE_FUNC_EQUAL,
+   PIPE_FUNC_LEQUAL,
+   PIPE_FUNC_GREATER,
+   PIPE_FUNC_NOTEQUAL,
+   PIPE_FUNC_GEQUAL,
+   PIPE_FUNC_ALWAYS,
+};
 
 /** Polygon fill mode */
-#define PIPE_POLYGON_MODE_FILL  0
-#define PIPE_POLYGON_MODE_LINE  1
-#define PIPE_POLYGON_MODE_POINT 2
+enum {
+   PIPE_POLYGON_MODE_FILL,
+   PIPE_POLYGON_MODE_LINE,
+   PIPE_POLYGON_MODE_POINT,
+};
 
 /** Polygon face specification, eg for culling */
 #define PIPE_FACE_NONE   0
@@ -131,60 +142,72 @@ enum pipe_error
 #define PIPE_FACE_FRONT_AND_BACK (PIPE_FACE_FRONT | PIPE_FACE_BACK)
 
 /** Stencil ops */
-#define PIPE_STENCIL_OP_KEEP   0
-#define PIPE_STENCIL_OP_ZERO   1
-#define PIPE_STENCIL_OP_REPLACE2
-#define PIPE_STENCIL_OP_INCR   3
-#define PIPE_STENCIL_OP_DECR   4
-#define PIPE_STENCIL_OP_INCR_WRAP  5
-#define PIPE_STENCIL_OP_DECR_WRAP  6
-#define PIPE_STENCIL_OP_INVERT 7
+enum {
+   PIPE_STENCIL_OP_KEEP,
+   PIPE_STENCIL_OP_ZERO,
+   PIPE_STENCIL_OP_REPLACE