On Sat, Jun 1, 2013 at 8:29 AM, Marek Olšák <<a href="mailto:maraeo@gmail.com" target="_blank">maraeo@gmail.com</a>> wrote: <div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> We have to use pipe->blit, not resource_copy_region, so that the read buffer is resolved if it's multisampled. I also removed the CPU-based copying, which just did format conversion (obsoleted by the blit). Also, the layer/slice/face of the read buffer is taken into account (this was ignored). Last but not least, the format choosing is improved to take float and integer read buffers into account. --- src/mesa/state_tracker/st_cb_drawpixels.c | 162 +++++++++++++---------------- 1 file changed, 70 insertions(+), 92 deletions(-) diff --git a/src/mesa/state_tracker/st_cb_drawpixels.c b/src/mesa/state_tracker/st_cb_drawpixels.c index eb75500..1c26315 100644 --- a/src/mesa/state_tracker/st_cb_drawpixels.c +++ b/src/mesa/state_tracker/st_cb_drawpixels.c @@ -1474,10 +1474,9 @@ st_CopyPixels(struct gl_context *ctx, GLint srcx, GLint srcy, struct pipe_sampler_view *sv[2]; int num_sampler_view = 1; GLfloat *color; - enum pipe_format srcFormat, texFormat; + enum pipe_format srcFormat; GLboolean invertTex = GL_FALSE; GLint readX, readY, readW, readH; - GLuint sample_count; struct gl_pixelstore_attrib pack = ctx->DefaultPacking; struct st_fp_variant *fpv; @@ -1539,35 +1538,51 @@ st_CopyPixels(struct gl_context *ctx, GLint srcx, GLint srcy, /* update fragment program constants */ st_upload_constants(st, fpv->parameters, PIPE_SHADER_FRAGMENT); - sample_count = rbRead->texture->nr_samples; - /* I believe this would be legal, presumably would need to do a resolve - for color, and for depth/stencil spec says to just use one of the - depth/stencil samples per pixel? Need some transfer clarifications. */ - assert(sample_count < 2); - + /* Choose the format for the temporary texture. */ srcFormat = rbRead->texture->format; - if (screen->is_format_supported(screen, srcFormat, st->internal_target, - sample_count, - PIPE_BIND_SAMPLER_VIEW)) { - texFormat = srcFormat; - } - else { - /* srcFormat can't be used as a texture format */ + if (!screen->is_format_supported(screen, srcFormat, st->internal_target, 0, + PIPE_BIND_SAMPLER_VIEW | + (type == GL_COLOR ? PIPE_BIND_RENDER_TARGET + : PIPE_BIND_DEPTH_STENCIL))) { if (type == GL_DEPTH) { - texFormat = st_choose_format(st, GL_DEPTH_COMPONENT, - GL_NONE, GL_NONE, st->internal_target, - sample_count, PIPE_BIND_DEPTH_STENCIL, - FALSE); - assert(texFormat != PIPE_FORMAT_NONE); + srcFormat = st_choose_format(st, GL_DEPTH_COMPONENT, GL_NONE, + GL_NONE, st->internal_target, 0, + PIPE_BIND_SAMPLER_VIEW | + PIPE_BIND_DEPTH_STENCIL, FALSE); } else { - /* default color format */ - texFormat = st_choose_format(st, GL_RGBA, - GL_NONE, GL_NONE, st->internal_target, - sample_count, PIPE_BIND_SAMPLER_VIEW, - FALSE); - assert(texFormat != PIPE_FORMAT_NONE); + assert(type == GL_COLOR); + + if (util_format_is_float(srcFormat)) { + srcFormat = st_choose_format(st, GL_RGBA32F, GL_NONE, + GL_NONE, st->internal_target, 0, + PIPE_BIND_SAMPLER_VIEW | + PIPE_BIND_RENDER_TARGET, FALSE); + } + else if (util_format_is_pure_sint(srcFormat)) { + srcFormat = st_choose_format(st, GL_RGBA32I, GL_NONE, + GL_NONE, st->internal_target, 0, + PIPE_BIND_SAMPLER_VIEW | + PIPE_BIND_RENDER_TARGET, FALSE); + } + else if (util_format_is_pure_uint(srcFormat)) { + srcFormat = st_choose_format(st, GL_RGBA32UI, GL_NONE, + GL_NONE, st->internal_target, 0, + PIPE_BIND_SAMPLER_VIEW | + PIPE_BIND_RENDER_TARGET, FALSE); + } + else { + srcFormat = st_choose_format(st, GL_RGBA, GL_NONE, + GL_NONE, st->internal_target, 0, + PIPE_BIND_SAMPLER_VIEW | + PIPE_BIND_RENDER_TARGET, FALSE); + } + } </blockquote><div> I seem to recall that this "choose a gallium format for a given GL format/type" code appears elsewhere in the state tracker (but haven't double-checked). In any case, it would be nicer if this code was moved into a separate function. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">+ + if (srcFormat == PIPE_FORMAT_NONE) { + assert(0 && "cannot choose a format for src of CopyPixels"); + return; } } </blockquote><div> Reviewed-by: Brian Paul <<a href="mailto:brianp@vmware.com">brianp@vmware.com</a>> </div></div>