[Mesa-dev] [PATCH] virgl: Support ARB_framebuffer_no_attachments

[Drew Davenport]

On Wed, Jun 27, 2018 at 1:57 AM Emil Velikov <emil.l.velikov at gmail.com> wrote:
>
> On 27 June 2018 at 09:55, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> > Hi Drew,
> >
> > Just some food for thought. The patch in itself looks correct albeit partial.
> >
> > On 27 June 2018 at 00:00, Drew Davenport <ddavenport at chromium.org> wrote:
> >> This change lets the following test pass on virgl:
> >> dEQP-GLES31.functional.state_query.framebuffer_default.framebuffer_default_samples_get_framebuffer_parameteriv
> >> ---
> >>  src/gallium/drivers/virgl/virgl_screen.c | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >> diff --git a/src/gallium/drivers/virgl/virgl_screen.c b/src/gallium/drivers/virgl/virgl_screen.c
> >> index 1eefbd6519f..3035d4b5e20 100644
> >> --- a/src/gallium/drivers/virgl/virgl_screen.c
> >> +++ b/src/gallium/drivers/virgl/virgl_screen.c
> >> @@ -495,6 +495,10 @@ virgl_is_format_supported( struct pipe_screen *screen,
> >>     }
> >>
> >>     if (bind & PIPE_BIND_RENDER_TARGET) {
> >> +      /* For ARB_framebuffer_no_attachments. */
> >> +      if (format == PIPE_FORMAT_NONE)
> >> +         return TRUE;
> >> +
> >
> > For ARB_framebuffer_no_attachments to be advertised, one should return
> > 1 for the PIPE_CAP_FRAMEBUFFER_NO_ATTACHMENT query.
> > In current master says "not supported" (returns 0) and I couldn't spot
> > any patch that toggles it.
This patch was on top of the gl-4.3 branch from
https://gitlab.freedesktop.org/airlied/mesa.git, which does advertise
support for that capability.

> >
> > Is this a test which requires the functionality, without checking for
> > the extension presence?
> > Or perhaps the test is part of a larger series, which flips the switch?
Perhaps I jumped the gun sending this patch now, since it depends on a
bunch of other work that hasn't been merged yet. If it makes more
sense I don't mind holding onto this patch for now and trying to get
it merged later.

> >
> Silly typo, above should read
> "Or perhaps the patch is part ..."
>
> -Emil