[Mesa-dev] [PATCH 6/9] mesa/es3.1: Allow Multisampled FrameBufferTextures
idr at freedesktop.org
Mon May 11 13:58:18 PDT 2015
On 05/11/2015 01:43 PM, Ilia Mirkin wrote:
> On Mon, May 11, 2015 at 4:34 PM, Ian Romanick <idr at freedesktop.org> wrote:
>> On 05/11/2015 07:57 AM, Ilia Mirkin wrote:
>>> On Mon, May 11, 2015 at 10:08 AM, Erik Faye-Lund <kusmabite at gmail.com> wrote:
>>>> On Mon, May 11, 2015 at 3:03 PM, Marta Lofstedt
>>>> <marta.lofstedt at linux.intel.com> wrote:
>>>>> From: Marta Lofstedt <marta.lofstedt at intel.com>
>>>>> GLES 3.1 must be allowed to use multisampled
>>>>> frambuffer textures.
>>>>> Signed-off-by: Marta Lofstedt <marta.lofstedt at intel.com>
>>>>> src/mesa/main/fbobject.c | 5 +++--
>>>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>>> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
>>>>> index 27cf97f..14a015e 100644
>>>>> --- a/src/mesa/main/fbobject.c
>>>>> +++ b/src/mesa/main/fbobject.c
>>>>> @@ -2756,8 +2756,9 @@ _mesa_FramebufferTexture2D(GLenum target, GLenum attachment,
>>>>> case GL_TEXTURE_2D_MULTISAMPLE:
>>>>> case GL_TEXTURE_2D_MULTISAMPLE_ARRAY:
>>>>> - error = _mesa_is_gles(ctx)
>>>>> - || !ctx->Extensions.ARB_texture_multisample;
>>>>> + error = (_mesa_is_gles(ctx)
>>>>> + || !ctx->Extensions.ARB_texture_multisample) &&
>>>>> + !_mesa_is_gles31(ctx);
>>> This seems correct. error = true when old condition, but not when
>>> es3.1 even if the old condition holds true. If the old condition is
>>> false, then the new addition doesn't matter.
>>> Personally I would have written this as
>>> error = _mesa_is_gles(ctx) && !mesa_is_gles31(ctx) ||
>>> The nice thing about this is that it will force an error even in the
>>> very hypothetical situation where a driver doesn't expose
>>> ARB_texture_multisample, but a GLES3.1 context was created (e.g. via
>>> an override flag).
>> I think this is a mis-feature. Most of the ARB extensions are supersets
>> of the ES3.1 functionality, so we could someday have a driver that does
>> one but not the other.
> OK, well, I won't fight this further -- I think I've made my point,
> and I believe you've understood it. Since you disagree, happy to let
> it go.
> I can't help but add though that in the situation where the ARB
> extension is a superset in terms of actual backend driver
> functionality, not just small frontend differences (e.g.
> ARB_gpu_shader5), we should add more enables that allow the (in this
> case) ES3.1 functionality. This enables a driver to be developed over
> time and lets people use version overrides without resulting in
> crashes or other driver weirdness.
Perhaps. Similar situations have been handled in a couple different
ways in the past. Sometimes we'll add a more complex function to
determine whether something is enabled (e.g.,
_mesa_has_geometry_shaders). Sometimes a driver will on enable a
certain extension for a certain kind of context (e.g., ARB_base_instance
in the i965 driver). And there are other cases where we have multiple
extension bits (e.g., OES_texture_float).
More information about the mesa-dev