[Mesa-dev] [Patch v3] Mesa: Add support for GL_OES_texture_*float* extensions.
kalyan kondapally
kondapallykalyancontribute at gmail.com
Wed Dec 3 08:09:56 PST 2014
>>I think this is a bad idea. Right now OpenGL ES 3.0 support depends on
>>ARB_texture_float (see compute_version_es2 in src/mesa/main/version.c).
Hmm Missed this :(
>>My suggestion: just enable the full set of extensions in the i965
>>driver.
K, will revert back to original version.
>>If we care about saving a
>>couple lines of code here and there, we should make a function that
>>drivers explicitly call to do that.
Will try do this once initial version gets merged.
Br,
Kalyan
On Tue, Dec 2, 2014 at 12:36 PM, Ian Romanick <idr at freedesktop.org> wrote:
> On 11/28/2014 01:08 PM, Kalyan Kondapally wrote:
>> This patch adds support for following GLES2 Texture Float extensions:
>> 1)GL_OES_texture_float,
>> 2)GL_OES_texture_half_float,
>> 3)GL_OES_texture_float_linear,
>> 4)GL_OES_texture_half_float_linear.
>>
>> If we are using GLES context and the driver has advertised support for
>> ARB_texture_float, then support for all texture_float* extensions is
>> enabled. Here, we are assuming that when driver advertises support for
>> ARB_texture_float, it should be able to support all these extensions.
>>
>> v2: Indentation fixes. (Brian Paul)
>> Fixed Comments and added some to new private functions.(Brian Paul)
>> Added assert in valid_filter_for_float.(Brian Paul)
>> Renamed Float and HALF_FLOAT_OES as IsFloatOES and IsHalfFloatOES.(Brian Paul)
>> adjust_for_oes_float_texture to return GLenum. (Brian Paul)
>> Use RGB_32F internal floating point format for RGB base format.
>>
>> v3: Don't indent case. (Matt Turner)
>> Enable support for float extensions in case driver supports
>> ARB_texture_float. (Fredrik Höglund)
>
> At this point, I think this patch should be split in three:
>
> 1. Extension infrastructure bits.
>
> 2. GL_HALF_FLOAT_OES fixes.
>
> 3. The rest.
>
> I suggest this because I think the first two should be pretty much
> landable. "The rest" may still need some changes.
>
> I'd also accept a 0th patch.
>
> 0. Global s/GL_HALF_FLOAT_ARB/GL_HALF_FLOAT/g;s/GLhalfARB/GLhalf/g
>
> :)
>
>> Signed-off-by: Kevin Rogovin <kevin.rogovin at intel.com>
>> Signed-off-by: Kalyan Kondapally <kalyan.kondapally at intel.com>
>> ---
>> src/mesa/main/context.c | 15 +++++++++++
>> src/mesa/main/extensions.c | 4 +++
>> src/mesa/main/glformats.c | 46 +++++++++++++++++++++++++++++---
>> src/mesa/main/glformats.h | 3 ++-
>> src/mesa/main/mtypes.h | 6 +++++
>> src/mesa/main/pack.c | 16 +++++++++++
>> src/mesa/main/teximage.c | 66 +++++++++++++++++++++++++++++++++++++++++++++-
>> src/mesa/main/texobj.c | 53 +++++++++++++++++++++++++++++++++++++
>> 8 files changed, 203 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
>> index 400c158..8b54967 100644
>> --- a/src/mesa/main/context.c
>> +++ b/src/mesa/main/context.c
>> @@ -1544,6 +1544,21 @@ handle_first_current(struct gl_context *ctx)
>> return;
>> }
>>
>> + /* If we are using GLES context and the driver has advertised support for
>> + * ARB_texture_float, then enable support for all texture_float* extensions.
>> + * Here, we are assuming that when driver advertises support for
>> + * ARB_texture_float, it should be able to support all these extensions. In
>> + * case any particular driver doesn't want to enable these extensions or
>> + * only a subset on GLES, then it shouldn't advertise support for
>> + * ARB_texture_float and toggle OES_texture_float* flags accordingly.
>> + */
>> + if (ctx->Extensions.ARB_texture_float && _mesa_is_gles(ctx)) {
>> + ctx->Extensions.OES_texture_float = GL_TRUE;
>> + ctx->Extensions.OES_texture_float_linear = GL_TRUE;
>> + ctx->Extensions.OES_texture_half_float = GL_TRUE;
>> + ctx->Extensions.OES_texture_half_float_linear = GL_TRUE;
>> + }
>
> I think this is a bad idea. Right now OpenGL ES 3.0 support depends on
> ARB_texture_float (see compute_version_es2 in src/mesa/main/version.c).
> That's a superset of OpenGL ES 3.0: GL_OES_texture_float_linear is not
> required. Now when someone wants to enable ES3 on their GPU that
> supports everything except GL_OES_texture_float_linear they will have to
> find-and-move this code because this code occurs after the version is
> calculated. That person will also have to modify other drivers (that
> they probably cannot test). Gosh, what could go wrong? :)
>
> There are a bunch of other cases where an extension can be automatically
> enabled if some other conditions are met. If we care about saving a
> couple lines of code here and there, we should make a function that
> drivers explicitly call to do that. Drivers that opt-in can call this
> function after they have enabled all of their other extensions in their
> create-context path.
>
> Moreover, this code is already broken for r300. Marek said in the
> previous thread that r300 advertises GL_ARB_texture_float (and
> developers know the documented caveats there) but does not want to
> advertise GL_OES_texture_*_linear.
>
> For the most part in Mesa, extensions are only enabled by default in
> three cases:
>
> 1. All in-tree drivers were already enabling the extension.
> GL_ARB_texture_env_add is an example. In the long past this extension
> was not enabled by default. Eventually all drivers either grew support
> or were removed from the tree.
>
> 2. The extension is just API "syntactic sugar" that the driver won't
> even see. GL_ARB_multi_bind is an example.
>
> 3. The extension has a secondary enable, such as a number of available
> formats, that drivers can control. GL_ARB_get_program_binary,
> GL_ARB_multisample, and GL_ARB_texture_compression are examples.
>
> In these cases we have a compelling testing story.
>
> - If the extension was already enabled by the driver, then we can assume
> that it has already been tested.
>
> - If an extension is syntactic sugar, then testing on driver X should be
> good enough for driver Y.
>
> - If an extension has a secondary enable, tests (and applications)
> should skip the functionality when the secondary enable is not set.
> Alas, this does not always hold, and this is why GL_ARB_occlusion_query
> is not enabled by default with GL_QUERY_COUNTER_BITS = 0.
>
> Especially given the existence of things like meta, enabling existing
> functionaly in a new API can have surprising results. I'd much prefer
> to get buy-in from other driver maintainers before enabling this by
> default. That buy-in generally comes in the form of patches that enable
> the functionality. :) See the GL_ARB_texture_env_add example above.
>
> Another way to think of it is that we have two competing goals. On one
> hand, we want to generally raise up the functionality supported by Mesa
> drivers, share common code, and avoid unnecessary extension checks. On
> the other hand, we don't want to cause bug reports that other people
> will have to field.
>
> My suggestion: just enable the full set of extensions in the i965
> driver. If someone that maintains a Gallium-based driver cares, they
> can add a cap bit to selectively enable the GL_OES_texture_*_linear
> extensions. They also have a different mechanism to independently
> enable GL_OES_texture_float and / or GL_OES_texture_half_float when the
> hardware natively supports those formats. I don't think you want to
> enable GL_OES_texture_half_float on hardware that only does 32-bit float
> textures, for example.
>
>> +++ b/src/mesa/main/extensions.c
>> @@ -314,6 +314,10 @@ static const struct extension extension_table[] = {
>> { "GL_OES_texture_3D", o(EXT_texture3D), ES2, 2005 },
>> { "GL_OES_texture_cube_map", o(ARB_texture_cube_map), ES1, 2007 },
>> { "GL_OES_texture_env_crossbar", o(ARB_texture_env_crossbar), ES1, 2005 },
>> + { "GL_OES_texture_float", o(OES_texture_float), ES2, 2005 },
>> + { "GL_OES_texture_float_linear", o(OES_texture_float_linear), ES2, 2005 },
>> + { "GL_OES_texture_half_float", o(OES_texture_half_float), ES2, 2005 },
>> + { "GL_OES_texture_half_float_linear", o(OES_texture_half_float_linear), ES2, 2005 },
>> { "GL_OES_texture_mirrored_repeat", o(dummy_true), ES1, 2005 },
>> { "GL_OES_texture_npot", o(ARB_texture_non_power_of_two), ES1 | ES2, 2005 },
>> { "GL_OES_vertex_array_object", o(dummy_true), ES1 | ES2, 2010 },
>> diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
>> index 00478f9..bd597c6 100644
>> --- a/src/mesa/main/glformats.c
>> +++ b/src/mesa/main/glformats.c
>> @@ -93,6 +93,7 @@ _mesa_sizeof_type(GLenum type)
>> case GL_DOUBLE:
>> return sizeof(GLdouble);
>> case GL_HALF_FLOAT_ARB:
>> + case GL_HALF_FLOAT_OES:
>> return sizeof(GLhalfARB);
>> case GL_FIXED:
>> return sizeof(GLfixed);
>> @@ -125,6 +126,7 @@ _mesa_sizeof_packed_type(GLenum type)
>> case GL_INT:
>> return sizeof(GLint);
>> case GL_HALF_FLOAT_ARB:
>> + case GL_HALF_FLOAT_OES:
>> return sizeof(GLhalfARB);
>> case GL_FLOAT:
>> return sizeof(GLfloat);
>> @@ -241,6 +243,7 @@ _mesa_bytes_per_pixel(GLenum format, GLenum type)
>> case GL_FLOAT:
>> return comps * sizeof(GLfloat);
>> case GL_HALF_FLOAT_ARB:
>> + case GL_HALF_FLOAT_OES:
>> return comps * sizeof(GLhalfARB);
>> case GL_UNSIGNED_BYTE_3_3_2:
>> case GL_UNSIGNED_BYTE_2_3_3_REV:
>> @@ -1448,6 +1451,18 @@ _mesa_error_check_format_and_type(const struct gl_context *ctx,
>> }
>> return GL_NO_ERROR;
>>
>> + case GL_HALF_FLOAT_OES:
>> + switch (format) {
>> + case GL_RGBA:
>> + case GL_RGB:
>> + case GL_LUMINANCE_ALPHA:
>> + case GL_LUMINANCE:
>> + case GL_ALPHA:
>> + return GL_NO_ERROR;
>> + default:
>> + return GL_INVALID_OPERATION;
>> + }
>> +
>> default:
>> ; /* fall-through */
>> }
>> @@ -1782,7 +1797,8 @@ _mesa_es_error_check_format_and_type(GLenum format, GLenum type,
>> * \return error code, or GL_NO_ERROR.
>> */
>> GLenum
>> -_mesa_es3_error_check_format_and_type(GLenum format, GLenum type,
>> +_mesa_es3_error_check_format_and_type(const struct gl_context *ctx,
>> + GLenum format, GLenum type,
>> GLenum internalFormat)
>> {
>> switch (format) {
>> @@ -1847,11 +1863,17 @@ _mesa_es3_error_check_format_and_type(GLenum format, GLenum type,
>> case GL_RGBA16F:
>> case GL_RGBA32F:
>> break;
>> + case GL_RGBA:
>> + if (ctx->Extensions.OES_texture_float && internalFormat == format)
>> + break;
> /* FALLTHROUGH */
>
>> default:
>> return GL_INVALID_OPERATION;
>> }
>> break;
>>
>> + case GL_HALF_FLOAT_OES:
>> + if (ctx->Extensions.OES_texture_half_float && internalFormat == format)
>> + break;
>> default:
>> return GL_INVALID_OPERATION;
>> }
>> @@ -1956,11 +1978,19 @@ _mesa_es3_error_check_format_and_type(GLenum format, GLenum type,
>> case GL_R11F_G11F_B10F:
>> case GL_RGB9_E5:
>> break;
>> + case GL_RGB:
>> + if (ctx->Extensions.OES_texture_float && internalFormat == format)
>> + break;
> /* FALLTHROUGH */
>
>> default:
>> return GL_INVALID_OPERATION;
>> }
>> break;
>>
>> + case GL_HALF_FLOAT_OES:
>> + if (!ctx->Extensions.OES_texture_half_float || internalFormat != format)
>> + return GL_INVALID_OPERATION;
>> + break;
>> +
>> case GL_UNSIGNED_INT_2_10_10_10_REV:
>> switch (internalFormat) {
>> case GL_RGB: /* GL_EXT_texture_type_2_10_10_10_REV */
>> @@ -2200,9 +2230,17 @@ _mesa_es3_error_check_format_and_type(GLenum format, GLenum type,
>> case GL_ALPHA:
>> case GL_LUMINANCE:
>> case GL_LUMINANCE_ALPHA:
>> - if (type != GL_UNSIGNED_BYTE || format != internalFormat)
>> - return GL_INVALID_OPERATION;
>> - break;
>> + switch (type) {
>> + case GL_FLOAT:
>> + if (ctx->Extensions.OES_texture_float && internalFormat == format)
>> + break;
> return GL_INVALID_OPERATION;
>
> And I'd suggest adding a test case that would have hit this bug: create
> a GL_FLOAT texture on an implementation that supports
> OES_texture_half_float but not OES_texture_float.
>
>> + case GL_HALF_FLOAT_OES:
>> + if (ctx->Extensions.OES_texture_half_float && internalFormat == format)
>> + break;
> return GL_INVALID_OPERATION;
>
>> + default:
>> + if (type != GL_UNSIGNED_BYTE || format != internalFormat)
>> + return GL_INVALID_OPERATION;
>> + }
>> }
>>
>> return GL_NO_ERROR;
>> diff --git a/src/mesa/main/glformats.h b/src/mesa/main/glformats.h
>> index 7b03215..9b1674e 100644
>> --- a/src/mesa/main/glformats.h
>> +++ b/src/mesa/main/glformats.h
>> @@ -122,7 +122,8 @@ _mesa_es_error_check_format_and_type(GLenum format, GLenum type,
>> unsigned dimensions);
>>
>> extern GLenum
>> -_mesa_es3_error_check_format_and_type(GLenum format, GLenum type,
>> +_mesa_es3_error_check_format_and_type(const struct gl_context *ctx,
>> + GLenum format, GLenum type,
>> GLenum internalFormat);
>>
>>
>> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
>> index 7389baa..4e44b6e 100644
>> --- a/src/mesa/main/mtypes.h
>> +++ b/src/mesa/main/mtypes.h
>> @@ -1220,6 +1220,8 @@ struct gl_texture_object
>> GLboolean Purgeable; /**< Is the buffer purgeable under memory
>> pressure? */
>> GLboolean Immutable; /**< GL_ARB_texture_storage */
>> + GLboolean IsFloatOES; /**< GL_OES_float_texture */
>> + GLboolean IsHalfFloatOES; /**< GL_OES_half_float_texture */
>
> Since these are derived values, they should start with an _. I'd also
> suggest dropping the OES suffix. _IsFloat and _IsHalfFloat.
>
>>
>> GLuint MinLevel; /**< GL_ARB_texture_view */
>> GLuint MinLayer; /**< GL_ARB_texture_view */
>> @@ -3842,6 +3844,10 @@ struct gl_extensions
>> GLboolean OES_draw_texture;
>> GLboolean OES_depth_texture_cube_map;
>> GLboolean OES_EGL_image_external;
>> + GLboolean OES_texture_float;
>> + GLboolean OES_texture_half_float;
>> + GLboolean OES_texture_float_linear;
>> + GLboolean OES_texture_half_float_linear;
>> GLboolean OES_compressed_ETC1_RGB8_texture;
>> GLboolean extension_sentinel;
>> /** The extension string */
>> diff --git a/src/mesa/main/pack.c b/src/mesa/main/pack.c
>> index 649a74c..daafb1e 100644
>> --- a/src/mesa/main/pack.c
>> +++ b/src/mesa/main/pack.c
>> @@ -2301,6 +2301,7 @@ _mesa_pack_rgba_span_float(struct gl_context *ctx, GLuint n, GLfloat rgba[][4],
>> }
>> break;
>> case GL_HALF_FLOAT_ARB:
>> + case GL_HALF_FLOAT_OES:
>> {
>> GLhalfARB *dst = (GLhalfARB *) dstAddr;
>> switch (dstFormat) {
>> @@ -2724,6 +2725,7 @@ extract_uint_indexes(GLuint n, GLuint indexes[],
>> srcType == GL_INT ||
>> srcType == GL_UNSIGNED_INT_24_8_EXT ||
>> srcType == GL_HALF_FLOAT_ARB ||
>> + srcType == GL_HALF_FLOAT_OES ||
>> srcType == GL_FLOAT ||
>> srcType == GL_FLOAT_32_UNSIGNED_INT_24_8_REV);
>>
>> @@ -2863,6 +2865,7 @@ extract_uint_indexes(GLuint n, GLuint indexes[],
>> }
>> break;
>> case GL_HALF_FLOAT_ARB:
>> + case GL_HALF_FLOAT_OES:
>> {
>> GLuint i;
>> const GLhalfARB *s = (const GLhalfARB *) src;
>> @@ -3102,6 +3105,7 @@ extract_float_rgba(GLuint n, GLfloat rgba[][4],
>> srcType == GL_UNSIGNED_INT ||
>> srcType == GL_INT ||
>> srcType == GL_HALF_FLOAT_ARB ||
>> + srcType == GL_HALF_FLOAT_OES ||
>> srcType == GL_FLOAT ||
>> srcType == GL_UNSIGNED_BYTE_3_3_2 ||
>> srcType == GL_UNSIGNED_BYTE_2_3_3_REV ||
>> @@ -3219,6 +3223,7 @@ extract_float_rgba(GLuint n, GLfloat rgba[][4],
>> PROCESS(aSrc, ACOMP, 1.0F, 1.0F, GLfloat, (GLfloat));
>> break;
>> case GL_HALF_FLOAT_ARB:
>> + case GL_HALF_FLOAT_OES:
>> PROCESS(rSrc, RCOMP, 0.0F, 0.0F, GLhalfARB, _mesa_half_to_float);
>> PROCESS(gSrc, GCOMP, 0.0F, 0.0F, GLhalfARB, _mesa_half_to_float);
>> PROCESS(bSrc, BCOMP, 0.0F, 0.0F, GLhalfARB, _mesa_half_to_float);
>> @@ -3717,6 +3722,7 @@ extract_uint_rgba(GLuint n, GLuint rgba[][4],
>> srcType == GL_UNSIGNED_INT ||
>> srcType == GL_INT ||
>> srcType == GL_HALF_FLOAT_ARB ||
>> + srcType == GL_HALF_FLOAT_OES ||
>> srcType == GL_FLOAT ||
>> srcType == GL_UNSIGNED_BYTE_3_3_2 ||
>> srcType == GL_UNSIGNED_BYTE_2_3_3_REV ||
>> @@ -3814,6 +3820,7 @@ extract_uint_rgba(GLuint n, GLuint rgba[][4],
>> PROCESS(aSrc, ACOMP, 1, GLfloat, clamp_float_to_uint);
>> break;
>> case GL_HALF_FLOAT_ARB:
>> + case GL_HALF_FLOAT_OES:
>> PROCESS(rSrc, RCOMP, 0, GLhalfARB, clamp_half_to_uint);
>> PROCESS(gSrc, GCOMP, 0, GLhalfARB, clamp_half_to_uint);
>> PROCESS(bSrc, BCOMP, 0, GLhalfARB, clamp_half_to_uint);
>> @@ -4218,6 +4225,7 @@ _mesa_unpack_color_span_ubyte(struct gl_context *ctx,
>> srcType == GL_UNSIGNED_INT ||
>> srcType == GL_INT ||
>> srcType == GL_HALF_FLOAT_ARB ||
>> + srcType == GL_HALF_FLOAT_OES ||
>> srcType == GL_FLOAT ||
>> srcType == GL_UNSIGNED_BYTE_3_3_2 ||
>> srcType == GL_UNSIGNED_BYTE_2_3_3_REV ||
>> @@ -4471,6 +4479,7 @@ _mesa_unpack_color_span_float( struct gl_context *ctx,
>> srcType == GL_UNSIGNED_INT ||
>> srcType == GL_INT ||
>> srcType == GL_HALF_FLOAT_ARB ||
>> + srcType == GL_HALF_FLOAT_OES ||
>> srcType == GL_FLOAT ||
>> srcType == GL_UNSIGNED_BYTE_3_3_2 ||
>> srcType == GL_UNSIGNED_BYTE_2_3_3_REV ||
>> @@ -4675,6 +4684,7 @@ _mesa_unpack_color_span_uint(struct gl_context *ctx,
>> srcType == GL_UNSIGNED_INT ||
>> srcType == GL_INT ||
>> srcType == GL_HALF_FLOAT_ARB ||
>> + srcType == GL_HALF_FLOAT_OES ||
>> srcType == GL_FLOAT ||
>> srcType == GL_UNSIGNED_BYTE_3_3_2 ||
>> srcType == GL_UNSIGNED_BYTE_2_3_3_REV ||
>> @@ -4803,6 +4813,7 @@ _mesa_unpack_index_span( struct gl_context *ctx, GLuint n,
>> srcType == GL_UNSIGNED_INT ||
>> srcType == GL_INT ||
>> srcType == GL_HALF_FLOAT_ARB ||
>> + srcType == GL_HALF_FLOAT_OES ||
>> srcType == GL_FLOAT);
>>
>> ASSERT(dstType == GL_UNSIGNED_BYTE ||
>> @@ -4974,6 +4985,7 @@ _mesa_pack_index_span( struct gl_context *ctx, GLuint n,
>> }
>> break;
>> case GL_HALF_FLOAT_ARB:
>> + case GL_HALF_FLOAT_OES:
>> {
>> GLhalfARB *dst = (GLhalfARB *) dest;
>> GLuint i;
>> @@ -5023,6 +5035,7 @@ _mesa_unpack_stencil_span( struct gl_context *ctx, GLuint n,
>> srcType == GL_INT ||
>> srcType == GL_UNSIGNED_INT_24_8_EXT ||
>> srcType == GL_HALF_FLOAT_ARB ||
>> + srcType == GL_HALF_FLOAT_OES ||
>> srcType == GL_FLOAT ||
>> srcType == GL_FLOAT_32_UNSIGNED_INT_24_8_REV);
>>
>> @@ -5213,6 +5226,7 @@ _mesa_pack_stencil_span( struct gl_context *ctx, GLuint n,
>> }
>> break;
>> case GL_HALF_FLOAT_ARB:
>> + case GL_HALF_FLOAT_OES:
>> {
>> GLhalfARB *dst = (GLhalfARB *) dest;
>> GLuint i;
>> @@ -5430,6 +5444,7 @@ _mesa_unpack_depth_span( struct gl_context *ctx, GLuint n,
>> needClamp = GL_TRUE;
>> break;
>> case GL_HALF_FLOAT_ARB:
>> + case GL_HALF_FLOAT_OES:
>> {
>> GLuint i;
>> const GLhalfARB *src = (const GLhalfARB *) source;
>> @@ -5619,6 +5634,7 @@ _mesa_pack_depth_span( struct gl_context *ctx, GLuint n, GLvoid *dest,
>> }
>> break;
>> case GL_HALF_FLOAT_ARB:
>> + case GL_HALF_FLOAT_OES:
>> {
>> GLhalfARB *dst = (GLhalfARB *) dest;
>> GLuint i;
>> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
>> index 4f4bb11..c121ef8 100644
>> --- a/src/mesa/main/teximage.c
>> +++ b/src/mesa/main/teximage.c
>> @@ -62,7 +62,58 @@
>> */
>> #define NEW_COPY_TEX_STATE (_NEW_BUFFERS | _NEW_PIXEL)
>>
>> +/**
>> + * Returns a corresponding internal floating point format for a given base
>> + * format as specifed by OES_texture_float. In case of Float, the internal
>> + * format needs to be a 32 bit component and in case of HALF_FLOAT it needs to
>> + * be a 16 bit component.
>> + *
>> + * For example, given base format GL_ALPHA, type Float return GL_ALPHA32F_ARB.
>> + */
>> +static GLenum
>> +adjust_for_oes_float_texture(GLenum format, GLenum type)
>> +{
>> + switch (type) {
>> + case GL_FLOAT:
>> + switch (format) {
>> + case GL_RGBA:
>> + return GL_RGBA32F_ARB;
>
> Use the non-_ARB suffixed names.
>
>> + case GL_RGB:
>> + return GL_RGB32F_ARB;
>> + case GL_ALPHA:
>> + return GL_ALPHA32F_ARB;
>> + case GL_LUMINANCE:
>> + return GL_LUMINANCE32F_ARB;
>> + case GL_LUMINANCE_ALPHA:
>> + return GL_LUMINANCE_ALPHA32F_ARB;
>> + default:
>> + break;
>> + }
>> + break;
>>
>> + case GL_HALF_FLOAT_OES:
>> + switch (format) {
>> + case GL_RGBA:
>> + return GL_RGBA16F_ARB;
>> + case GL_RGB:
>> + return GL_RGB16F_ARB;
>> + case GL_ALPHA:
>> + return GL_ALPHA16F_ARB;
>> + case GL_LUMINANCE:
>> + return GL_LUMINANCE16F_ARB;
>> + case GL_LUMINANCE_ALPHA:
>> + return GL_LUMINANCE_ALPHA16F_ARB;
>> + default:
>> + break;
>> + }
>> + break;
>> +
>> + default:
>> + break;
>> + }
>> +
>> + return format;
>> +}
>>
>> /**
>> * Return the simple base format for a given internal texture format.
>> @@ -2137,7 +2188,7 @@ texture_error_check( struct gl_context *ctx,
>>
>> if (_mesa_is_gles(ctx)) {
>> if (_mesa_is_gles3(ctx)) {
>> - err = _mesa_es3_error_check_format_and_type(format, type,
>> + err = _mesa_es3_error_check_format_and_type(ctx, format, type,
>> internalFormat);
>> } else {
>> if (format != internalFormat) {
>> @@ -3239,6 +3290,19 @@ teximage(struct gl_context *ctx, GLboolean compressed, GLuint dims,
>> texFormat = _mesa_glenum_to_compressed_format(internalFormat);
>> }
>> else {
>> + /* In case of HALF_FLOAT_OES or FLOAT_OES, find corresponding sized
>> + * internal floating point format for the given base format.
>> + */
>> + if (_mesa_is_gles(ctx) && format == internalFormat) {
>> + if (type == GL_HALF_FLOAT_OES && ctx->Extensions.OES_texture_half_float)
>> + texObj->IsHalfFloatOES = GL_TRUE;
>> + else if (type == GL_FLOAT && ctx->Extensions.OES_texture_float)
>> + texObj->IsFloatOES = GL_TRUE;
>
> I'd simplify this to
>
> texObj->_IsFloat = type == GL_FLOAT;
> texObj->_IsHalfFloat = type == GL_HALF_FLOAT_OES || type ==
> GL_HALF_FLOAT;
>
> You need GL_HALF_FLOAT for GLES3.
>
> Also, _mesa_copy_texture_object needs to copy these fields.
>
>> +
>> + if (texObj->IsHalfFloatOES || texObj->IsFloatOES)
>> + internalFormat = adjust_for_oes_float_texture(format, type);
>
> I'd just call this unconditionally.
>
>> + }
>> +
>> texFormat = _mesa_choose_texture_format(ctx, texObj, target, level,
>> internalFormat, format, type);
>> }
>> diff --git a/src/mesa/main/texobj.c b/src/mesa/main/texobj.c
>> index 923cf60..4d0099a 100644
>> --- a/src/mesa/main/texobj.c
>> +++ b/src/mesa/main/texobj.c
>> @@ -49,6 +49,51 @@
>> /** \name Internal functions */
>> /*@{*/
>>
>> +/**
>> + * This function checks for all valid combinations of Min and Mag filters for
>> + * Float types, when extensions like OES_texture_float and
>> + * OES_texture_float_linear are supported . OES_texture_float mentions support
>> + * for NEAREST, NEAREST_MIPMAP_NEAREST magnification and minification filters.
>> + * Mag filters like LINEAR and min filters like NEAREST_MIPMAP_LINEAR,
>> + * LINEAR_MIPMAP_NEAREST and LINEAR_MIPMAP_LINEAR are only valid in case
>> + * OES_texture_float_linear is supported.
>> + *
>> + * Returns true in case the filter is valid for given Float type else false.
>> + */
>> +static bool valid_filter_for_float(const struct gl_context *ctx,
>> + const struct gl_texture_object *obj) {
>
> Please follow Mesa coding conventions.
>
> static bool
> valid_filter_for_float(...)
> {
> ...
> }
>
>> + ASSERT(_mesa_is_gles(ctx));
>> + switch (obj->Sampler.MagFilter) {
>> + case GL_LINEAR:
>> + if (obj->IsHalfFloatOES && !ctx->Extensions.OES_texture_half_float_linear)
>> + return false;
>> + else if (obj->IsFloatOES && !ctx->Extensions.OES_texture_float_linear)
>> + return false;
>> + case GL_NEAREST:
>> + case GL_NEAREST_MIPMAP_NEAREST:
>> + break;
>> + default:
>> + return false;
>
> Since the default case should be impossible (or handled elsewhere), I
> would mark replace the return with
>
> unreachable("Invalid mag filter");
>
>> + }
>> +
>> + switch (obj->Sampler.MinFilter) {
>> + case GL_NEAREST:
>> + case GL_NEAREST_MIPMAP_NEAREST:
>> + return true;
>
> Please use the same pattern in this switch-statement as in the previous:
> break on success, return false on failure. Change the final 'return
> false', which is currently unreachable, to 'return true'.
>
>> + case GL_LINEAR:
>> + case GL_NEAREST_MIPMAP_LINEAR:
>> + case GL_LINEAR_MIPMAP_NEAREST:
>> + case GL_LINEAR_MIPMAP_LINEAR:
>> + if (obj->IsHalfFloatOES && ctx->Extensions.OES_texture_half_float_linear)
>> + return true;
>> + else if (obj->IsFloatOES && ctx->Extensions.OES_texture_float_linear)
>> + return true;
>> + default:
>> + return false;
>
> unreachable("Invalid min filter");
>
>> + }
>> +
>> + return false;
>> +}
>>
>> /**
>> * Return the gl_texture_object for a given ID.
>> @@ -542,6 +587,14 @@ _mesa_test_texobj_completeness( const struct gl_context *ctx,
>> t->_IsIntegerFormat = datatype == GL_INT || datatype == GL_UNSIGNED_INT;
>> }
>>
>> + /* Check if the texture type is Float or HalfFloatOES and ensure Min and Mag
>> + * filters are supported in this case.
>> + */
>> + if ((t->IsFloatOES || t->IsHalfFloatOES) && !valid_filter_for_float(ctx, t)) {
>
> Either check here or check in valid_filter_for_float. Don't do it in
> both places. Since the function will be inlined, I don't think it
> matters where you do it... but it might be interesting to see what the
> compiler generates in both cases.
>
>> + incomplete(t, BASE, "Filter is not supported with Float types.");
>> + return;
>> + }
>> +
>> /* Compute _MaxLevel (the maximum mipmap level we'll sample from given the
>> * mipmap image sizes and GL_TEXTURE_MAX_LEVEL state).
>> */
>>
>
More information about the mesa-dev
mailing list