On 02/02/2017 07:38 PM, Ilia Mirkin wrote:
> On Thu, Feb 2, 2017 at 1:36 PM, Ilia Mirkin <imir...@alum.mit.edu> wrote:
>> On Thu, Feb 2, 2017 at 1:29 PM, Eduardo Lima Mitev <el...@igalia.com> wrote:
>>> OpenGL 4.5 spec, section "8.11.4 Texture Image Queries", page 233 of
>>> the PDF states:
>>>
>>> "An INVALID_OPERATION error is generated if texture is the name of a
>>> buffer
>>> or multisample texture."
>>>
>>> Currently, this is not being checked and the multisample texture image is
>>> passed
>>> down to the driver hook. On i965, it is crashing the driver with an
>>> assertion:
>>>
>>> intel_mipmap_tree.c:3125: intel_miptree_map: Assertion `mt->num_samples <=
>>> 1' failed.
>>> ---
>>> src/mesa/main/texgetimage.c | 14 ++++++++++++++
>>> 1 file changed, 14 insertions(+)
>>>
>>> diff --git a/src/mesa/main/texgetimage.c b/src/mesa/main/texgetimage.c
>>> index d5cb1636605..6a23e4bbb8c 100644
>>> --- a/src/mesa/main/texgetimage.c
>>> +++ b/src/mesa/main/texgetimage.c
>>> @@ -1185,6 +1185,20 @@ getteximage_error_check(struct gl_context *ctx,
>>> texImage = select_tex_image(texObj, target, level, zoffset);
>>> assert(texImage);
>>>
>>> + /* Check that texObj is not a buffer or multisample texture if called
>>> + * from glGetTextureSubImage. OpenGL 4.5 spec, section "8.11.4 Texture
>>> + * Image Queries", page 233 of the PDF states:
>>> + *
>>> + * "An INVALID_OPERATION error is generated if texture is the
>>> + * name of a buffer or multisample texture."
>>> + */
>>> + if (texImage->NumSamples > 0 &&
>>> + strcmp(caller, "glGetTextureSubImage") == 0) {
>>
>> So... glGetTextureSubImage is not OK but glGetTextureImage is OK?
>>
>> I think the issue is a missing
>>
>> if (!legal_getteximage_target(ctx, texObj->Target, true)) {
>> _mesa_error(ctx, GL_INVALID_ENUM, "%s", caller);
>> return;
>> }
>>
>> block in _mesa_GetTextureSubImage.
>
> Or actually I guess this should get a custom block in that function
> (since it's actually different from GetTextureImage). Either way, the
> strcmp is not the way to go.
>
Note that this is not about the target, but the texture image object
itself, which shouldn't had been initialized with glTexImage2DMultisample.
I agree that using strcmp against the caller is not very elegant, but
passing another argument to getteximage_error_check() to discriminate
caller is even worse, IMO, since we already have a "caller" arg.
So, will you be ok with moving the check to _mesa_GetTextureSubImage,
duplicating the code necessary to get the gl_texture_image object?
IMHO, both texgetimage.c and teximage.c can use a good refactoring.
Thanks for the feedback,
Eduardo
>>
>>> + _mesa_error(ctx, GL_INVALID_OPERATION,
>>> + "%s(multisample texture)", caller);
>>> + return true;
>>> + }
>>> +
>>> /*
>>> * Format and type checking has been moved up to GetnTexImage and
>>> * GetTextureImage so that it happens before getting the texImage
>>> object.
>>> --
>>> 2.11.0
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
