On 11/7/19 20:12, Marek Olšák wrote:
The test passes here. I wouldn't push a commit that doesn't pass. It looks like v3d can't blit stencil.


FWIW, just in case you are curious: I was able to take a look today to this. This problem only affects the fbo-stencil case with GL_DEPTH32F_STENCIL8. So for example, the following ones:

./bin/fbo-stencil copypixels GL_DEPTH24_STENCIL8 -auto -fbo

./bin/fbo-depthstencil copypixels GL_DEPTH32F_STENCIL8 -auto -fbo

works fine, so I guess that we need to do something special with that format on v3d.



Marek

On Thu, Jul 11, 2019 at 6:29 AM apinheiro <apinhe...@igalia.com <mailto:apinhe...@igalia.com>> wrote:

    Hi, the following piglit test:

    ./bin/fbo-stencil copypixels GL_DEPTH32F_STENCIL8 -auto -fbo

    regressed after this patch landed master with the v3d driver. So
    Marek
    and anyone reading this email, could you execute that test and
    confirms
    if only regress with v3d?

    Thanks in advance.

    On 25/6/19 2:12, Marek Olšák wrote:
    > From: Marek Olšák <marek.ol...@amd.com <mailto:marek.ol...@amd.com>>
    >
    > ---
    >   src/mesa/state_tracker/st_cb_drawpixels.c | 58
    +++++++++++++++--------
    >   1 file changed, 38 insertions(+), 20 deletions(-)
    >
    > diff --git a/src/mesa/state_tracker/st_cb_drawpixels.c
    b/src/mesa/state_tracker/st_cb_drawpixels.c
    > index 59868d3ff1d..26d3cc33e5c 100644
    > --- a/src/mesa/state_tracker/st_cb_drawpixels.c
    > +++ b/src/mesa/state_tracker/st_cb_drawpixels.c
    > @@ -1508,35 +1508,35 @@ static GLboolean
    >   blit_copy_pixels(struct gl_context *ctx, GLint srcx, GLint srcy,
    >                    GLsizei width, GLsizei height,
    >                    GLint dstx, GLint dsty, GLenum type)
    >   {
    >      struct st_context *st = st_context(ctx);
    >      struct pipe_context *pipe = st->pipe;
    >      struct pipe_screen *screen = pipe->screen;
    >      struct gl_pixelstore_attrib pack, unpack;
    >      GLint readX, readY, readW, readH, drawX, drawY, drawW, drawH;
    >
    > -   if (type == GL_COLOR &&
    > -       ctx->Pixel.ZoomX == 1.0 &&
    > +   if (ctx->Pixel.ZoomX == 1.0 &&
    >          ctx->Pixel.ZoomY == 1.0 &&
    > -       ctx->_ImageTransferState == 0x0 &&
    > -       !ctx->Color.BlendEnabled &&
    > -       !ctx->Color.AlphaEnabled &&
    > -       (!ctx->Color.ColorLogicOpEnabled || ctx->Color.LogicOp
    == GL_COPY) &&
    > -       !ctx->Depth.Test &&
    > -       !ctx->Fog.Enabled &&
    > -       !ctx->Stencil.Enabled &&
    > -       !ctx->FragmentProgram.Enabled &&
    > -       !ctx->VertexProgram.Enabled &&
    > -  !ctx->_Shader->CurrentProgram[MESA_SHADER_FRAGMENT] &&
    > -       !_mesa_ati_fragment_shader_enabled(ctx) &&
    > -       ctx->DrawBuffer->_NumColorDrawBuffers == 1 &&
    > +       (type != GL_COLOR ||
    > +        (ctx->_ImageTransferState == 0x0 &&
    > +         !ctx->Color.BlendEnabled &&
    > +         !ctx->Color.AlphaEnabled &&
    > +         (!ctx->Color.ColorLogicOpEnabled || ctx->Color.LogicOp
    == GL_COPY) &&
    > +         !ctx->Depth.Test &&
    > +         !ctx->Fog.Enabled &&
    > +         !ctx->Stencil.Enabled &&
    > +         !ctx->FragmentProgram.Enabled &&
    > +         !ctx->VertexProgram.Enabled &&
    > +  !ctx->_Shader->CurrentProgram[MESA_SHADER_FRAGMENT] &&
    > +         !_mesa_ati_fragment_shader_enabled(ctx) &&
    > +         ctx->DrawBuffer->_NumColorDrawBuffers == 1)) &&
    >          !ctx->Query.CondRenderQuery &&
    >          !ctx->Query.CurrentOcclusionObject) {
    >         struct st_renderbuffer *rbRead, *rbDraw;
    >
    >         /*
    >          * Clip the read region against the src buffer bounds.
    >          * We'll still allocate a temporary buffer/texture for
    the original
    >          * src region size but we'll only read the region which
    is on-screen.
    >          * This may mean that we draw garbage pixels into the
    dest region, but
    >          * that's expected.
    > @@ -1555,22 +1555,32 @@ blit_copy_pixels(struct gl_context *ctx,
    GLint srcx, GLint srcy,
    >         unpack = pack;
    >         if (!_mesa_clip_drawpixels(ctx, &drawX, &drawY, &readW,
    &readH, &unpack))
    >            return GL_TRUE; /* all done */
    >
    >         readX = readX - pack.SkipPixels + unpack.SkipPixels;
    >         readY = readY - pack.SkipRows + unpack.SkipRows;
    >
    >         drawW = readW;
    >         drawH = readH;
    >
    > -      rbRead = st_get_color_read_renderbuffer(ctx);
    > -      rbDraw =
    st_renderbuffer(ctx->DrawBuffer->_ColorDrawBuffers[0]);
    > +      if (type == GL_COLOR) {
    > +         rbRead = st_get_color_read_renderbuffer(ctx);
    > +         rbDraw =
    st_renderbuffer(ctx->DrawBuffer->_ColorDrawBuffers[0]);
    > +      } else if (type == GL_DEPTH || type == GL_DEPTH_STENCIL) {
    > +         rbRead =
    st_renderbuffer(ctx->ReadBuffer->Attachment[BUFFER_DEPTH].Renderbuffer);
    > +         rbDraw =
    st_renderbuffer(ctx->DrawBuffer->Attachment[BUFFER_DEPTH].Renderbuffer);
    > +      } else if (type == GL_STENCIL) {
    > +         rbRead =
    st_renderbuffer(ctx->ReadBuffer->Attachment[BUFFER_STENCIL].Renderbuffer);
    > +         rbDraw =
    st_renderbuffer(ctx->DrawBuffer->Attachment[BUFFER_STENCIL].Renderbuffer);
    > +      } else {
    > +         return false;
    > +      }
    >
    >         /* Flip src/dst position depending on the orientation of
    buffers. */
    >         if (st_fb_orientation(ctx->ReadBuffer) == Y_0_TOP) {
    >            readY = rbRead->Base.Height - readY;
    >            readH = -readH;
    >         }
    >
    >         if (st_fb_orientation(ctx->DrawBuffer) == Y_0_TOP) {
    >            /* We can't flip the destination for pipe->blit, so
    we only adjust
    >             * its position and flip the source.
    > @@ -1597,23 +1607,31 @@ blit_copy_pixels(struct gl_context *ctx,
    GLint srcx, GLint srcy,
    >            blit.src.box.depth = 1;
    >            blit.dst.resource = rbDraw->texture;
    >            blit.dst.level = rbDraw->surface->u.tex.level;
    >            blit.dst.format = rbDraw->texture->format;
    >            blit.dst.box.x = drawX;
    >            blit.dst.box.y = drawY;
    >            blit.dst.box.z = rbDraw->surface->u.tex.first_layer;
    >            blit.dst.box.width = drawW;
    >            blit.dst.box.height = drawH;
    >            blit.dst.box.depth = 1;
    > -         blit.mask = PIPE_MASK_RGBA;
    >            blit.filter = PIPE_TEX_FILTER_NEAREST;
    >
    > +         if (type == GL_COLOR)
    > +            blit.mask |= PIPE_MASK_RGBA;
    > +         if (type == GL_DEPTH)
    > +            blit.mask |= PIPE_MASK_Z;
    > +         if (type == GL_STENCIL)
    > +            blit.mask |= PIPE_MASK_S;
    > +         if (type == GL_DEPTH_STENCIL)
    > +            blit.mask |= PIPE_MASK_ZS;
    > +
    >            if (ctx->DrawBuffer != ctx->WinSysDrawBuffer)
    >               st_window_rectangles_to_blit(ctx, &blit);
    >
    >            if (screen->is_format_supported(screen, blit.src.format,
    > blit.src.resource->target,
    > blit.src.resource->nr_samples,
    > blit.src.resource->nr_storage_samples,
    > PIPE_BIND_SAMPLER_VIEW) &&
    >                screen->is_format_supported(screen, blit.dst.format,
    > blit.dst.resource->target,
    > @@ -1650,36 +1668,36 @@ st_CopyPixels(struct gl_context *ctx,
    GLint srcx, GLint srcy,
    >      GLint readX, readY, readW, readH;
    >      struct gl_pixelstore_attrib pack = ctx->DefaultPacking;
    >
    >      _mesa_update_draw_buffer_bounds(ctx, ctx->DrawBuffer);
    >
    >      st_flush_bitmap_cache(st);
    >      st_invalidate_readpix_cache(st);
    >
    >      st_validate_state(st, ST_PIPELINE_META);
    >
    > +   if (blit_copy_pixels(ctx, srcx, srcy, width, height, dstx,
    dsty, type))
    > +      return;
    > +
    >      if (type == GL_DEPTH_STENCIL) {
    >         /* XXX make this more efficient */
    >         st_CopyPixels(ctx, srcx, srcy, width, height, dstx,
    dsty, GL_STENCIL);
    >         st_CopyPixels(ctx, srcx, srcy, width, height, dstx,
    dsty, GL_DEPTH);
    >         return;
    >      }
    >
    >      if (type == GL_STENCIL) {
    >         /* can't use texturing to do stencil */
    >         copy_stencil_pixels(ctx, srcx, srcy, width, height,
    dstx, dsty);
    >         return;
    >      }
    >
    > -   if (blit_copy_pixels(ctx, srcx, srcy, width, height, dstx,
    dsty, type))
    > -      return;
    > -
    >      /*
    >       * The subsequent code implements glCopyPixels by copying
    the source
    >       * pixels into a temporary texture that's then applied to a
    textured quad.
    >       * When we draw the textured quad, all the usual
    per-fragment operations
    >       * are handled.
    >       */
    >
    >      st_make_passthrough_vertex_shader(st);
    >
    >      /*

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to