Re: [Mesa-dev] [RFC 09/21] mesa: Generate a helper function for each extension

2015-10-26 Thread Chad Versace 
On Fri 23 Oct 2015, Nanley Chery wrote:
> On Thu, Oct 22, 2015 at 11:30 AM, Chad Versace 
> wrote:
> 
> > On Mon 19 Oct 2015, Nanley Chery wrote:
> > > From: Nanley Chery 
> > >
> > > Generate functions which determine if an extension is supported in the
> > > current context. Initially, enums were going to be explicitly used with
> > > _mesa_extension_supported(). The idea to embed the function and enums
> > > into generated helper functions was suggested by Kristian Høgsberg.
> > >
> > > For performance, the function body no longer uses
> > > _mesa_extension_supported() and, as suggested by Chad Versace, the
> > > functions are also declared static inline.
> > >
> > > Signed-off-by: Nanley Chery 
> > > ---
> > >  src/mesa/main/context.h|  1 +
> > >  src/mesa/main/extensions.c | 22 +-
> > >  src/mesa/main/extensions.h | 39 +++
> > >  3 files changed, 41 insertions(+), 21 deletions(-)
> >
> > [...]


> > > @@ -55,6 +55,45 @@ _mesa_get_extension_count(struct gl_context *ctx);
> > >  extern const GLubyte *
> > >  _mesa_get_enabled_extension(struct gl_context *ctx, GLuint index);
> > >
> > > +
> > > +/**
> > > + * \brief An element of the \c extension_table.
> > > + */
> > > +struct extension {
> >
> > In addition to Marek's comment, the struct should be prefixed too.
> >
> >
> After rereading the coding style guidelines, I've found that it only says
> that
> functions need to be prefixed. Should these two suggestions be added?

Personally, I would avoid placing such a rule in the style guide,
because there is no hard, absolute rule that dictates when a symbol
should be prefixed. For example, many of the global symbols in
src/util/*.h are not prefixed.

If you examine the symbols in src/mesa/main/mtypes.h, though, nearly
everything is prefixed. And I view src/mesa/main/extensions.h as a file
that belongs in the same family as mtypes.h.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [RFC 09/21] mesa: Generate a helper function for each extension

2015-10-23 Thread Nanley Chery 
On Thu, Oct 22, 2015 at 11:30 AM, Chad Versace 
wrote:

> On Mon 19 Oct 2015, Nanley Chery wrote:
> > From: Nanley Chery 
> >
> > Generate functions which determine if an extension is supported in the
> > current context. Initially, enums were going to be explicitly used with
> > _mesa_extension_supported(). The idea to embed the function and enums
> > into generated helper functions was suggested by Kristian Høgsberg.
> >
> > For performance, the function body no longer uses
> > _mesa_extension_supported() and, as suggested by Chad Versace, the
> > functions are also declared static inline.
> >
> > Signed-off-by: Nanley Chery 
> > ---
> >  src/mesa/main/context.h|  1 +
> >  src/mesa/main/extensions.c | 22 +-
> >  src/mesa/main/extensions.h | 39 +++
> >  3 files changed, 41 insertions(+), 21 deletions(-)
>
> [...]
>
> > diff --git a/src/mesa/main/extensions.h b/src/mesa/main/extensions.h
> > index 595512a..24cc04d 100644
> > --- a/src/mesa/main/extensions.h
> > +++ b/src/mesa/main/extensions.h
> > @@ -55,6 +55,45 @@ _mesa_get_extension_count(struct gl_context *ctx);
> >  extern const GLubyte *
> >  _mesa_get_enabled_extension(struct gl_context *ctx, GLuint index);
> >
> > +
> > +/**
> > + * \brief An element of the \c extension_table.
> > + */
> > +struct extension {
>
> In addition to Marek's comment, the struct should be prefixed too.
>
>
After rereading the coding style guidelines, I've found that it only says
that
functions need to be prefixed. Should these two suggestions be added?


> > +   /** Name of extension, such as "GL_ARB_depth_clamp". */
> > +   const char *name;
> > +
> > +   /** Offset (in bytes) of the corresponding member in struct
> gl_extensions. */
> > +   size_t offset;
> > +
> > +   /** Minimum version the extension requires for the given API
> > +* (see gl_api defined in mtypes.h)
> > +*/
> > +   GLuint version[API_OPENGL_LAST + 1];
> > +
> > +   /** Year the extension was proposed or approved.  Used to sort the
> > +* extension string chronologically. */
> > +   uint16_t year;
> > +} extern const extension_table[];
>
> [...]
>
> > +/** Checks if the context suports a user-facing extension */
> > +#define EXT(name_str, driver_cap, ...) \
> > +static inline bool _mesa_has_##name_str(const struct gl_context *ctx) {
> \
> > +  return ctx->Extensions.driver_cap && (ctx->Version >= \
> > +
>  extension_table[MESA_EXTENSION_##name_str].version[ctx->API]); \
> > +}
> > +#include "extensions_table.h"
> > +#undef EXT
>
> The function should be formatted like this, to follow Mesa style:
>
>   |static inline bool
>   |_mesa_has_foo(...)
>   |{
>   |   ...
>   |}
>

Will do.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [RFC 09/21] mesa: Generate a helper function for each extension

2015-10-22 Thread Chad Versace 
On Mon 19 Oct 2015, Nanley Chery wrote:
> From: Nanley Chery 
> 
> Generate functions which determine if an extension is supported in the
> current context. Initially, enums were going to be explicitly used with
> _mesa_extension_supported(). The idea to embed the function and enums
> into generated helper functions was suggested by Kristian Høgsberg.
> 
> For performance, the function body no longer uses
> _mesa_extension_supported() and, as suggested by Chad Versace, the
> functions are also declared static inline.
> 
> Signed-off-by: Nanley Chery 
> ---
>  src/mesa/main/context.h|  1 +
>  src/mesa/main/extensions.c | 22 +-
>  src/mesa/main/extensions.h | 39 +++
>  3 files changed, 41 insertions(+), 21 deletions(-)

[...]

> diff --git a/src/mesa/main/extensions.h b/src/mesa/main/extensions.h
> index 595512a..24cc04d 100644
> --- a/src/mesa/main/extensions.h
> +++ b/src/mesa/main/extensions.h
> @@ -55,6 +55,45 @@ _mesa_get_extension_count(struct gl_context *ctx);
>  extern const GLubyte *
>  _mesa_get_enabled_extension(struct gl_context *ctx, GLuint index);
>  
> +
> +/**
> + * \brief An element of the \c extension_table.
> + */
> +struct extension {

In addition to Marek's comment, the struct should be prefixed too.

> +   /** Name of extension, such as "GL_ARB_depth_clamp". */
> +   const char *name;
> +
> +   /** Offset (in bytes) of the corresponding member in struct 
> gl_extensions. */
> +   size_t offset;
> +
> +   /** Minimum version the extension requires for the given API
> +* (see gl_api defined in mtypes.h)
> +*/
> +   GLuint version[API_OPENGL_LAST + 1];
> +
> +   /** Year the extension was proposed or approved.  Used to sort the 
> +* extension string chronologically. */
> +   uint16_t year;
> +} extern const extension_table[];

[...]

> +/** Checks if the context suports a user-facing extension */
> +#define EXT(name_str, driver_cap, ...) \
> +static inline bool _mesa_has_##name_str(const struct gl_context *ctx) { \
> +  return ctx->Extensions.driver_cap && (ctx->Version >= \
> + extension_table[MESA_EXTENSION_##name_str].version[ctx->API]); \
> +}
> +#include "extensions_table.h"
> +#undef EXT

The function should be formatted like this, to follow Mesa style:

  |static inline bool
  |_mesa_has_foo(...)
  |{
  |   ...
  |}
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [RFC 09/21] mesa: Generate a helper function for each extension

2015-10-20 Thread Marek Olšák 
On Tue, Oct 20, 2015 at 12:44 AM, Nanley Chery  wrote:
> From: Nanley Chery 
>
> Generate functions which determine if an extension is supported in the
> current context. Initially, enums were going to be explicitly used with
> _mesa_extension_supported(). The idea to embed the function and enums
> into generated helper functions was suggested by Kristian Høgsberg.
>
> For performance, the function body no longer uses
> _mesa_extension_supported() and, as suggested by Chad Versace, the
> functions are also declared static inline.
>
> Signed-off-by: Nanley Chery 
> ---
>  src/mesa/main/context.h|  1 +
>  src/mesa/main/extensions.c | 22 +-
>  src/mesa/main/extensions.h | 39 +++
>  3 files changed, 41 insertions(+), 21 deletions(-)
>
> diff --git a/src/mesa/main/context.h b/src/mesa/main/context.h
> index 1e7a12c..4798b1f 100644
> --- a/src/mesa/main/context.h
> +++ b/src/mesa/main/context.h
> @@ -50,6 +50,7 @@
>
>
>  #include "imports.h"
> +#include "extensions.h"
>  #include "mtypes.h"
>  #include "vbo/vbo.h"
>
> diff --git a/src/mesa/main/extensions.c b/src/mesa/main/extensions.c
> index 9451085..136313f 100644
> --- a/src/mesa/main/extensions.c
> +++ b/src/mesa/main/extensions.c
> @@ -42,26 +42,6 @@ struct gl_extensions _mesa_extension_override_disables;
>  static char *extra_extensions = NULL;
>  static char *cant_disable_extensions = NULL;
>
> -/**
> - * \brief An element of the \c extension_table.
> - */
> -struct extension {
> -   /** Name of extension, such as "GL_ARB_depth_clamp". */
> -   const char *name;
> -
> -   /** Offset (in bytes) of the corresponding member in struct 
> gl_extensions. */
> -   size_t offset;
> -
> -   /** Minimum version the extension requires for the given API
> -* (see gl_api defined in mtypes.h)
> -*/
> -   GLuint version[API_OPENGL_LAST + 1];
> -
> -   /** Year the extension was proposed or approved.  Used to sort the
> -* extension string chronologically. */
> -   uint16_t year;
> -};
> -
>
>  /**
>   * Given a member \c x of struct gl_extensions, return offset of
> @@ -73,7 +53,7 @@ struct extension {
>  /**
>   * \brief Table of supported OpenGL extensions for all API's.
>   */
> -static const struct extension extension_table[] = {
> +const struct extension extension_table[] = {
>  #define EXT(name_str, driver_cap, gll_ver, glc_ver, gles_ver, gles2_ver, 
> ) \
>  { .name = "GL_" #name_str, .offset = o(driver_cap), \
>.version = { \
> diff --git a/src/mesa/main/extensions.h b/src/mesa/main/extensions.h
> index 595512a..24cc04d 100644
> --- a/src/mesa/main/extensions.h
> +++ b/src/mesa/main/extensions.h
> @@ -55,6 +55,45 @@ _mesa_get_extension_count(struct gl_context *ctx);
>  extern const GLubyte *
>  _mesa_get_enabled_extension(struct gl_context *ctx, GLuint index);
>
> +
> +/**
> + * \brief An element of the \c extension_table.
> + */
> +struct extension {
> +   /** Name of extension, such as "GL_ARB_depth_clamp". */
> +   const char *name;
> +
> +   /** Offset (in bytes) of the corresponding member in struct 
> gl_extensions. */
> +   size_t offset;
> +
> +   /** Minimum version the extension requires for the given API
> +* (see gl_api defined in mtypes.h)
> +*/
> +   GLuint version[API_OPENGL_LAST + 1];
> +
> +   /** Year the extension was proposed or approved.  Used to sort the
> +* extension string chronologically. */
> +   uint16_t year;
> +} extern const extension_table[];

Global variables should have a proper prefix.

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

Re: [Mesa-dev] [RFC 09/21] mesa: Generate a helper function for each extension

2015-10-20 Thread Nanley Chery 
On Tue, Oct 20, 2015 at 8:26 AM, Marek Olšák  wrote:

> On Tue, Oct 20, 2015 at 12:44 AM, Nanley Chery 
> wrote:
> > From: Nanley Chery 
> >
> > Generate functions which determine if an extension is supported in the
> > current context. Initially, enums were going to be explicitly used with
> > _mesa_extension_supported(). The idea to embed the function and enums
> > into generated helper functions was suggested by Kristian Høgsberg.
> >
> > For performance, the function body no longer uses
> > _mesa_extension_supported() and, as suggested by Chad Versace, the
> > functions are also declared static inline.
> >
> > Signed-off-by: Nanley Chery 
> > ---
> >  src/mesa/main/context.h|  1 +
> >  src/mesa/main/extensions.c | 22 +-
> >  src/mesa/main/extensions.h | 39 +++
> >  3 files changed, 41 insertions(+), 21 deletions(-)
> >
> > diff --git a/src/mesa/main/context.h b/src/mesa/main/context.h
> > index 1e7a12c..4798b1f 100644
> > --- a/src/mesa/main/context.h
> > +++ b/src/mesa/main/context.h
> > @@ -50,6 +50,7 @@
> >
> >
> >  #include "imports.h"
> > +#include "extensions.h"
> >  #include "mtypes.h"
> >  #include "vbo/vbo.h"
> >
> > diff --git a/src/mesa/main/extensions.c b/src/mesa/main/extensions.c
> > index 9451085..136313f 100644
> > --- a/src/mesa/main/extensions.c
> > +++ b/src/mesa/main/extensions.c
> > @@ -42,26 +42,6 @@ struct gl_extensions
> _mesa_extension_override_disables;
> >  static char *extra_extensions = NULL;
> >  static char *cant_disable_extensions = NULL;
> >
> > -/**
> > - * \brief An element of the \c extension_table.
> > - */
> > -struct extension {
> > -   /** Name of extension, such as "GL_ARB_depth_clamp". */
> > -   const char *name;
> > -
> > -   /** Offset (in bytes) of the corresponding member in struct
> gl_extensions. */
> > -   size_t offset;
> > -
> > -   /** Minimum version the extension requires for the given API
> > -* (see gl_api defined in mtypes.h)
> > -*/
> > -   GLuint version[API_OPENGL_LAST + 1];
> > -
> > -   /** Year the extension was proposed or approved.  Used to sort the
> > -* extension string chronologically. */
> > -   uint16_t year;
> > -};
> > -
> >
> >  /**
> >   * Given a member \c x of struct gl_extensions, return offset of
> > @@ -73,7 +53,7 @@ struct extension {
> >  /**
> >   * \brief Table of supported OpenGL extensions for all API's.
> >   */
> > -static const struct extension extension_table[] = {
> > +const struct extension extension_table[] = {
> >  #define EXT(name_str, driver_cap, gll_ver, glc_ver, gles_ver,
> gles2_ver, ) \
> >  { .name = "GL_" #name_str, .offset = o(driver_cap), \
> >.version = { \
> > diff --git a/src/mesa/main/extensions.h b/src/mesa/main/extensions.h
> > index 595512a..24cc04d 100644
> > --- a/src/mesa/main/extensions.h
> > +++ b/src/mesa/main/extensions.h
> > @@ -55,6 +55,45 @@ _mesa_get_extension_count(struct gl_context *ctx);
> >  extern const GLubyte *
> >  _mesa_get_enabled_extension(struct gl_context *ctx, GLuint index);
> >
> > +
> > +/**
> > + * \brief An element of the \c extension_table.
> > + */
> > +struct extension {
> > +   /** Name of extension, such as "GL_ARB_depth_clamp". */
> > +   const char *name;
> > +
> > +   /** Offset (in bytes) of the corresponding member in struct
> gl_extensions. */
> > +   size_t offset;
> > +
> > +   /** Minimum version the extension requires for the given API
> > +* (see gl_api defined in mtypes.h)
> > +*/
> > +   GLuint version[API_OPENGL_LAST + 1];
> > +
> > +   /** Year the extension was proposed or approved.  Used to sort the
> > +* extension string chronologically. */
> > +   uint16_t year;
> > +} extern const extension_table[];
>
> Global variables should have a proper prefix.
>
>
I'll send out an update for this.

Thanks,
Nanley

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

[Mesa-dev] [RFC 09/21] mesa: Generate a helper function for each extension

2015-10-19 Thread Nanley Chery 
From: Nanley Chery 

Generate functions which determine if an extension is supported in the
current context. Initially, enums were going to be explicitly used with
_mesa_extension_supported(). The idea to embed the function and enums
into generated helper functions was suggested by Kristian Høgsberg.

For performance, the function body no longer uses
_mesa_extension_supported() and, as suggested by Chad Versace, the
functions are also declared static inline.

Signed-off-by: Nanley Chery 
---
 src/mesa/main/context.h|  1 +
 src/mesa/main/extensions.c | 22 +-
 src/mesa/main/extensions.h | 39 +++
 3 files changed, 41 insertions(+), 21 deletions(-)

diff --git a/src/mesa/main/context.h b/src/mesa/main/context.h
index 1e7a12c..4798b1f 100644
--- a/src/mesa/main/context.h
+++ b/src/mesa/main/context.h
@@ -50,6 +50,7 @@
 
 
 #include "imports.h"
+#include "extensions.h"
 #include "mtypes.h"
 #include "vbo/vbo.h"
 
diff --git a/src/mesa/main/extensions.c b/src/mesa/main/extensions.c
index 9451085..136313f 100644
--- a/src/mesa/main/extensions.c
+++ b/src/mesa/main/extensions.c
@@ -42,26 +42,6 @@ struct gl_extensions _mesa_extension_override_disables;
 static char *extra_extensions = NULL;
 static char *cant_disable_extensions = NULL;
 
-/**
- * \brief An element of the \c extension_table.
- */
-struct extension {
-   /** Name of extension, such as "GL_ARB_depth_clamp". */
-   const char *name;
-
-   /** Offset (in bytes) of the corresponding member in struct gl_extensions. 
*/
-   size_t offset;
-
-   /** Minimum version the extension requires for the given API
-* (see gl_api defined in mtypes.h)
-*/
-   GLuint version[API_OPENGL_LAST + 1];
-
-   /** Year the extension was proposed or approved.  Used to sort the 
-* extension string chronologically. */
-   uint16_t year;
-};
-
 
 /**
  * Given a member \c x of struct gl_extensions, return offset of
@@ -73,7 +53,7 @@ struct extension {
 /**
  * \brief Table of supported OpenGL extensions for all API's.
  */
-static const struct extension extension_table[] = {
+const struct extension extension_table[] = {
 #define EXT(name_str, driver_cap, gll_ver, glc_ver, gles_ver, gles2_ver, ) 
\
 { .name = "GL_" #name_str, .offset = o(driver_cap), \
   .version = { \
diff --git a/src/mesa/main/extensions.h b/src/mesa/main/extensions.h
index 595512a..24cc04d 100644
--- a/src/mesa/main/extensions.h
+++ b/src/mesa/main/extensions.h
@@ -55,6 +55,45 @@ _mesa_get_extension_count(struct gl_context *ctx);
 extern const GLubyte *
 _mesa_get_enabled_extension(struct gl_context *ctx, GLuint index);
 
+
+/**
+ * \brief An element of the \c extension_table.
+ */
+struct extension {
+   /** Name of extension, such as "GL_ARB_depth_clamp". */
+   const char *name;
+
+   /** Offset (in bytes) of the corresponding member in struct gl_extensions. 
*/
+   size_t offset;
+
+   /** Minimum version the extension requires for the given API
+* (see gl_api defined in mtypes.h)
+*/
+   GLuint version[API_OPENGL_LAST + 1];
+
+   /** Year the extension was proposed or approved.  Used to sort the 
+* extension string chronologically. */
+   uint16_t year;
+} extern const extension_table[];
+
+
+/* Generate enums for the functions below */
+enum {
+#define EXT(name_str, ...) MESA_EXTENSION_##name_str,
+#include "extensions_table.h"
+#undef EXT
+};
+
+
+/** Checks if the context suports a user-facing extension */
+#define EXT(name_str, driver_cap, ...) \
+static inline bool _mesa_has_##name_str(const struct gl_context *ctx) { \
+  return ctx->Extensions.driver_cap && (ctx->Version >= \
+ extension_table[MESA_EXTENSION_##name_str].version[ctx->API]); \
+}
+#include "extensions_table.h"
+#undef EXT
+
 extern struct gl_extensions _mesa_extension_override_enables;
 extern struct gl_extensions _mesa_extension_override_disables;
 
-- 
2.6.1

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