[Mesa-dev] [Patch v3] Mesa: Add support for GL_OES_texture_*float* extensions.
Ian Romanick
idr at freedesktop.org
Tue Dec 2 12:36:46 PST 2014
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