These comments have been addressed:
http://cgit.freedesktop.org/~ldeks/mesa/commit/?h=adsa-textures&id=a2f20c936f6db31986220938db06cf28885e7ee6
.
Thanks.
Laura
On Wed, Dec 31, 2014 at 6:03 PM, Anuj Phogat <anuj.pho...@gmail.com> wrote:
> On Tue, Dec 16, 2014 at 10:59 AM, Laura Ekstrand <la...@jlekstrand.net>
> wrote:
> > This happens almost everywhere. I prefer to call _mesa_error(ctx,
> > "glTex%sStorage".... rather than _mesa_error(ctx, "%s" .... because it's
> > more obvious when reading the code which API function you're in (texture
> > storage rather than copy texture sub image, etc). Would this work
> better?
> >
> > const char *suffix = dsa ? "ture" : "";
> >
> > _mesa_error(ctx, "glTex%sStorage", suffix....
> > _mesa_error(ctx, "glTex%sStorage", suffix....
> >
> > Thanks.
> >
> > Laura
> >
> > On Tue, Dec 16, 2014 at 7:46 AM, Brian Paul <bri...@vmware.com> wrote:
> >>
> >> On 12/15/2014 06:22 PM, Laura Ekstrand wrote:
> >>>
> >>> ---
> >>> src/mapi/glapi/gen/ARB_direct_state_access.xml | 24 +++
> >>> src/mesa/main/texstorage.c | 205
> >>> +++++++++++++++++++------
> >>> src/mesa/main/texstorage.h | 31 ++++
> >>> 3 files changed, 209 insertions(+), 51 deletions(-)
> >>>
> >>> diff --git a/src/mapi/glapi/gen/ARB_direct_state_access.xml
> >>> b/src/mapi/glapi/gen/ARB_direct_state_access.xml
> >>> index 9f2eacb..37aac7e 100644
> >>> --- a/src/mapi/glapi/gen/ARB_direct_state_access.xml
> >>> +++ b/src/mapi/glapi/gen/ARB_direct_state_access.xml
> >>> @@ -15,5 +15,29 @@
> >>> <param name="textures" type="GLuint *" />
> >>> </function>
> >>>
> >>> + <function name="TextureStorage1D" offset="assign">
> >>> + <param name="texture" type="GLuint" />
> >>> + <param name="levels" type="GLsizei" />
> >>> + <param name="internalformat" type="GLenum" />
> >>> + <param name="width" type="GLsizei" />
> >>> + </function>
> >>> +
> >>> + <function name="TextureStorage2D" offset="assign">
> >>> + <param name="texture" type="GLuint" />
> >>> + <param name="levels" type="GLsizei" />
> >>> + <param name="internalformat" type="GLenum" />
> >>> + <param name="width" type="GLsizei" />
> >>> + <param name="height" type="GLsizei" />
> >>> + </function>
> >>> +
> >>> + <function name="TextureStorage3D" offset="assign">
> >>> + <param name="texture" type="GLuint" />
> >>> + <param name="levels" type="GLsizei" />
> >>> + <param name="internalformat" type="GLenum" />
> >>> + <param name="width" type="GLsizei" />
> >>> + <param name="height" type="GLsizei" />
> >>> + <param name="depth" type="GLsizei" />
> >>> + </function>
> >>> +
> >>> </category>
> >>> </OpenGLAPI>
> >>> diff --git a/src/mesa/main/texstorage.c b/src/mesa/main/texstorage.c
> >>> index f41076a..7565a43 100644
> >>> --- a/src/mesa/main/texstorage.c
> >>> +++ b/src/mesa/main/texstorage.c
> >>> @@ -42,7 +42,7 @@
> >>> #include "textureview.h"
> >>> #include "mtypes.h"
> >>> #include "glformats.h"
> >>> -
> >>> +#include "hash.h"
> >>>
> >>>
> >>> /**
> >>> @@ -274,34 +274,25 @@ _mesa_AllocTextureStorage_sw(struct gl_context
> >>> *ctx,
> >>> * \return GL_TRUE if any error, GL_FALSE otherwise.
> >>> */
> >>> static GLboolean
> >>> -tex_storage_error_check(struct gl_context *ctx, GLuint dims, GLenum
> >>> target,
> >>> +tex_storage_error_check(struct gl_context *ctx,
> >>> + struct gl_texture_object *texObj,
> >>> + GLuint dims, GLenum target,
> >>> GLsizei levels, GLenum internalformat,
> >>> - GLsizei width, GLsizei height, GLsizei depth)
> >>> + GLsizei width, GLsizei height, GLsizei depth,
> >>> + bool dsa)
> >>> {
> >>> - struct gl_texture_object *texObj;
> >>>
> >>> - if (!_mesa_is_legal_tex_storage_format(ctx, internalformat)) {
> >>> - _mesa_error(ctx, GL_INVALID_ENUM,
> >>> - "glTexStorage%uD(internalformat = %s)", dims,
> >>> - _mesa_lookup_enum_by_nr(internalformat));
> >>> - return GL_TRUE;
> >>> - }
> >>> + /* Legal format checking has been moved to texstorage and
> >>> texturestorage in
> >>> + * order to allow meta functions to use legacy formats. */
> >>>
> >>> /* size check */
> >>> if (width < 1 || height < 1 || depth < 1) {
> >>> _mesa_error(ctx, GL_INVALID_VALUE,
> >>> - "glTexStorage%uD(width, height or depth < 1)",
> dims);
> >>> + "glTex%sStorage%uD(width, height or depth < 1)",
> >>> + dsa ? "ture" : "", dims);
> >>
> >>
> >> This kind of string replacement seems clunky, and often repeated in this
> >> patch and at least 12/41.
> >>
> >> How about declaring a local var:
> >>
> >> const char *func = dsa ? "glTextureStorage" : "glTexStorage";
> >>
> >> and using 'func' in the _mesa_error() calls?
> >>
> >>
> >>
> >>
> >>> return GL_TRUE;
> >>> }
> >>>
> >>> - /* target check */
> >>> - if (!legal_texobj_target(ctx, dims, target)) {
> >>> - _mesa_error(ctx, GL_INVALID_ENUM,
> >>> - "glTexStorage%uD(illegal target=%s)",
> >>> - dims, _mesa_lookup_enum_by_nr(target));
> >>> - return GL_TRUE;
> >>> - }
> >>> -
> >>> /* From section 3.8.6, page 146 of OpenGL ES 3.0 spec:
> >>> *
> >>> * "The ETC2/EAC texture compression algorithm supports only
> >>> @@ -315,50 +306,55 @@ tex_storage_error_check(struct gl_context *ctx,
> >>> GLuint dims, GLenum target,
> >>> && !_mesa_target_can_be_compressed(ctx, target,
> internalformat))
> >>> {
> >>> _mesa_error(ctx, _mesa_is_desktop_gl(ctx)?
> >>> GL_INVALID_ENUM : GL_INVALID_OPERATION,
> >>> - "glTexStorage3D(internalformat = %s)",
> >>> + "glTex%sStorage%dD(internalformat = %s)",
> >>> + dsa ? "ture" : "", dims,
> >>> _mesa_lookup_enum_by_nr(internalformat));
> >>> }
> >>>
> >>> /* levels check */
> >>> if (levels < 1) {
> >>> - _mesa_error(ctx, GL_INVALID_VALUE, "glTexStorage%uD(levels <
> 1)",
> >>> - dims);
> >>> + _mesa_error(ctx, GL_INVALID_VALUE, "glTex%sStorage%uD(levels <
> >>> 1)",
> >>> + dsa ? "ture" : "", dims);
> >>> return GL_TRUE;
> >>> }
> >>>
> >>> /* check levels against maximum (note different error than above)
> */
> >>> if (levels > (GLint) _mesa_max_texture_levels(ctx, target)) {
> >>> _mesa_error(ctx, GL_INVALID_OPERATION,
> >>> - "glTexStorage%uD(levels too large)", dims);
> >>> + "glTex%sStorage%uD(levels too large)",
> >>> + dsa ? "ture" : "", dims);
> >>> return GL_TRUE;
> >>> }
> >>>
> >>> /* check levels against width/height/depth */
> >>> if (levels > _mesa_get_tex_max_num_levels(target, width, height,
> >>> depth)) {
> >>> _mesa_error(ctx, GL_INVALID_OPERATION,
> >>> - "glTexStorage%uD(too many levels for max texture
> >>> dimension)",
> >>> - dims);
> >>> + "glTex%sStorage%uD(too many levels"
> >>> + " for max texture dimension)",
> >>> + dsa ? "ture" : "", dims);
> >>> return GL_TRUE;
> >>> }
> >>>
> >>> /* non-default texture object check */
> >>> - texObj = _mesa_get_current_tex_object(ctx, target);
> >>> if (!_mesa_is_proxy_texture(target) && (!texObj || (texObj->Name
> ==
> >>> 0))) {
> >>> _mesa_error(ctx, GL_INVALID_OPERATION,
> >>> - "glTexStorage%uD(texture object 0)", dims);
> >>> + "glTex%sStorage%uD(texture object 0)",
> >>> + dsa ? "ture" : "", dims);
> >>> return GL_TRUE;
> >>> }
> >>>
> >>> /* Check if texObj->Immutable is set */
> >>> if (!_mesa_is_proxy_texture(target) && texObj->Immutable) {
> >>> - _mesa_error(ctx, GL_INVALID_OPERATION,
> >>> "glTexStorage%uD(immutable)",
> >>> - dims);
> >>> + _mesa_error(ctx, GL_INVALID_OPERATION,
> >>> "glTex%sStorage%uD(immutable)",
> >>> + dsa ? "ture" : "", dims);
> >>> return GL_TRUE;
> >>> }
> >>>
> >>> /* additional checks for depth textures */
> >>> if (!_mesa_legal_texture_base_format_for_target(ctx, target,
> >>> internalformat,
> >>> - dims,
> >>> "glTexStorage"))
> >>> + dims, dsa ?
> >>> + "glTextureStorage"
> :
> >>> + "glTexStorage"))
> >>> return GL_TRUE;
> >>>
> >>> return GL_FALSE;
> >>> @@ -366,32 +362,26 @@ tex_storage_error_check(struct gl_context *ctx,
> >>> GLuint dims, GLenum target,
> >>>
> >>>
> >>> /**
> >>> - * Helper used by _mesa_TexStorage1/2/3D().
> >>> + * Helper that does the storage allocation for
> _mesa_TexStorage1/2/3D()
> >>> + * and _mesa_TextureStorage1/2/3D().
> >>> */
> >>> -static void
> >>> -texstorage(GLuint dims, GLenum target, GLsizei levels, GLenum
> >>> internalformat,
> >>> - GLsizei width, GLsizei height, GLsizei depth)
> >>> +void
> >>> +_mesa_texture_storage(struct gl_context *ctx, GLuint dims,
> >>> + struct gl_texture_object *texObj,
> >>> + GLenum target, GLsizei levels,
> >>> + GLenum internalformat, GLsizei width,
> >>> + GLsizei height, GLsizei depth, bool dsa)
> >>> {
> >>> - struct gl_texture_object *texObj;
> >>> GLboolean sizeOK, dimensionsOK;
> >>> mesa_format texFormat;
> >>>
> >>> - GET_CURRENT_CONTEXT(ctx);
> >>> -
> >>> - if (MESA_VERBOSE & (VERBOSE_API|VERBOSE_TEXTURE))
> >>> - _mesa_debug(ctx, "glTexStorage%uD %s %d %s %d %d %d\n",
> >>> - dims,
> >>> - _mesa_lookup_enum_by_nr(target), levels,
> >>> - _mesa_lookup_enum_by_nr(internalformat),
> >>> - width, height, depth);
> >>> + assert(texObj);
> >>>
> >>> - if (tex_storage_error_check(ctx, dims, target, levels,
> >>> - internalformat, width, height, depth))
> {
> >>> + if (tex_storage_error_check(ctx, texObj, dims, target, levels,
> >>> + internalformat, width, height, depth,
> >>> dsa)) {
> >>> return; /* error was recorded */
> >>> }
> >>>
> >>> - texObj = _mesa_get_current_tex_object(ctx, target);
> >>> - assert(texObj);
> >>>
> >>> texFormat = _mesa_choose_texture_format(ctx, texObj, target, 0,
> >>> internalformat, GL_NONE,
> >>> GL_NONE);
> >>> @@ -404,7 +394,7 @@ texstorage(GLuint dims, GLenum target, GLsizei
> >>> levels, GLenum internalformat,
> >>> sizeOK = ctx->Driver.TestProxyTexImage(ctx, target, 0, texFormat,
> >>> width, height, depth, 0);
> >>>
> >>> - if (_mesa_is_proxy_texture(texObj->Target)) {
> >>> + if (_mesa_is_proxy_texture(target)) {
> >>> if (dimensionsOK && sizeOK) {
> >>> initialize_texture_fields(ctx, texObj, levels, width,
> height,
> >>> depth,
> >>> internalformat, texFormat);
> >>> @@ -417,13 +407,15 @@ texstorage(GLuint dims, GLenum target, GLsizei
> >>> levels, GLenum internalformat,
> >>> else {
> >>> if (!dimensionsOK) {
> >>> _mesa_error(ctx, GL_INVALID_VALUE,
> >>> - "glTexStorage%uD(invalid width, height or
> depth)",
> >>> dims);
> >>> + "glTex%sStorage%uD(invalid width, height or
> >>> depth)",
> >>> + dsa ? "ture" : "", dims);
> >>> return;
> >>> }
> >>>
> >>> if (!sizeOK) {
> >>> _mesa_error(ctx, GL_OUT_OF_MEMORY,
> >>> - "glTexStorage%uD(texture too large)", dims);
> >>> + "glTex%sStorage%uD(texture too large)",
> >>> + dsa ? "ture" : "", dims);
> >>> }
> >>>
> >>> assert(levels > 0);
> >>> @@ -445,7 +437,8 @@ texstorage(GLuint dims, GLenum target, GLsizei
> >>> levels, GLenum internalformat,
> >>> * state but this puts things in a consistent state.
> >>> */
> >>> clear_texture_fields(ctx, texObj);
> >>> - _mesa_error(ctx, GL_OUT_OF_MEMORY, "glTexStorage%uD", dims);
> >>> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glTex%sStorage%uD",
> >>> + dsa ? "ture" : "", dims);
> >>> return;
> >>> }
> >>>
> >>> @@ -454,6 +447,94 @@ texstorage(GLuint dims, GLenum target, GLsizei
> >>> levels, GLenum internalformat,
> >>> }
> >>> }
> >>>
> >>> +/**
> >>> + * Helper used by _mesa_TexStorage1/2/3D().
> >>> + */
> >>> +static void
> >>> +texstorage(GLuint dims, GLenum target, GLsizei levels, GLenum
> >>> internalformat,
> >>> + GLsizei width, GLsizei height, GLsizei depth)
> >>> +{
> >>> + struct gl_texture_object *texObj;
> >>> + GET_CURRENT_CONTEXT(ctx);
> >>> +
> >>> + /* target check */
> >>> + /* This is done here so that _mesa_texture_storage can receive
> >>> unsized
> >>> + * formats. */
> >>> + if (!legal_texobj_target(ctx, dims, target)) {
> >>> + _mesa_error(ctx, GL_INVALID_ENUM,
> >>> + "glTexStorage%uD(illegal target=%s)",
> >>> + dims, _mesa_lookup_enum_by_nr(target));
> >>> + return;
> >>> + }
> >>> +
> >>> + if (MESA_VERBOSE & (VERBOSE_API|VERBOSE_TEXTURE))
> >>> + _mesa_debug(ctx, "glTexStorage%uD %s %d %s %d %d %d\n",
> >>> + dims,
> >>> + _mesa_lookup_enum_by_nr(target), levels,
> >>> + _mesa_lookup_enum_by_nr(internalformat),
> >>> + width, height, depth);
> >>> + /* Check the format to make sure it is sized. */
> >>> + if (!_mesa_is_legal_tex_storage_format(ctx, internalformat)) {
> >>> + _mesa_error(ctx, GL_INVALID_ENUM,
> >>> + "glTexStorage%uD(internalformat = %s)", dims,
> >>> + _mesa_lookup_enum_by_nr(internalformat));
> >>> + return;
> >>> + }
> >>> +
> >>> + texObj = _mesa_get_current_tex_object(ctx, target);
> >>> + if (!texObj)
> >>> + return;
> >>> +
> >>> + _mesa_texture_storage(ctx, dims, texObj, target, levels,
> >>> + internalformat, width, height, depth, false);
> >>> +}
> >>> +
> >>> +/**
> >>> + * Helper used by _mesa_TextureStorage1/2/3D().
> >>> + */
> >>> +static void
> >>> +texturestorage(GLuint dims, GLuint texture, GLsizei levels,
> >>> + GLenum internalformat, GLsizei width, GLsizei height,
> >>> + GLsizei depth)
> >>> +{
> >>> + struct gl_texture_object *texObj;
> >>> + GET_CURRENT_CONTEXT(ctx);
> >>> +
> >>> + if (MESA_VERBOSE & (VERBOSE_API|VERBOSE_TEXTURE))
> >>> + _mesa_debug(ctx, "glTextureStorage%uD %s %d %s %d %d %d\n",
> >>> + dims,
> >>> + _mesa_lookup_enum_by_nr(texObj->Target), levels,
> texObj is uninitialized here.
> >>> + _mesa_lookup_enum_by_nr(internalformat),
> >>> + width, height, depth);
> >>> + /* Check the format to make sure it is sized. */
> >>> + if (!_mesa_is_legal_tex_storage_format(ctx, internalformat)) {
> >>> + _mesa_error(ctx, GL_INVALID_ENUM,
> >>> + "glTextureStorage%uD(internalformat = %s)", dims,
> >>> + _mesa_lookup_enum_by_nr(internalformat));
> >>> + return;
> >>> + }
> >>> +
> >>> + /* Get the texture object by Name. */
> >>> + texObj = _mesa_lookup_texture(ctx, texture);
> >>> + if (!texObj) {
> >>> + _mesa_error(ctx, GL_INVALID_OPERATION,
> >>> + "glTextureStorage%uD(texture = %d)", dims, texture);
> >>> + return;
> >>> + }
> >>> +
> >>> + /* target check */
> >>> + /* This is done here so that _mesa_texture_storage can receive
> >>> unsized
> >>> + * formats. */
> >>> + if (!legal_texobj_target(ctx, dims, texObj->Target)) {
> >>> + _mesa_error(ctx, GL_INVALID_ENUM,
> >>> + "glTextureStorage%uD(illegal target=%s)",
> >>> + dims, _mesa_lookup_enum_by_nr(texObj->Target));
> >>> + return;
> >>> + }
> >>> +
> >>> + _mesa_texture_storage(ctx, dims, texObj, texObj->Target,
> >>> + levels, internalformat, width, height, depth,
> >>> true);
> >>> +}
> >>>
> >>> void GLAPIENTRY
> >>> _mesa_TexStorage1D(GLenum target, GLsizei levels, GLenum
> >>> internalformat,
> >>> @@ -478,6 +559,28 @@ _mesa_TexStorage3D(GLenum target, GLsizei levels,
> >>> GLenum internalformat,
> >>> texstorage(3, target, levels, internalformat, width, height,
> depth);
> >>> }
> >>>
> >>> +void GLAPIENTRY
> >>> +_mesa_TextureStorage1D(GLuint texture, GLsizei levels, GLenum
> >>> internalformat,
> >>> + GLsizei width)
> >>> +{
> >>> + texturestorage(1, texture, levels, internalformat, width, 1, 1);
> >>> +}
> >>> +
> >>> +
> >>> +void GLAPIENTRY
> >>> +_mesa_TextureStorage2D(GLuint texture, GLsizei levels,
> >>> + GLenum internalformat,
> >>> + GLsizei width, GLsizei height)
> >>> +{
> >>> + texturestorage(2, texture, levels, internalformat, width, height,
> 1);
> >>> +}
> >>> +
> >>> +void GLAPIENTRY
> >>> +_mesa_TextureStorage3D(GLuint texture, GLsizei levels, GLenum
> >>> internalformat,
> >>> + GLsizei width, GLsizei height, GLsizei depth)
> >>> +{
> >>> + texturestorage(3, texture, levels, internalformat, width, height,
> >>> depth);
> >>> +}
> >>>
> >>>
> >>> /*
> >>> diff --git a/src/mesa/main/texstorage.h b/src/mesa/main/texstorage.h
> >>> index 79f228c..7de8080 100644
> >>> --- a/src/mesa/main/texstorage.h
> >>> +++ b/src/mesa/main/texstorage.h
> >>> @@ -26,6 +26,24 @@
> >>> #ifndef TEXSTORAGE_H
> >>> #define TEXSTORAGE_H
> >>>
> >>> +/**
> >>> + * \name Internal functions
> >>> + */
> >>> +/*@{*/
> >>> +
> >>> +extern void
> >>> +_mesa_texture_storage( struct gl_context *ctx, GLuint dims,
> >>> + struct gl_texture_object *texObj,
> >>> + GLenum target, GLsizei levels,
> >>> + GLenum internalformat, GLsizei width,
> >>> + GLsizei height, GLsizei depth, bool dsa );
> >>> +
> >>> +/*@}*/
> >>> +
> >>> +/**
> >>> + * \name API functions
> >>> + */
> >>> +/*@{*/
> >>>
> >>> extern void GLAPIENTRY
> >>> _mesa_TexStorage1D(GLenum target, GLsizei levels, GLenum
> >>> internalformat,
> >>> @@ -41,6 +59,19 @@ extern void GLAPIENTRY
> >>> _mesa_TexStorage3D(GLenum target, GLsizei levels, GLenum
> >>> internalformat,
> >>> GLsizei width, GLsizei height, GLsizei depth);
> >>>
> >>> +extern void GLAPIENTRY
> >>> +_mesa_TextureStorage1D(GLuint texture, GLsizei levels, GLenum
> >>> internalformat,
> >>> + GLsizei width);
> >>> +
> >>> +
> >>> +extern void GLAPIENTRY
> >>> +_mesa_TextureStorage2D(GLuint texture, GLsizei levels, GLenum
> >>> internalformat,
> >>> + GLsizei width, GLsizei height);
> >>> +
> >>> +
> >>> +extern void GLAPIENTRY
> >>> +_mesa_TextureStorage3D(GLuint texture, GLsizei levels, GLenum
> >>> internalformat,
> >>> + GLsizei width, GLsizei height, GLsizei depth);
> >>>
> >>>
> >>> extern void GLAPIENTRY
> >>>
> >>
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
