On Fri, Sep 9, 2016 at 7:03 PM, Emil Velikov <emil.l.veli...@gmail.com>
wrote:
> Hi Nayan,
>
> Just a couple of fly-by comments. As always don't read too much into them.
>
> On 9 September 2016 at 14:17, Nayan Deshmukh <nayan26deshm...@gmail.com>
> wrote:
> > use a linear buffer in case of back buffer
> >
> You might want to mention a bit more about the implementation and/or
> why doing things like X won't work/is bad idea.
>
> Yeah I will add more detail to the commit message.
> > + if (scrn->is_different_gpu) {
> > + templ.format = PIPE_FORMAT_B8G8R8X8_UNORM;
> > + templ.target = PIPE_TEXTURE_2D;
> > + templ.last_level = 0;
> > + templ.width0 = scrn->width;
> > + templ.height0 = scrn->height;
> > + templ.depth0 = 1;
> > + templ.array_size = 1;
> bind == 0 here, that doesn't seem right ?
>
> I was trying to replicate the code used in loader_dri3_helper.c. I was not
sure
which bind flags should be used, I was hoping that michel can have a look
and suggest the necessary changes. I tested it on my hardware and it was
working fine. On second thought it should have a PIPE_BIND_RENDER_TARGET
flag.
> + buffer->texture = scrn->base.pscreen->resource_
> create(scrn->base.pscreen,
> > + &templ);
> > + if (!buffer->texture)
> > + goto unmap_shm;
> > +
> With the bind fixed the above hunk is common so can be kept prior to
> the conditional, no ?
>
> With bind flag fixed this common code could be taken out of the
conditional.
>
> > @@ -485,6 +516,17 @@ vl_dri3_flush_frontbuffer(struct pipe_screen
> *screen,
> > return;
> > }
> >
> > + if (scrn->is_different_gpu) {
> > + u_box_origin_2d(scrn->width,
> > + scrn->height,
> > + &src_box);
> > + scrn->pipe->resource_copy_region(scrn->pipe,
> > + back->linear_buffer,
> > + 0, 0, 0, 0,
> > + back->texture,
> > + 0, &src_box);
> > +
> Please try to use the full width of the line.
>
> The entire file uses this kind of wraping so I was trying to follow the
style.
It would be better if entire file has the same kind of wraping.
Thanks for the review. I'll make the necessary changes in the next version
once michel has a look and approves it.
> Regards,
> Emil
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
