On 06/07/2012 10:17 AM, Paul Berry wrote: > On 6 June 2012 23:42, Kenneth Graunke <kenn...@whitecape.org > <mailto:kenn...@whitecape.org>> wrote: > > On 06/06/2012 12:20 PM, Paul Berry wrote: > > Previously the blorp engine only supported RGBA8 color buffers and > > 24-bit depth buffers. This patch adds support for any color buffer > > format that is supported as a render target, and for 16-bit and 32-bit > > depth buffers. > > > > This required threading the brw_context struct through into > > brw_blorp_surface_info::set() so that it can consult the > > brw->render_target_format array. > > --- > > src/mesa/drivers/dri/i965/brw_blorp.cpp | 37 > +++++++++++++++++++++++--- > > src/mesa/drivers/dri/i965/brw_blorp.h | 3 +- > > src/mesa/drivers/dri/i965/brw_blorp_blit.cpp | 4 +- > > 3 files changed, 37 insertions(+), 7 deletions(-) > > > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.cpp > b/src/mesa/drivers/dri/i965/brw_blorp.cpp > > index d6d00e7..32ea738 100644 > > --- a/src/mesa/drivers/dri/i965/brw_blorp.cpp > > +++ b/src/mesa/drivers/dri/i965/brw_blorp.cpp > > @@ -53,23 +53,52 @@ brw_blorp_mip_info::set(struct > intel_mipmap_tree *mt, > > } > > > > void > > -brw_blorp_surface_info::set(struct intel_mipmap_tree *mt, > > +brw_blorp_surface_info::set(struct brw_context *brw, > > + struct intel_mipmap_tree *mt, > > unsigned int level, unsigned int layer) > > { > > brw_blorp_mip_info::set(mt, level, layer); > > this->num_samples = mt->num_samples; > > this->array_spacing_lod0 = mt->array_spacing_lod0; > > + this->map_stencil_as_y_tiled = false; > > > > - if (mt->format == MESA_FORMAT_S8) { > > + switch (mt->format) { > > + case MESA_FORMAT_S8: > > /* The miptree is a W-tiled stencil buffer. Surface states > can't be set > > * up for W tiling, so we'll need to use Y tiling and have > the WM > > * program swizzle the coordinates. > > */ > > this->map_stencil_as_y_tiled = true; > > this->brw_surfaceformat = BRW_SURFACEFORMAT_R8_UNORM; > > - } else { > > - this->map_stencil_as_y_tiled = false; > > + break; > > + case MESA_FORMAT_X8_Z24: > > + case MESA_FORMAT_Z32_FLOAT: > > + /* The miptree consists of 32 bits per pixel, arranged > either as 24-bit > > + * depth values interleaved with 8 "don't care" bits, or as > 32-bit > > + * floating point depth values. Since depth values don't > require any > > + * blending, it doesn't matter how we interpret the bit > pattern as long > > + * as we copy the right amount of data, so just map it as > 8-bit RGBA. > > Nitpick...you're not mapping it as RGBA. I would believe either > "just map it as 8-bit BGRA" ... or "as 8-bit ARGB" since I can never > keep the endianness straight. > > Of course, the point of the comment is precisely that it doesn't > matter... :) > > > Yeah, I didn't want to trouble the reader with this subtlety, but you > are technically correct. I'll update the comment. > > > > > + */ > > this->brw_surfaceformat = BRW_SURFACEFORMAT_B8G8R8A8_UNORM; > > + break; > > + case MESA_FORMAT_Z16: > > + /* The miptree consists of 16 bits per pixel of depth data. > Since depth > > + * values don't require any blending, it doesn't matter how > we interpret > > + * the bit pattern as long as we copy the right amount of > data, so just > > + * map is as 8-bit RG. > > + */ > > + this->brw_surfaceformat = BRW_SURFACEFORMAT_R8G8_UNORM; > > + break; > > + default: > > + /* Blorp blits don't support any sort of format conversion, > so we can > > + * safely assume that the same format is being used for the > source and > > + * destination. Therefore the format must be supported as > a render > > + * target, even if this is the source image. So we can > convert to a > > + * surface format using brw->render_target_format. > > + */ > > + assert(brw->format_supported_as_render_target[mt->format]); > > + this->brw_surfaceformat = > brw->render_target_format[mt->format]; > > Why not just do: > > default: > this->brw_surfaceformat = brw_format_for_mesa_format(mt->format); > > > The trouble is that brw->render_target_format[] and > brw_format_for_mesa_format() don't do the same thing. In particular, > brw->render_target_format maps several formats that are otherwise > unsupported as render targets onto equivalent formats that are supported: > > BRW_SURFACEFORMAT_B8G8R8X8_UNORM -> BRW_SURFACEFORMAT_B8G8R8A8_UNORM > BRW_SURFACEFORMAT_I32_FLOAT -> BRW_SURFACEFORMAT_R32_FLOAT > BRW_SURFACEFORMAT_L32_FLOAT -> BRW_SURFACEFORMAT_R32_FLOAT > BRW_SURFACEFORMAT_I16_FLOAT -> BRW_SURFACEFORMAT_R16_FLOAT > BRW_SURFACEFORMAT_L16_FLOAT -> BRW_SURFACEFORMAT_R16_FLOAT > > Blorp needs to use brw->render_target_format[] so that these mappings > will occur, otherwise it will try to map the render target using an > unsupported format.
*grumble-grumble* luminance formats *grumble* Right then :) Reviewed-by: Kenneth Graunke <kenn...@whitecape.org> _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev