On Thu, 2009-09-10 at 13:07 -0700, Thomas Hellstrom wrote: > Brian Paul wrote: > > Brian Paul wrote: > > > >> Thomas Hellstrom wrote: > >> > >>> Brian Paul wrote: > >>> > >>>> Thomas & Jose, I think you are most familiar with this code... > >>>> > >>>> The attached patch fixes a bug I found with softpipe and > >>>> glDrawPixels(GL_STENCIL_INDEX). > >>>> > >>>> The failing case was basically: > >>>> > >>>> glClear(GL_STENCIL_BUFFER_BIT); > >>>> ... > >>>> glDrawPixels(GL_STENCIL_INDEX). > >>>> > >>>> Drawing stencil images is done by mapping the stencil buffer, writing > >>>> the stencil values then unmapping the stencil buffer. > >>>> > >>>> With softpipe, a glClear() call just sets tile 'clear' flags; it > >>>> doesn't actually write to the framebuffer. > >>>> > >>>> So when we mapped the stencil buffer to do the glDrawPixels we weren't > >>>> getting the glClear()'d values. Later, the tile cache was getting > >>>> flushed and that would overwrite the glDrawPixels() values. > >>>> > >>>> The attached patch flushes the driver's render cache when we get a > >>>> pipe_transfer object. Also, I think the > >>>> PIPE_TEXTURE_USAGE_RENDER_TARGET flag needs to be set on the z/stencil > >>>> buffer (since we do render to it). > >>>> > >>>> -Brian > >>>> > >>>> > >>>> > >>> Hi, Brian. > >>> > >>> The conditional flush mechanism is such that when a driver has > >>> "buffered" an operation without flushing it to the "hardware", it should > >>> indicate that to the state-tracker. Typically in the HW driver case that > >>> means that commands have been queued in a batch buffer that is not yet > >>> flushed. However, if there is a pending clear in the softpipe driver, it > >>> should perhaps indicate that to the state-tracker, and the conditional > >>> flush code in st_teximage_flush_before_map() would take care of the > >>> flush. I think that would be the solution which is most consistent with > >>> how the HW drivers work. > >>> > >> OK, how about the new attached patch? I think softpipe needs to set > >> its dirty_render_cache flag in clear() and then we need to call > >> st_teximage_flush_before_map() in the st draw_stencil_pixels() code. > >> > >> > >> > >>> I've had the discussion about PIPE_TEXTURE_USAGE_RENDER_TARGET with > >>> Keith and José before (for the dri state tracker) and IIRC the meaning > >>> of that define is strictly color render target. It really comes down to > >>> whether we define RENDER_TARGET as anything being written to or any > >>> color buffer being written to. Currently I think it's the latter. > >>> > >> OK, I think we've got some loose ends related to the > >> PIPE_TEXTURE_USAGE_* flags. For example, we never set > >> PIPE_TEXTURE_USAGE_PRIMARY in the state tracker (I think we should for > >> the front color buffer). And, we always set > >> PIPE_TEXTURE_USAGE_DISPLAY_TARGET even for user-created/off-screen > >> renderbuffers. > >> > > > > Forgot attachment again! :( > > > > -Brian > > > > > Brian, See inline comments below. > Also, for the user mode state tracker > > diff --git a/src/mesa/state_tracker/st_cb_drawpixels.c > b/src/mesa/state_tracker/st_cb_drawpixels.c > index a9cafbf..e6d7739 100644 > --- a/src/mesa/state_tracker/st_cb_drawpixels.c > +++ b/src/mesa/state_tracker/st_cb_drawpixels.c > @@ -675,6 +675,8 @@ draw_stencil_pixels(GLcontext *ctx, GLint x, GLint y, > else > usage = PIPE_TRANSFER_WRITE; > > + st_teximage_flush_before_map(st, strb->texture, 0, 0, usage); > + > pt = st_cond_flush_get_tex_transfer(st_context(ctx), strb->texture, 0, 0, > 0, > usage, x, y, > width, height); > > > I think the above shouldn't be necessary, as st_cond_flush_get_tex_transfer() > does call > st_teximage_flush_before_map(). > > > > diff --git a/src/gallium/drivers/softpipe/sp_clear.c > b/src/gallium/drivers/softpipe/sp_clear.c > index fa59277..bc8f919 100644 > --- a/src/gallium/drivers/softpipe/sp_clear.c > +++ b/src/gallium/drivers/softpipe/sp_clear.c > @@ -86,4 +86,6 @@ softpipe_clear(struct pipe_context *pipe, unsigned buffers, > const float *rgba, > pipe->surface_fill(pipe, ps, 0, 0, ps->width, ps->height, cv); > #endif > } > + > + softpipe->dirty_render_cache = TRUE; > } > > > Yes, looking at the softpipe is_texture_referenced() code, this should be > sufficient. > > /Thomas
Yes, the sp_clear.c should be sufficient. If softpipe's clears are deferred, then softpipe-> is_texture_referenced should return yes for a texture/level that has pending clears. Jose ------------------------------------------------------------------------------ Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day trial. Simplify your report design, integration and deployment - and focus on what you do best, core application coding. Discover what's new with Crystal Reports now. http://p.sf.net/sfu/bobj-july _______________________________________________ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
