[Mesa-dev] [PATCH 01/11] gallium: Add __DRIimageDriverExtension support to gallium

[Michel Dänzer]

On 28.05.2014 21:13, Axel Davy wrote:
> On 28/05/2014 03:47, Michel Dänzer wrote :
>> On 28.05.2014 09:55, Axel Davy wrote:
>>> From: Keith Packard <keithp at keithp.com>
>>>
>>> Provide the hook to pull textures out of __DRIimage structures and
>>> use them as
>>> renderbuffers.
>>>
>>> Signed-off-by: Keith Packard <keithp at keithp.com>
>> This patch breaks a number of piglit tests with DRI2, see the backtrace
>> below. This fixes it:
>>
>> diff --git a/src/gallium/state_trackers/dri/drm/dri2.c
>> b/src/gallium/state_trackers/dri/drm/dri2.c
>> index cd9964c..2d93686 100644
>> --- a/src/gallium/state_trackers/dri/drm/dri2.c
>> +++ b/src/gallium/state_trackers/dri/drm/dri2.c
>> @@ -761,7 +761,7 @@ dri2_flush_frontbuffer(struct dri_context *ctx,
>>
>>      pipe->flush(pipe, NULL, 0);
>>
>> -   if (image->flushFrontBuffer) {
>> +   if (image) {
>>         image->flushFrontBuffer(dri_drawable,
>> dri_drawable->loaderPrivate);
>>      } else if (loader->flushFrontBuffer) {
>>         loader->flushFrontBuffer(dri_drawable,
>> dri_drawable->loaderPrivate);

[...]

> I agree with the fix.
> 
> However the test may be better to be 'image && image->flushFrontBuffer',
> even if for the moment the field is non-NULL for all the drivers
> defining __DRIimageLoaderExtension.

The intel drivers indeed check for it being non-NULL, but it looks like
that may just be an artifact of how it uses the same code for this and
the DRI2 loader extension.

FWIW, I think it would be better to make this hook non-optional in the
image extension: It's easy to provide a no-op hook if there are indeed
cases where nothing needs to be done. But if the hook is optional, it's
easy to accidentally fail to provide it when it's necesary.


-- 
Earthling Michel Dänzer            |                  http://www.amd.com
Libre software enthusiast          |                Mesa and X developer