[Mesa-dev] [PATCH rework] mesa: rework Driver.CopyImageSubData() and related code

[Jason Ekstrand]

On Mon, Sep 21, 2015 at 4:41 PM, Brian Paul <brianp at vmware.com> wrote:
> On 09/04/2015 12:39 PM, Brian Paul wrote:
>>
>> On 08/28/2015 11:03 AM, Jason Ekstrand wrote:
>>>
>>> On Fri, Aug 28, 2015 at 7:43 AM, Brian Paul <brianp at vmware.com> wrote:
>>>>
>>>> On 08/28/2015 12:43 AM, Jason Ekstrand wrote:
>>>>>
>>>>>
>>>>> On Thu, Aug 27, 2015 at 11:42 PM, Jason Ekstrand <jason at jlekstrand.net>
>>>>> wrote:
>>>>>>
>>>>>>
>>>>>> From: Brian Paul <brianp at vmware.com>
>>>>>>
>>>>>> Previously, core Mesa's _mesa_CopyImageSubData() created temporary
>>>>>> textures
>>>>>> to wrap renderbuffer sources/destinations.  This caused a bit of a
>>>>>> mess
>>>>>> in
>>>>>> the Mesa/gallium state tracker because we had to basically undo that
>>>>>> wrapping.
>>>>>>
>>>>>> Instead, change ctx->Driver.CopyImageSubData() to take both
>>>>>> gl_renderbuffer
>>>>>> and gl_texture_image src/dst pointers (one being null, the other
>>>>>> non-null)
>>>>>> so the driver can handle renderbuffer vs. texture as needed.
>>>>>>
>>>>>> For the i965 driver, we basically moved the code that wrapped textures
>>>>>> around renderbuffers from copyimage.c down into the driver.  So that
>>>>>> approach is still used there as before.
>>>>>>
>>>>>> The old code in copyimage.c also made some questionable calls to
>>>>>> _mesa_BindTexture(), etc. which weren't undone at the end.
>>>>>>
>>>>>> v2 (Jason Ekstrand): Rework the intel bits
>>>>>> ---
>>>>>>
>>>>>> TBH, I haven't actually reviewed the rest of the patch yet but I
>>>>>> figured
>>>>>> I'd save Brian the time of guess-and-check implementing it on
>>>>>> Intel.  I
>>>>>> completely reworked the intel bits so we now use the tex/rb
>>>>>> directly and
>>>>>> pun the renderbuffer wrapping all the way down to the meta layer
>>>>>> where it
>>>>>> belongs.
>>>>>
>>>>>
>>>>>
>>>>> I ran a few quick piglit tests on my Broadwell and it seems to work.
>>>>> I also sent it off to our CI system and I'll have those results in the
>>>>> morning.
>>>>
>>>>
>>>>
>>>> Thanks, Jason!  I appreciate you taking the time to do this.
>>>>
>>>> -Brian
>>>
>>>
>>> I looked through the patch for real this time, and everything looks
>>> pretty good.  thanks for cleaning things up and adding spec comments.
>>> The non-Intel bits are
>>>
>>> Reviewed-by: Jason Ekstrand <jason.ekstrand at intel.com>
>>>
>>> Someone else should review the intel bits since I wrote them.  Also,
>>> FWIW, I ran this patch through our CI system and there are no piglit
>>> regressions on any Intel platform.
>>
>>
>> The intel changes looked ok to me, FWIW.  Can we get this committed so I
>> can commit the rest of my series?
>
>
> Ping, Jason.

You got another review from Topi.  That's good enough for me.
--Jason