[Mesa-dev] [v3 6/8] mesa: Fill out ARB_texture_view entry points
Courtney Goeltzenleuchter
courtney at lunarg.com
Mon Nov 25 15:14:52 PST 2013
On Thu, Nov 21, 2013 at 5:56 PM, Brian Paul <brianp at vmware.com> wrote:
> I'd rename this summary as something like "mesa: implement the
> _mesa_TextureView() function"
>
>
>
> On 11/19/2013 04:16 PM, Courtney Goeltzenleuchter wrote:
>
>> Add Mesa TextureView logic.
>> Incorporate feedback on ARB_texture_view
>>
>> Signed-off-by: Courtney Goeltzenleuchter <courtney at LunarG.com>
>> ---
>> src/mesa/main/textureview.c | 562 ++++++++++++++++++++++++++++++
>> +++++++++++++-
>> 1 file changed, 561 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/main/textureview.c b/src/mesa/main/textureview.c
>> index 4a6bd62..1858465 100644
>> --- a/src/mesa/main/textureview.c
>> +++ b/src/mesa/main/textureview.c
>> @@ -40,8 +40,346 @@
>> #include "texobj.h"
>> #include "texstorage.h"
>> #include "textureview.h"
>> +#include "stdbool.h"
>> #include "mtypes.h"
>>
>> +/* Table 3.X.2 (Compatible internal formats for TextureView)
>> + ------------------------------------------------------------
>> ---------------
>> + | Class | Internal formats
>> |
>> + ------------------------------------------------------------
>> ---------------
>> + | VIEW_CLASS_128_BITS | RGBA32F, RGBA32UI, RGBA32I
>> |
>> + ------------------------------------------------------------
>> ---------------
>> + | VIEW_CLASS_96_BITS | RGB32F, RGB32UI, RGB32I
>> |
>> + ------------------------------------------------------------
>> ---------------
>> + | VIEW_CLASS_64_BITS | RGBA16F, RG32F, RGBA16UI, RG32UI, RGBA16I,
>> |
>> + | | RG32I, RGBA16, RGBA16_SNORM
>> |
>> + ------------------------------------------------------------
>> ---------------
>> + | VIEW_CLASS_48_BITS | RGB16, RGB16_SNORM, RGB16F, RGB16UI,
>> RGB16I |
>> + ------------------------------------------------------------
>> ---------------
>> + | VIEW_CLASS_32_BITS | RG16F, R11F_G11F_B10F, R32F,
>> |
>> + | | RGB10_A2UI, RGBA8UI, RG16UI, R32UI,
>> |
>> + | | RGBA8I, RG16I, R32I, RGB10_A2, RGBA8,
>> RG16, |
>> + | | RGBA8_SNORM, RG16_SNORM, SRGB8_ALPHA8,
>> RGB9_E5 |
>> + ------------------------------------------------------------
>> ---------------
>> + | VIEW_CLASS_24_BITS | RGB8, RGB8_SNORM, SRGB8, RGB8UI, RGB8I
>> |
>> + ------------------------------------------------------------
>> ---------------
>> + | VIEW_CLASS_16_BITS | R16F, RG8UI, R16UI, RG8I, R16I, RG8, R16,
>> |
>> + | | RG8_SNORM, R16_SNORM
>> |
>> + ------------------------------------------------------------
>> ---------------
>> + | VIEW_CLASS_8_BITS | R8UI, R8I, R8, R8_SNORM
>> |
>> + ------------------------------------------------------------
>> ---------------
>> + | VIEW_CLASS_RGTC1_RED | COMPRESSED_RED_RGTC1,
>> |
>> + | | COMPRESSED_SIGNED_RED_RGTC1
>> |
>> + ------------------------------------------------------------
>> ---------------
>> + | VIEW_CLASS_RGTC2_RG | COMPRESSED_RG_RGTC2,
>> |
>> + | | COMPRESSED_SIGNED_RG_RGTC2
>> |
>> + ------------------------------------------------------------
>> ---------------
>> + | VIEW_CLASS_BPTC_UNORM | COMPRESSED_RGBA_BPTC_UNORM,
>> |
>> + | | COMPRESSED_SRGB_ALPHA_BPTC_UNORM
>> |
>> + ------------------------------------------------------------
>> ---------------
>> + | VIEW_CLASS_BPTC_FLOAT | COMPRESSED_RGB_BPTC_SIGNED_FLOAT,
>> |
>> + | | COMPRESSED_RGB_BPTC_UNSIGNED_FLOAT
>> |
>> + ------------------------------------------------------------
>> ---------------
>> + */
>> +struct internal_format_class_info {
>> + GLenum view_class;
>> + GLenum internal_format;
>> +};
>> +#define INFO(c,f) {GL_##c, GL_##f}
>> +static const struct internal_format_class_info
>> _compatible_internal_formats[] = {
>> + INFO(VIEW_CLASS_128_BITS, RGBA32F),
>>
>
> If the point of the macro is just to save typing GL_ prefixes, I don't
> think it's worth it.
>
I mostly did it so that I could copy & paste the values from the spec to
here.
>
> The leading underscore on _compatible_internal_formats isn't really
> necessary.
underscore removed.
>
>
>
> + INFO(VIEW_CLASS_128_BITS, RGBA32UI),
>> + INFO(VIEW_CLASS_128_BITS, RGBA32I),
>> + INFO(VIEW_CLASS_96_BITS, RGB32F),
>> + INFO(VIEW_CLASS_96_BITS, RGB32UI),
>> + INFO(VIEW_CLASS_96_BITS, RGB32I),
>> + INFO(VIEW_CLASS_64_BITS, RGBA16F),
>> + INFO(VIEW_CLASS_64_BITS, RG32F),
>> + INFO(VIEW_CLASS_64_BITS, RGBA16UI),
>> + INFO(VIEW_CLASS_64_BITS, RG32UI),
>> + INFO(VIEW_CLASS_64_BITS, RGBA16I),
>> + INFO(VIEW_CLASS_64_BITS, RG32I),
>> + INFO(VIEW_CLASS_64_BITS, RGBA16),
>> + INFO(VIEW_CLASS_64_BITS, RGBA16_SNORM),
>> + INFO(VIEW_CLASS_48_BITS, RGB16),
>> + INFO(VIEW_CLASS_48_BITS, RGB16_SNORM),
>> + INFO(VIEW_CLASS_48_BITS, RGB16F),
>> + INFO(VIEW_CLASS_48_BITS, RGB16UI),
>> + INFO(VIEW_CLASS_48_BITS, RGB16I),
>> + INFO(VIEW_CLASS_32_BITS, RG16F),
>> + INFO(VIEW_CLASS_32_BITS, R11F_G11F_B10F),
>> + INFO(VIEW_CLASS_32_BITS, R32F),
>> + INFO(VIEW_CLASS_32_BITS, RGB10_A2UI),
>> + INFO(VIEW_CLASS_32_BITS, RGBA8UI),
>> + INFO(VIEW_CLASS_32_BITS, RG16UI),
>> + INFO(VIEW_CLASS_32_BITS, R32UI),
>> + INFO(VIEW_CLASS_32_BITS, RGBA8I),
>> + INFO(VIEW_CLASS_32_BITS, RG16I),
>> + INFO(VIEW_CLASS_32_BITS, R32I),
>> + INFO(VIEW_CLASS_32_BITS, RGB10_A2),
>> + INFO(VIEW_CLASS_32_BITS, RGBA8),
>> + INFO(VIEW_CLASS_32_BITS, RG16),
>> + INFO(VIEW_CLASS_32_BITS, RGBA8_SNORM),
>> + INFO(VIEW_CLASS_32_BITS, RG16_SNORM),
>> + INFO(VIEW_CLASS_32_BITS, SRGB8_ALPHA8),
>> + INFO(VIEW_CLASS_32_BITS, RGB9_E5),
>> + INFO(VIEW_CLASS_24_BITS, RGB8),
>> + INFO(VIEW_CLASS_24_BITS, RGB8_SNORM),
>> + INFO(VIEW_CLASS_24_BITS, SRGB8),
>> + INFO(VIEW_CLASS_24_BITS, RGB8UI),
>> + INFO(VIEW_CLASS_24_BITS, RGB8I),
>> + INFO(VIEW_CLASS_16_BITS, R16F),
>> + INFO(VIEW_CLASS_16_BITS, RG8UI),
>> + INFO(VIEW_CLASS_16_BITS, R16UI),
>> + INFO(VIEW_CLASS_16_BITS, RG8I),
>> + INFO(VIEW_CLASS_16_BITS, R16I),
>> + INFO(VIEW_CLASS_16_BITS, RG8),
>> + INFO(VIEW_CLASS_16_BITS, R16),
>> + INFO(VIEW_CLASS_16_BITS, RG8_SNORM),
>> + INFO(VIEW_CLASS_16_BITS, R16_SNORM),
>> + INFO(VIEW_CLASS_8_BITS, R8UI),
>> + INFO(VIEW_CLASS_8_BITS, R8I),
>> + INFO(VIEW_CLASS_8_BITS, R8),
>> + INFO(VIEW_CLASS_8_BITS, R8_SNORM),
>> + INFO(VIEW_CLASS_RGTC1_RED, COMPRESSED_RED_RGTC1),
>> + INFO(VIEW_CLASS_RGTC1_RED, COMPRESSED_SIGNED_RED_RGTC1),
>> + INFO(VIEW_CLASS_RGTC2_RG, COMPRESSED_RG_RGTC2),
>> + INFO(VIEW_CLASS_RGTC2_RG, COMPRESSED_SIGNED_RG_RGTC2),
>> + INFO(VIEW_CLASS_BPTC_UNORM, COMPRESSED_RGBA_BPTC_UNORM_ARB),
>> + INFO(VIEW_CLASS_BPTC_UNORM, COMPRESSED_SRGB_ALPHA_BPTC_UNORM_ARB),
>> + INFO(VIEW_CLASS_BPTC_FLOAT, COMPRESSED_RGB_BPTC_SIGNED_FLOAT_ARB),
>> + INFO(VIEW_CLASS_BPTC_FLOAT, COMPRESSED_RGB_BPTC_UNSIGNED_FLOAT_ARB),
>> +};
>> +#undef INFO
>> +
>> +/**
>> + * Lookup format view class based on internalformat
>> + * \return VIEW_CLASS if internalformat found in table, GL_FALSE
>> otherwise.
>> + */
>> +static unsigned int
>>
>
> Return GLenum?
I had feedback elsewhere that it was better to stay away from GLenums for
internal functions.
So I could either change the comment to match the return of false. Hmm, as
I look at this again the function is going to either return a
GL_VIEW_CLASS... enum from the lookup table or false. Yeah, probably make
more sense to just be GLenum for consistency.
>
>
>
> +lookup_view_class(GLenum internalformat)
>> +{
>> + uint i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(_compatible_internal_formats); i++) {
>> + if (_compatible_internal_formats[i].internal_format ==
>> internalformat)
>> + return _compatible_internal_formats[i].view_class;
>> + }
>> + return false;
>>
>
> return GL_FALSE, per the comment.
got it.
>
>
>
> +}
>> +
>> +/**
>> + * Compute the size of the next mipmap level.
>> + */
>> +static void
>> +next_mipmap_level_size(GLenum target,
>> + GLint *width, GLint *height, GLint *depth)
>> +{
>> + if (*width > 1) {
>> + *width /= 2;
>> + }
>> +
>> + if ((*height > 1) && (target != GL_TEXTURE_1D_ARRAY)) {
>> + *height /= 2;
>> + }
>> +
>> + if ((*depth > 1) && (target != GL_TEXTURE_2D_ARRAY)) {
>> + *depth /= 2;
>> + }
>> +}
>>
>
> Can the next_mipmap_level_size() helper in mipmap.c be re-used?
>
> And yes, we should probably re-use that function in texstorage.c
Sure, that will work. So will one in texstorage.c.
Okay, I'll make both texstorage.c and textureview.c use the mipmap function.
>
>
> +
>> +/** Helper to get a particular texture image in a texture object */
>> +static struct gl_texture_image *
>> +get_tex_image(struct gl_context *ctx,
>> + struct gl_texture_object *texObj,
>> + GLuint face, GLuint level)
>> +{
>> + struct gl_texture_image *texImage;
>> +
>> + if (!texObj)
>> + return NULL;
>> +
>> + texImage = texObj->Image[face][level];
>> + if (!texImage) {
>> + texImage = ctx->Driver.NewTextureImage(ctx);
>> + if (!texImage) {
>> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "texture image allocation");
>> + return NULL;
>> + }
>> +
>> + texObj->Image[face][level] = texImage;
>> +
>> + /* Set the 'back' pointer */
>> + texImage->TexObject = texObj;
>> + texImage->Level = level;
>> + texImage->Face = face;
>> + }
>> +
>> + return texImage;
>> +}
>>
>
> That function looks a lot like _mesa_get_tex_image(), or get_tex_image()
> in texstorage.c.
Hadn't seen that one. _mesa_get_tex_image() does the same thing.
>
>
>
>
> +
>> +/**
>> + * Initialize new texture's gl_texture_image structures. Will not call
>> driver
>> + * to allocate new space, simply record relevant layer, face, format,
>> etc.
>> + * \return GL_FALSE if any error, GL_TRUE otherwise.
>> + */
>> +static GLboolean
>> +initialize_texture_fields(struct gl_context *ctx,
>> + GLenum target,
>> + struct gl_texture_object *texObj,
>> + GLint levels,
>> + GLsizei width, GLsizei height, GLsizei depth,
>> + GLenum internalFormat, gl_format texFormat)
>> +{
>> + const GLuint numFaces = _mesa_num_tex_faces(target);
>> + GLint level, levelWidth = width, levelHeight = height, levelDepth =
>> depth;
>> + GLuint face;
>> +
>> + /* Pretend we are bound to initialize the gl_texture_image structs */
>> + texObj->Target = target;
>> +
>> + /* Set up all the texture object's gl_texture_images */
>> + for (level = 0; level < levels; level++) {
>> + for (face = 0; face < numFaces; face++) {
>> + struct gl_texture_image *texImage =
>> + get_tex_image(ctx, texObj, face, level);
>> +
>> + if (!texImage) {
>> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glTexStorage");
>> + return GL_FALSE;
>> + }
>> +
>> + _mesa_init_teximage_fields(ctx, texImage,
>> + levelWidth, levelHeight, levelDepth,
>> + 0, internalFormat, texFormat);
>> + }
>> +
>> + next_mipmap_level_size(target, &levelWidth, &levelHeight,
>> &levelDepth);
>> + }
>> +
>> + /* "unbind" */
>> + texObj->Target = 0;
>> +
>> + return GL_TRUE;
>> +}
>> +
>> +#define RETURN_IF_SUPPORTED(t) do { \
>> + if (newTarget == GL_ ## t) \
>> + return GL_TRUE; \
>> +} while (0)
>> +
>> +/**
>> + * Check for compatible target
>> + * If an error is found, record it with _mesa_error()
>> + * \return GL_FALSE if any error, GL_TRUE otherwise.
>> + */
>> +static GLboolean
>> +target_valid(struct gl_context *ctx, GLenum origTarget, GLenum newTarget)
>> +{
>> + /*
>> + * From ARB_texture_view spec:
>> + ------------------------------------------------------------
>> ---------------------------------------------
>> + | Original target | Valid new targets |
>> + ------------------------------------------------------------
>> ---------------------------------------------
>> + | TEXTURE_1D | TEXTURE_1D, TEXTURE_1D_ARRAY |
>> + | ------------------------------------------------------------
>> ------------------------------------------- |
>> + | TEXTURE_2D | TEXTURE_2D, TEXTURE_2D_ARRAY |
>> + | ------------------------------------------------------------
>> ------------------------------------------- |
>> + | TEXTURE_3D | TEXTURE_3D |
>> + | ------------------------------------------------------------
>> ------------------------------------------- |
>> + | TEXTURE_CUBE_MAP | TEXTURE_CUBE_MAP, TEXTURE_2D,
>> TEXTURE_2D_ARRAY, TEXTURE_CUBE_MAP_ARRAY |
>> + | ------------------------------------------------------------
>> ------------------------------------------- |
>> + | TEXTURE_RECTANGLE | TEXTURE_RECTANGLE |
>> + | ------------------------------------------------------------
>> ------------------------------------------- |
>> + | TEXTURE_BUFFER | <none> |
>> + | ------------------------------------------------------------
>> ------------------------------------------- |
>> + | TEXTURE_1D_ARRAY | TEXTURE_1D_ARRAY, TEXTURE_1D |
>> + | ------------------------------------------------------------
>> ------------------------------------------- |
>> + | TEXTURE_2D_ARRAY | TEXTURE_2D_ARRAY, TEXTURE_2D,
>> TEXTURE_CUBE_MAP, TEXTURE_CUBE_MAP_ARRAY |
>> + | ------------------------------------------------------------
>> ------------------------------------------- |
>> + | TEXTURE_CUBE_MAP_ARRAY | TEXTURE_CUBE_MAP_ARRAY,
>> TEXTURE_2D_ARRAY, TEXTURE_2D, TEXTURE_CUBE_MAP |
>> + | ------------------------------------------------------------
>> ------------------------------------------- |
>> + | TEXTURE_2D_MULTISAMPLE | TEXTURE_2D_MULTISAMPLE,
>> TEXTURE_2D_MULTISAMPLE_ARRAY |
>> + | ------------------------------------------------------------
>> ------------------------------------------- |
>> + | TEXTURE_2D_MULTISAMPLE_ARRAY | TEXTURE_2D_MULTISAMPLE,
>> TEXTURE_2D_MULTISAMPLE_ARRAY |
>> + ------------------------------------------------------------
>> ---------------------------------------------
>> + */
>> +
>> + switch (origTarget) {
>> + case GL_TEXTURE_1D:
>> + case GL_TEXTURE_1D_ARRAY:
>> + RETURN_IF_SUPPORTED(TEXTURE_1D);
>> + RETURN_IF_SUPPORTED(TEXTURE_1D_ARRAY);
>> + break;
>> + case GL_TEXTURE_2D:
>> + RETURN_IF_SUPPORTED(TEXTURE_2D);
>> + RETURN_IF_SUPPORTED(TEXTURE_2D_ARRAY);
>> + break;
>> + case GL_TEXTURE_3D:
>> + RETURN_IF_SUPPORTED(TEXTURE_3D);
>> + break;
>> + case GL_TEXTURE_RECTANGLE:
>> + RETURN_IF_SUPPORTED(TEXTURE_RECTANGLE);
>> + break;
>> + case GL_TEXTURE_CUBE_MAP:
>> + case GL_TEXTURE_2D_ARRAY:
>> + case GL_TEXTURE_CUBE_MAP_ARRAY:
>> + RETURN_IF_SUPPORTED(TEXTURE_2D);
>> + RETURN_IF_SUPPORTED(TEXTURE_2D_ARRAY);
>> + RETURN_IF_SUPPORTED(TEXTURE_CUBE_MAP);
>> + RETURN_IF_SUPPORTED(TEXTURE_CUBE_MAP_ARRAY);
>> + break;
>> + case GL_TEXTURE_2D_MULTISAMPLE:
>> + case GL_TEXTURE_2D_MULTISAMPLE_ARRAY:
>> + RETURN_IF_SUPPORTED(TEXTURE_2D_MULTISAMPLE);
>> + RETURN_IF_SUPPORTED(TEXTURE_2D_MULTISAMPLE_ARRAY);
>> + break;
>> + }
>> + _mesa_error(ctx, GL_INVALID_OPERATION,
>> + "glTextureView(illegal target=%s)",
>> + _mesa_lookup_enum_by_nr(newTarget));
>> + return GL_FALSE;
>>
>
> Typically, we put a _mesa_error() call like that after a default switch
> case.
I modeled this function after _mesa_choose_tex_format which uses the same
pattern. Default case will not catch all the error conditions.
>
>
>
> +}
>> +#undef RETURN_IF_SUPPORTED
>> +
>> +/**
>> + * Check for compatible format
>> + * If an error is found, record it with _mesa_error()
>> + * \return GL_FALSE if any error, GL_TRUE otherwise.
>> + */
>> +static GLboolean
>>
>
> Since this is an internal function, it could return bool.
OK
>
>
>
> +compatible_format(struct gl_context *ctx, struct gl_texture_object
>> *origTexObj,
>>
>
> origTexObj could be const qualified.
OK
>
>
>
> + GLenum internalformat)
>> +{
>> + /* Level 0 of a texture created by glTextureStorage or glTextureView
>> + * is always defined.
>> + */
>> + struct gl_texture_image *texImage = origTexObj->Image[0][0];
>> + GLint origInternalFormat = texImage->InternalFormat;
>> + unsigned int origViewClass, newViewClass;
>> +
>> +
>> + /* The two textures' internal formats must be compatible according to
>> + * Table 3.X.2 (Compatible internal formats for TextureView)
>> + * if the internal format exists in that table the view class must
>> match.
>> + * The internal formats must be identical if not in that table,
>> + * or an INVALID_OPERATION error is generated.
>> + */
>> + if (origInternalFormat == internalformat)
>> + return GL_TRUE;
>> +
>> + origViewClass = lookup_view_class(origInternalFormat);
>> + newViewClass = lookup_view_class(internalformat);
>> + if ((origViewClass == newViewClass) && origViewClass != false)
>>
>
> != GL_FALSE
Another internal function, I'll change it to bool.
>
>
>
> + return GL_TRUE;
>> +
>> + _mesa_error(ctx, GL_INVALID_OPERATION,
>> + "glTextureView(internalformat %s not compatible with
>> origtexture %s)",
>>
>
> Extra space before first %s
>
>
>
> + _mesa_lookup_enum_by_nr(internalformat),
>> + _mesa_lookup_enum_by_nr(origInternalFormat));
>> + return GL_FALSE;
>> +}
>> +
>> /**
>> * glTextureView (ARB_texture_view)
>> * If an error is found, record it with _mesa_error()
>> @@ -53,13 +391,235 @@ _mesa_TextureView(GLuint texture, GLenum target,
>> GLuint origtexture,
>> GLuint minlevel, GLuint numlevels,
>> GLuint minlayer, GLuint numlayers)
>> {
>> + struct gl_texture_object *texObj;
>> + struct gl_texture_object *origTexObj;
>> + struct gl_texture_image *origTexImage;
>> + GLuint newViewMinLevel, newViewMinLayer;
>> + GLuint newViewNumLevels, newViewNumLayers;
>> + GLsizei width, height, depth;
>> + gl_format texFormat;
>> + GLboolean sizeOK, dimensionsOK;
>> + GLenum faceTarget;
>> +
>> GET_CURRENT_CONTEXT(ctx);
>>
>> if (MESA_VERBOSE & (VERBOSE_API | VERBOSE_TEXTURE))
>> - _mesa_debug(ctx, "glTextureView (unfinished) %d %s %d %s %d %d %d
>> %d\n",
>> + _mesa_debug(ctx, "glTextureView %d %s %d %s %d %d %d %d\n",
>> texture, _mesa_lookup_enum_by_nr(target),
>> origtexture,
>> _mesa_lookup_enum_by_nr(internalformat),
>> minlevel, numlevels, minlayer, numlayers);
>>
>> + if (origtexture == 0) {
>> + _mesa_error(ctx, GL_INVALID_VALUE, "glTextureView(origtexture =
>> 0x%x)", origtexture);
>> + return;
>> + }
>> +
>> + /* Need original texture information to validate arguments */
>> + origTexObj = _mesa_lookup_texture(ctx, origtexture);
>> +
>> + /* If <origtexture> is not the name of a texture, INVALID_VALUE is
>> generated. */
>> + if (!origTexObj) {
>> + _mesa_error(ctx, GL_INVALID_VALUE, "glTextureView(origtexture =
>> 0x%x)", origtexture);
>>
>
> I think we usually print/record object IDs as decimal.
OK
>
>
>
> + return;
>> + }
>> +
>> + /* If <origtexture>'s TEXTURE_IMMUTABLE_FORMAT value is not TRUE,
>> + * INVALID_OPERATION is generated.
>> + */
>> + if (!origTexObj->Immutable) {
>> + _mesa_error(ctx, GL_INVALID_OPERATION, "glTextureView(origtexture
>> not immutable)");
>> + return;
>> + }
>> +
>> + /* If <texture> is 0, INVALID_VALUE is generated. */
>> + if (texture == 0) {
>> + _mesa_error(ctx, GL_INVALID_VALUE, "glTextureView(texture = 0)");
>> + return;
>> + }
>> +
>> + /* If <texture> is not a valid name returned by GenTextures,
>> + * the error INVALID_OPERATION is generated.
>> + */
>> + texObj = _mesa_lookup_texture(ctx, texture);
>> + if (texObj == NULL) {
>> + _mesa_error(ctx, GL_INVALID_OPERATION, "glTextureView(texture 0x%x
>> non-gen name)", texture);
>> + return;
>> + }
>> +
>> + /* If <texture> has already been bound and given a target, then
>> + * the error INVALID_OPERATION is generated.
>> + */
>> + if (texObj->Target) {
>> + _mesa_error(ctx, GL_INVALID_OPERATION, "glTextureView(texture 0x%x
>> already bound)", texture);
>> + return;
>> + }
>> +
>> + /* Check for compatible target */
>> + if (!target_valid(ctx, origTexObj->Target, target)) {
>> + return; /* error was recorded */
>> + }
>> +
>> + /* minlevel and minlayer are relative to the view of origtexture */
>> + /* If minlevel or minlayer is greater than level or layer,
>> respectively,
>> + * of origtexture return INVALID_VALUE.
>> + */
>> + newViewMinLevel = origTexObj->MinLevel + minlevel;
>> + newViewMinLayer = origTexObj->MinLayer + minlayer;
>> + if (newViewMinLevel >= (origTexObj->MinLevel + origTexObj->NumLevels)
>> ||
>> + newViewMinLayer >= (origTexObj->MinLayer +
>> origTexObj->NumLayers)) {
>> + _mesa_error(ctx, GL_INVALID_VALUE, "glTextureView(origtexture =
>> 0x%x)", origtexture);
>> + return;
>> + }
>>
>
> I'd probably split the level and layer error checks into two separate if
> statements. Then, the error message could have more detailed info about
> the invalid layer or level.
>
>
>
>
> + if (!compatible_format(ctx, origTexObj, internalformat)) {
>> + return; // Error logged
>>
>
> /* error logged */ just to be consistent with other comments.
OK
>
>
>
> + }
>> +
>> + texFormat = _mesa_choose_texture_format(ctx, texObj, target, 0,
>> + internalformat, GL_NONE,
>> GL_NONE);
>> + assert(texFormat != MESA_FORMAT_NONE);
>>
>
> Just to be safe, let's also put in a "if (texFormat == MESA_FORMAT_NONE)
> return" here for release builds.
OK
>
>
>
> +
>> + newViewNumLevels = MIN2(numlevels, origTexObj->NumLevels - minlevel);
>> + newViewNumLayers = MIN2(numlayers, origTexObj->NumLayers - minlayer);
>> +
>> + faceTarget = origTexObj->Target;
>> + if (faceTarget == GL_TEXTURE_CUBE_MAP)
>>
>
> or GL_TEXTURE_CUBE_MAP_ARRAY?
That's a great question! I believe the answer is no. As I understand it,
for a CUBE_MAP the numlayers parameter corresponds to the faces (a CUBE_MAP
will always have numlayers = 6). For a CUBE_MAP_ARRAY the numlayers refers
to the depth not the face.
>
>
> + faceTarget = GL_TEXTURE_CUBE_MAP_POSITIVE_X + minlayer;
>> +
>> + /* Get a reference to what will become this View's base level */
>> + origTexImage = _mesa_select_tex_image(ctx, origTexObj,
>> + faceTarget, minlevel);
>> + width = origTexImage->Width;
>> + height = origTexImage->Height;
>> + depth = origTexImage->Depth;
>> +
>> + /* Adjust width, height, depth to be appropriate for new target */
>> + switch (target) {
>> + case GL_TEXTURE_1D:
>> + case GL_TEXTURE_3D:
>> + break;
>> +
>> + case GL_TEXTURE_1D_ARRAY:
>> + height = (GLsizei) newViewNumLayers;
>> + break;
>> +
>> + case GL_TEXTURE_2D:
>> + case GL_TEXTURE_2D_MULTISAMPLE:
>> + case GL_TEXTURE_RECTANGLE:
>> + case GL_TEXTURE_CUBE_MAP:
>> + depth = 1;
>> + break;
>> +
>> + case GL_TEXTURE_2D_ARRAY:
>> + case GL_TEXTURE_CUBE_MAP_ARRAY:
>> + case GL_TEXTURE_2D_MULTISAMPLE_ARRAY:
>> + depth = newViewNumLayers;
>> + break;
>> + }
>> +
>> + /* If the dimensions of the original texture are larger than the
>> maximum
>> + * supported dimensions of the new target, the error
>> INVALID_OPERATION is
>> + * generated. For example, if the original texture has a
>> TEXTURE_2D_ARRAY
>> + * target and its width is greater than MAX_CUBE_MAP_TEXTURE_SIZE, an
>> error
>> + * will be generated if TextureView is called to create a
>> TEXTURE_CUBE_MAP
>> + * view.
>> + */
>> + dimensionsOK = _mesa_legal_texture_dimensions(ctx, target, 0,
>> + width, height, depth,
>> 0);
>> + if (!dimensionsOK) {
>> + _mesa_error(ctx, GL_INVALID_OPERATION, "glTextureView(invalid
>> width or height or depth)");
>> + return;
>> + }
>> +
>> + sizeOK = ctx->Driver.TestProxyTexImage(ctx, target, 0, texFormat,
>> + width, height, depth, 0);
>> + if (!sizeOK) {
>> + _mesa_error(ctx, GL_INVALID_OPERATION, "glTextureView(invalid
>> texture size)");
>> + return;
>> + }
>> +
>> + /* If <target> is TEXTURE_1D, TEXTURE_2D, TEXTURE_3D,
>> TEXTURE_RECTANGLE,
>> + * or TEXTURE_2D_MULTISAMPLE and <numlayers> does not equal 1, the
>> error
>> + * INVALID_VALUE is generated.
>> + */
>> + switch (target) {
>> + case GL_TEXTURE_1D:
>> + case GL_TEXTURE_2D:
>> + case GL_TEXTURE_3D:
>> + case GL_TEXTURE_RECTANGLE:
>> + case GL_TEXTURE_2D_MULTISAMPLE:
>> + if (numlayers != 1) {
>> + _mesa_error(ctx, GL_INVALID_VALUE, "glTextureView(numlayers %d
>> != 1)", numlayers);
>> + return;
>> + }
>> + break;
>> +
>> + case GL_TEXTURE_CUBE_MAP:
>> + /* If the new texture's target is TEXTURE_CUBE_MAP, the clamped
>> <numlayers>
>> + * must be equal to 6.
>> + */
>> + if (newViewNumLayers != 6) {
>> + _mesa_error(ctx, GL_INVALID_VALUE, "glTextureView(clamped
>> numlayers %d != 6)",
>> + newViewNumLayers);
>> + return;
>> + }
>> + break;
>> +
>> + case GL_TEXTURE_CUBE_MAP_ARRAY:
>> + /* If the new texture's target is TEXTURE_CUBE_MAP_ARRAY,
>> + * then <numlayers> counts layer-faces rather than layers,
>> + * and the clamped <numlayers> must be a multiple of 6.
>> + * Otherwise, the error INVALID_VALUE is generated.
>> + */
>> + if ((newViewNumLayers % 6) != 0) {
>> + _mesa_error(ctx, GL_INVALID_VALUE, "glTextureView(clamped
>> numlayers %d is not a multiple of 6)",
>>
>
> If that line's longer than 78 chars, can you wrap it?
OK
>
>
>
> + newViewNumLayers);
>> + return;
>> + }
>> + break;
>> + }
>> +
>> + /* If the new texture's target is TEXTURE_CUBE_MAP or
>> + * TEXTURE_CUBE_MAP_ARRAY, the width and height of the original
>> texture's
>> + * levels must be equal otherwise the error INVALID_OPERATION is
>> generated.
>> + */
>> + if ((target == GL_TEXTURE_CUBE_MAP || target ==
>> GL_TEXTURE_CUBE_MAP_ARRAY) &&
>> + (origTexImage->Width != origTexImage->Height)) {
>> + _mesa_error(ctx, GL_INVALID_OPERATION, "glTextureView(origtexture
>> width (%d) != height (%d))",
>> + origTexImage->Width, origTexImage->Height);
>> + return;
>> + }
>> +
>> + /* When the original texture's target is TEXTURE_CUBE_MAP, the layer
>> + * parameters are interpreted in the same order as if it were a
>> + * TEXTURE_CUBE_MAP_ARRAY with 6 layer-faces.
>> + */
>> +
>> + /* If the internal format does not exactly match the internal format
>> of the
>> + * original texture, the contents of the memory are reinterpreted in
>> the
>> + * same manner as for image bindings described in
>> + * section 3.9.20 (Texture Image Loads and Stores).
>> + */
>> +
>> + /* TEXTURE_BASE_LEVEL and TEXTURE_MAX_LEVEL are interpreted
>> + * relative to the view and not relative to the original data store.
>> + */
>> +
>> + if (!initialize_texture_fields(ctx, target, texObj, newViewNumLevels,
>> + width, height, depth,
>> + internalformat, texFormat)) {
>> + return; /* Already recorded error */
>> + }
>> +
>> + texObj->MinLevel = newViewMinLevel;
>> + texObj->MinLayer = newViewMinLayer;
>> + texObj->NumLevels = newViewNumLevels;
>> + texObj->NumLayers = newViewNumLayers;
>> + texObj->Immutable = GL_TRUE;
>> + texObj->ImmutableLevels = origTexObj->ImmutableLevels;
>> + texObj->Target = target;
>>
>
> Maybe these assignments should be rolled into initialize_texture_fields()?
Why?
initialize_texture_fields is initializing the teximage properties of the
texture whereas the assignments are setting the texture object's properties.
Thanks for the feedback!
>
>
>
>> + if (!ctx->Driver.TextureView(ctx, texObj, origTexObj)) {
>> + return; /* driver recorded error */
>> + }
>> }
>>
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
--
Courtney Goeltzenleuchter
LunarG
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20131125/a9876bfa/attachment-0001.html>
More information about the mesa-dev
mailing list