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

[Brian Paul]

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.

-Brian