On Fri, Sep 04, 2015 at 12:39:52PM -0600, Brian Paul wrote: > On 08/28/2015 11:03 AM, Jason Ekstrand wrote: > >On Fri, Aug 28, 2015 at 7:43 AM, Brian Paul <bri...@vmware.com> wrote: > >>On 08/28/2015 12:43 AM, Jason Ekstrand wrote: > >>> > >>>On Thu, Aug 27, 2015 at 11:42 PM, Jason Ekstrand <ja...@jlekstrand.net> > >>>wrote: > >>>> > >>>>From: Brian Paul <bri...@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.ekstr...@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?
Just in case Jason was waiting for another review: Reviewed-by: Topi Pohjolainen <topi.pohjolai...@intel.com> Just for the record though, I think there was too much happening in one patch and I would have preferred it split to ease the review. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
