<div dir="ltr"><div>These comments have been addressed: <a href="http://cgit.freedesktop.org/~ldeks/mesa/commit/?h=adsa-textures&id=a2f20c936f6db31986220938db06cf28885e7ee6">http://cgit.freedesktop.org/~ldeks/mesa/commit/?h=adsa-textures&id=a2f20c936f6db31986220938db06cf28885e7ee6</a>. </div>Thanks. Laura </div><div class="gmail_extra"> <div class="gmail_quote">On Wed, Dec 31, 2014 at 6:03 PM, Anuj Phogat <<a href="mailto:anuj.phogat@gmail.com" target="_blank">anuj.phogat@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Tue, Dec 16, 2014 at 10:59 AM, Laura Ekstrand <<a href="mailto:laura@jlekstrand.net">laura@jlekstrand.net</a>> 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 <<a href="mailto:brianp@vmware.com">brianp@vmware.com</a>> 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, </div></div>texObj is uninitialized here. <div><div class="h5">>>> + _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 >>> >> > </div></div>> _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> > </blockquote></div> </div>