Discussion:
GL_EXT_texture_integer on i965
(too old to reply)
Eric Anholt
2011-11-04 22:01:21 UTC
Permalink
Here's a patch series to get GL_EXT_texture_integer partially working
on i965, hidden under the GL 3.0 override. There's a bunch of support
I think isn't finished yet, mostly because testing is in a bad state.
A few things I noticed while looking at the spec:

Test "Per-fragment operations that require floating-point color
components, including multisample alpha operations, alpha test,
blending, and dithering, have no effect when the corresponding colors
are written to an integer color buffer."

Test texture border color

Test texture logic op

Test rendering from each internalformat

Test rendering to each internalformat

Test getteximage from each internalformat

Test teximage clamping to the range of the internalformat

Test fragment clamping

Test reading RGBA to LUMINANCE_INTEGER producing L=R

Test gettexparameter of integer border values

Test glBlitFramebuffer() for integer

Test glBlitFramebuffer() for mixing int/float buffers.
Eric Anholt
2011-11-04 22:01:25 UTC
Permalink
Like non-integer, we can't render to RGB textures, so we promote them
to RGBA.
---
src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 24 ++++++++++++++++++++++
1 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
index 04dc389..66ff1e6 100644
--- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
+++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
@@ -124,6 +124,30 @@ brw_format_for_mesa_format(gl_format mesa_format)
[MESA_FORMAT_SIGNED_RG_RGTC2] = BRW_SURFACEFORMAT_BC5_SNORM,
[MESA_FORMAT_RGB9_E5_FLOAT] = BRW_SURFACEFORMAT_R9G9B9E5_SHAREDEXP,
[MESA_FORMAT_R11_G11_B10_FLOAT] = BRW_SURFACEFORMAT_R11G11B10_FLOAT,
+
+ [MESA_FORMAT_R_INT32] = BRW_SURFACEFORMAT_R32_SINT,
+ [MESA_FORMAT_RG_INT32] = BRW_SURFACEFORMAT_R32G32_SINT,
+ [MESA_FORMAT_RGB_INT32] = BRW_SURFACEFORMAT_R32G32B32_SINT,
+ [MESA_FORMAT_RGBA_INT32] = BRW_SURFACEFORMAT_R32G32B32A32_SINT,
+
+ [MESA_FORMAT_R_UINT32] = BRW_SURFACEFORMAT_R32_UINT,
+ [MESA_FORMAT_RG_UINT32] = BRW_SURFACEFORMAT_R32G32_UINT,
+ [MESA_FORMAT_RGB_UINT32] = BRW_SURFACEFORMAT_R32G32B32_UINT,
+ [MESA_FORMAT_RGBA_UINT32] = BRW_SURFACEFORMAT_R32G32B32A32_UINT,
+
+ [MESA_FORMAT_RGBA_UINT16] = BRW_SURFACEFORMAT_R16G16B16A16_UINT,
+ [MESA_FORMAT_RGBA_INT16] = BRW_SURFACEFORMAT_R16G16B16A16_SINT,
+ [MESA_FORMAT_RG_UINT16] = BRW_SURFACEFORMAT_R16G16_UINT,
+ [MESA_FORMAT_RG_INT16] = BRW_SURFACEFORMAT_R16G16_SINT,
+ [MESA_FORMAT_R_UINT16] = BRW_SURFACEFORMAT_R16_UINT,
+ [MESA_FORMAT_R_INT16] = BRW_SURFACEFORMAT_R16_SINT,
+
+ [MESA_FORMAT_RGBA_UINT8] = BRW_SURFACEFORMAT_R8G8B8A8_UINT,
+ [MESA_FORMAT_RGBA_INT8] = BRW_SURFACEFORMAT_R8G8B8A8_SINT,
+ [MESA_FORMAT_RG_UINT8] = BRW_SURFACEFORMAT_R8G8_UINT,
+ [MESA_FORMAT_RG_INT8] = BRW_SURFACEFORMAT_R8G8_SINT,
+ [MESA_FORMAT_R_UINT8] = BRW_SURFACEFORMAT_R8_UINT,
+ [MESA_FORMAT_R_INT8] = BRW_SURFACEFORMAT_R8_SINT,
};
assert(mesa_format < MESA_FORMAT_COUNT);
return table[mesa_format];
--
1.7.7
Eric Anholt
2011-11-04 22:01:22 UTC
Permalink
Texture filtering is not supported on floating-point textures until
gen5, which showed up as failures in texwrap in particular.
---
src/mesa/drivers/dri/intel/intel_extensions.c | 13 +++++++------
1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/mesa/drivers/dri/intel/intel_extensions.c b/src/mesa/drivers/dri/intel/intel_extensions.c
index 7a5ef3e..5212168 100644
--- a/src/mesa/drivers/dri/intel/intel_extensions.c
+++ b/src/mesa/drivers/dri/intel/intel_extensions.c
@@ -93,8 +93,14 @@ intelInitExtensions(struct gl_context *ctx)
ctx->Const.GLSLVersion = 120;
_mesa_override_glsl_version(ctx);

- if (intel->gen >= 5)
+ if (intel->gen >= 5) {
ctx->Extensions.EXT_timer_query = true;
+#ifdef TEXTURE_FLOAT_ENABLED
+ ctx->Extensions.ARB_texture_float = true;
+ ctx->Extensions.EXT_texture_shared_exponent = true;
+ ctx->Extensions.EXT_packed_float = true;
+#endif
+ }

if (intel->gen >= 4) {
ctx->Extensions.ARB_color_buffer_float = true;
@@ -107,11 +113,6 @@ intelInitExtensions(struct gl_context *ctx)
ctx->Extensions.ARB_point_sprite = true;
ctx->Extensions.ARB_seamless_cube_map = true;
ctx->Extensions.ARB_shader_texture_lod = true;
-#ifdef TEXTURE_FLOAT_ENABLED
- ctx->Extensions.ARB_texture_float = true;
- ctx->Extensions.EXT_texture_shared_exponent = true;
- ctx->Extensions.EXT_packed_float = true;
-#endif
ctx->Extensions.ARB_texture_compression_rgtc = true;
ctx->Extensions.ARB_texture_rg = true;
ctx->Extensions.EXT_draw_buffers2 = true;
--
1.7.7
Kenneth Graunke
2011-11-08 20:28:20 UTC
Permalink
Post by Eric Anholt
Texture filtering is not supported on floating-point textures until
gen5, which showed up as failures in texwrap in particular.
---
src/mesa/drivers/dri/intel/intel_extensions.c | 13 +++++++------
1 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/mesa/drivers/dri/intel/intel_extensions.c b/src/mesa/drivers/dri/intel/intel_extensions.c
index 7a5ef3e..5212168 100644
--- a/src/mesa/drivers/dri/intel/intel_extensions.c
+++ b/src/mesa/drivers/dri/intel/intel_extensions.c
@@ -93,8 +93,14 @@ intelInitExtensions(struct gl_context *ctx)
ctx->Const.GLSLVersion = 120;
_mesa_override_glsl_version(ctx);
- if (intel->gen>= 5)
+ if (intel->gen>= 5) {
ctx->Extensions.EXT_timer_query = true;
+#ifdef TEXTURE_FLOAT_ENABLED
+ ctx->Extensions.ARB_texture_float = true;
+ ctx->Extensions.EXT_texture_shared_exponent = true;
+ ctx->Extensions.EXT_packed_float = true;
+#endif
+ }
if (intel->gen>= 4) {
ctx->Extensions.ARB_color_buffer_float = true;
@@ -107,11 +113,6 @@ intelInitExtensions(struct gl_context *ctx)
ctx->Extensions.ARB_point_sprite = true;
ctx->Extensions.ARB_seamless_cube_map = true;
ctx->Extensions.ARB_shader_texture_lod = true;
-#ifdef TEXTURE_FLOAT_ENABLED
- ctx->Extensions.ARB_texture_float = true;
- ctx->Extensions.EXT_texture_shared_exponent = true;
- ctx->Extensions.EXT_packed_float = true;
-#endif
ctx->Extensions.ARB_texture_compression_rgtc = true;
ctx->Extensions.ARB_texture_rg = true;
ctx->Extensions.EXT_draw_buffers2 = true;
Based on my reading of Vol_4_G45_subsystem.pdf pages 131-132, it looks
like texture filtering is supported for R9G9B9E5_SHAREDEXP, so I think
we ought to be able to leave EXT_texture_shared_exponent enabled.

It also looks like filtering is supported for R11G11B10_FLOAT, so we
might be able to leave EXT_packed_float enabled as well.

I agree that we can't do ARB_texture_float---texture filtering is
clearly not supported for R32G32B32A32_FLOAT.
Eric Anholt
2011-11-09 19:02:44 UTC
Permalink
Post by Kenneth Graunke
Post by Eric Anholt
Texture filtering is not supported on floating-point textures until
gen5, which showed up as failures in texwrap in particular.
---
src/mesa/drivers/dri/intel/intel_extensions.c | 13 +++++++------
1 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/mesa/drivers/dri/intel/intel_extensions.c b/src/mesa/drivers/dri/intel/intel_extensions.c
index 7a5ef3e..5212168 100644
--- a/src/mesa/drivers/dri/intel/intel_extensions.c
+++ b/src/mesa/drivers/dri/intel/intel_extensions.c
@@ -93,8 +93,14 @@ intelInitExtensions(struct gl_context *ctx)
ctx->Const.GLSLVersion = 120;
_mesa_override_glsl_version(ctx);
- if (intel->gen>= 5)
+ if (intel->gen>= 5) {
ctx->Extensions.EXT_timer_query = true;
+#ifdef TEXTURE_FLOAT_ENABLED
+ ctx->Extensions.ARB_texture_float = true;
+ ctx->Extensions.EXT_texture_shared_exponent = true;
+ ctx->Extensions.EXT_packed_float = true;
+#endif
+ }
if (intel->gen>= 4) {
ctx->Extensions.ARB_color_buffer_float = true;
@@ -107,11 +113,6 @@ intelInitExtensions(struct gl_context *ctx)
ctx->Extensions.ARB_point_sprite = true;
ctx->Extensions.ARB_seamless_cube_map = true;
ctx->Extensions.ARB_shader_texture_lod = true;
-#ifdef TEXTURE_FLOAT_ENABLED
- ctx->Extensions.ARB_texture_float = true;
- ctx->Extensions.EXT_texture_shared_exponent = true;
- ctx->Extensions.EXT_packed_float = true;
-#endif
ctx->Extensions.ARB_texture_compression_rgtc = true;
ctx->Extensions.ARB_texture_rg = true;
ctx->Extensions.EXT_draw_buffers2 = true;
Based on my reading of Vol_4_G45_subsystem.pdf pages 131-132, it looks
like texture filtering is supported for R9G9B9E5_SHAREDEXP, so I think
we ought to be able to leave EXT_texture_shared_exponent enabled.
It also looks like filtering is supported for R11G11B10_FLOAT, so we
might be able to leave EXT_packed_float enabled as well.
I agree that we can't do ARB_texture_float---texture filtering is
clearly not supported for R32G32B32A32_FLOAT.
Actually, if float16 works, then we can do ARB_texture_float on gen4 as
long as we don't claim 3.0. The 2.x spec on sized internalformat lets
us choose something "close", where "close" is up to us.

I'll pull this out of the series and prod gen4 some more.
Eric Anholt
2011-11-04 22:01:23 UTC
Permalink
---
docs/relnotes-7.12.html | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/docs/relnotes-7.12.html b/docs/relnotes-7.12.html
index a92278e..06aeda0 100644
--- a/docs/relnotes-7.12.html
+++ b/docs/relnotes-7.12.html
@@ -41,7 +41,7 @@ tbd
<li>GL_ARB_vertex_type_2_10_10_10_rev (r600g)
<li>GL_ARB_texture_storage (gallium drivers and swrast)
<li>GL_EXT_packed_float (i965)
-<li>GL_EXT_texture_array (r600g)
+<li>GL_EXT_texture_array (r600g, i965)
<li>GL_EXT_texture_shared_exponent (i965)
<li>GL_NV_fog_distance (all gallium drivers, nouveau classic)
<li>GL_NV_primitive_restart (r600g)
--
1.7.7
Eric Anholt
2011-11-04 22:01:24 UTC
Permalink
This will let the feature be incrementally developed, hidden behind
the flag we're all using as we work on GL 3.0 support.
---
src/mesa/drivers/dri/intel/intel_extensions.c | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/src/mesa/drivers/dri/intel/intel_extensions.c b/src/mesa/drivers/dri/intel/intel_extensions.c
index 5212168..effa3bf 100644
--- a/src/mesa/drivers/dri/intel/intel_extensions.c
+++ b/src/mesa/drivers/dri/intel/intel_extensions.c
@@ -41,6 +41,14 @@ void
intelInitExtensions(struct gl_context *ctx)
{
struct intel_context *intel = intel_context(ctx);
+ char *override = getenv("MESA_GL_VERSION_OVERRIDE");
+ int override_major, override_minor;
+ int override_version = 0;
+
+ if (override &&
+ sscanf(override, "%u.%u", &override_major, &override_minor) == 2) {
+ override_version = override_major * 10 + override_minor;
+ }

ctx->Extensions.ARB_draw_elements_base_vertex = true;
ctx->Extensions.ARB_explicit_attrib_location = true;
@@ -118,6 +126,8 @@ intelInitExtensions(struct gl_context *ctx)
ctx->Extensions.EXT_draw_buffers2 = true;
ctx->Extensions.EXT_framebuffer_sRGB = true;
ctx->Extensions.EXT_texture_array = true;
+ if (override_version >= 30)
+ ctx->Extensions.EXT_texture_integer = true;
ctx->Extensions.EXT_texture_snorm = true;
ctx->Extensions.EXT_texture_sRGB = true;
ctx->Extensions.EXT_texture_sRGB_decode = true;
--
1.7.7
Eric Anholt
2011-11-04 22:01:26 UTC
Permalink
We don't get to use the 3-component formats, but faking of RGB with
RGBA seems to be working.
---
src/mesa/drivers/dri/intel/intel_context.c | 28 ++++++++++++++++++++++++++++
1 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/src/mesa/drivers/dri/intel/intel_context.c b/src/mesa/drivers/dri/intel/intel_context.c
index 801b747..6b493e1 100644
--- a/src/mesa/drivers/dri/intel/intel_context.c
+++ b/src/mesa/drivers/dri/intel/intel_context.c
@@ -706,6 +706,34 @@ intelInitContext(struct intel_context *intel,
ctx->TextureFormatSupported[MESA_FORMAT_SLA8] = true;
}

+ if (intel->gen >= 4) {
+ /* Each combination of 32-bit ints are supported, but the RGB 32-bit ints
+ * don't support use as a render target (GPU hangs).
+ */
+ ctx->TextureFormatSupported[MESA_FORMAT_R_INT32] = GL_TRUE;
+ ctx->TextureFormatSupported[MESA_FORMAT_RG_INT32] = GL_TRUE;
+ ctx->TextureFormatSupported[MESA_FORMAT_RGBA_INT32] = GL_TRUE;
+
+ ctx->TextureFormatSupported[MESA_FORMAT_R_UINT32] = GL_TRUE;
+ ctx->TextureFormatSupported[MESA_FORMAT_RG_UINT32] = GL_TRUE;
+ ctx->TextureFormatSupported[MESA_FORMAT_RGBA_UINT32] = GL_TRUE;
+
+ /* For 16 and 8 bits, RGB is unsupported entirely. */
+ ctx->TextureFormatSupported[MESA_FORMAT_R_UINT16] = GL_TRUE;
+ ctx->TextureFormatSupported[MESA_FORMAT_RG_UINT16] = GL_TRUE;
+ ctx->TextureFormatSupported[MESA_FORMAT_RGBA_UINT16] = GL_TRUE;
+ ctx->TextureFormatSupported[MESA_FORMAT_R_INT16] = GL_TRUE;
+ ctx->TextureFormatSupported[MESA_FORMAT_RG_INT16] = GL_TRUE;
+ ctx->TextureFormatSupported[MESA_FORMAT_RGBA_INT16] = GL_TRUE;
+
+ ctx->TextureFormatSupported[MESA_FORMAT_R_UINT8] = GL_TRUE;
+ ctx->TextureFormatSupported[MESA_FORMAT_RG_UINT8] = GL_TRUE;
+ ctx->TextureFormatSupported[MESA_FORMAT_RGBA_UINT8] = GL_TRUE;
+ ctx->TextureFormatSupported[MESA_FORMAT_R_INT8] = GL_TRUE;
+ ctx->TextureFormatSupported[MESA_FORMAT_RG_INT8] = GL_TRUE;
+ ctx->TextureFormatSupported[MESA_FORMAT_RGBA_INT8] = GL_TRUE;
+ }
+
#ifdef TEXTURE_FLOAT_ENABLED
ctx->TextureFormatSupported[MESA_FORMAT_RGBA_FLOAT32] = true;
ctx->TextureFormatSupported[MESA_FORMAT_RG_FLOAT32] = true;
--
1.7.7
Kenneth Graunke
2011-11-08 20:47:13 UTC
Permalink
Post by Eric Anholt
We don't get to use the 3-component formats, but faking of RGB with
RGBA seems to be working.
---
src/mesa/drivers/dri/intel/intel_context.c | 28 ++++++++++++++++++++++++++++
1 files changed, 28 insertions(+), 0 deletions(-)
diff --git a/src/mesa/drivers/dri/intel/intel_context.c b/src/mesa/drivers/dri/intel/intel_context.c
index 801b747..6b493e1 100644
--- a/src/mesa/drivers/dri/intel/intel_context.c
+++ b/src/mesa/drivers/dri/intel/intel_context.c
@@ -706,6 +706,34 @@ intelInitContext(struct intel_context *intel,
ctx->TextureFormatSupported[MESA_FORMAT_SLA8] = true;
}
+ if (intel->gen>= 4) {
+ /* Each combination of 32-bit ints are supported, but the RGB 32-bit ints
+ * don't support use as a render target (GPU hangs).
+ */
+ ctx->TextureFormatSupported[MESA_FORMAT_R_INT32] = GL_TRUE;
+ ctx->TextureFormatSupported[MESA_FORMAT_RG_INT32] = GL_TRUE;
+ ctx->TextureFormatSupported[MESA_FORMAT_RGBA_INT32] = GL_TRUE;
+
+ ctx->TextureFormatSupported[MESA_FORMAT_R_UINT32] = GL_TRUE;
+ ctx->TextureFormatSupported[MESA_FORMAT_RG_UINT32] = GL_TRUE;
+ ctx->TextureFormatSupported[MESA_FORMAT_RGBA_UINT32] = GL_TRUE;
+
+ /* For 16 and 8 bits, RGB is unsupported entirely. */
+ ctx->TextureFormatSupported[MESA_FORMAT_R_UINT16] = GL_TRUE;
+ ctx->TextureFormatSupported[MESA_FORMAT_RG_UINT16] = GL_TRUE;
+ ctx->TextureFormatSupported[MESA_FORMAT_RGBA_UINT16] = GL_TRUE;
+ ctx->TextureFormatSupported[MESA_FORMAT_R_INT16] = GL_TRUE;
+ ctx->TextureFormatSupported[MESA_FORMAT_RG_INT16] = GL_TRUE;
+ ctx->TextureFormatSupported[MESA_FORMAT_RGBA_INT16] = GL_TRUE;
+
+ ctx->TextureFormatSupported[MESA_FORMAT_R_UINT8] = GL_TRUE;
+ ctx->TextureFormatSupported[MESA_FORMAT_RG_UINT8] = GL_TRUE;
+ ctx->TextureFormatSupported[MESA_FORMAT_RGBA_UINT8] = GL_TRUE;
+ ctx->TextureFormatSupported[MESA_FORMAT_R_INT8] = GL_TRUE;
+ ctx->TextureFormatSupported[MESA_FORMAT_RG_INT8] = GL_TRUE;
+ ctx->TextureFormatSupported[MESA_FORMAT_RGBA_INT8] = GL_TRUE;
+ }
+
#ifdef TEXTURE_FLOAT_ENABLED
ctx->TextureFormatSupported[MESA_FORMAT_RGBA_FLOAT32] = true;
ctx->TextureFormatSupported[MESA_FORMAT_RG_FLOAT32] = true;
I think you mean "true" here. Otherwise,
Reviewed-by: Kenneth Graunke <***@whitecape.org>
Eric Anholt
2011-11-04 22:01:27 UTC
Permalink
Before, I was tracking the ir_variable * found for gl_FragColor or
gl_FragData[]. Instead, when visiting those variables, set up an
array of per-render-target fs_regs to copy the output data from. This
cleans up the color emit path, while making handling of multiple
user-defined out variables easier.

Note that brw_type_for_base_type() now returns a sensible value for
arrays. This probably obsoletes a bunch of type overrides we have
laying around.
---
src/mesa/drivers/dri/i965/brw_fs.h | 8 +-
src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 80 ++++++++++++++-----------
src/mesa/drivers/dri/i965/brw_shader.cpp | 1 +
3 files changed, 50 insertions(+), 39 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
index e2ad649..854a935 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.h
+++ b/src/mesa/drivers/dri/i965/brw_fs.h
@@ -376,9 +376,8 @@ public:
else
this->reg_null_cmp = reg_null_f;

- this->frag_color = NULL;
- this->frag_data = NULL;
this->frag_depth = NULL;
+ memset(this->outputs, 0, sizeof(this->outputs));
this->first_non_payload_grf = 0;

this->current_annotation = NULL;
@@ -526,7 +525,7 @@ public:
void emit_if_gen6(ir_if *ir);
void emit_unspill(fs_inst *inst, fs_reg reg, uint32_t spill_offset);

- void emit_color_write(int index, int first_color_mrf, fs_reg color);
+ void emit_color_write(int target, int index, int first_color_mrf);
void emit_fb_writes();
bool try_rewrite_rhs_to_dst(ir_assignment *ir,
fs_reg dst,
@@ -574,7 +573,8 @@ public:
int *params_remap;

struct hash_table *variable_ht;
- ir_variable *frag_color, *frag_data, *frag_depth;
+ ir_variable *frag_depth;
+ fs_reg outputs[BRW_MAX_DRAW_BUFFERS];
int first_non_payload_grf;
int urb_setup[FRAG_ATTRIB_MAX];
bool kill_emitted;
diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
index 15009dc..e28a2e4 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
@@ -58,14 +58,6 @@ fs_visitor::visit(ir_variable *ir)
if (variable_storage(ir))
return;

- if (strcmp(ir->name, "gl_FragColor") == 0) {
- this->frag_color = ir;
- } else if (strcmp(ir->name, "gl_FragData") == 0) {
- this->frag_data = ir;
- } else if (strcmp(ir->name, "gl_FragDepth") == 0) {
- this->frag_depth = ir;
- }
-
if (ir->mode == ir_var_in) {
if (!strcmp(ir->name, "gl_FragCoord")) {
reg = emit_fragcoord_interpolation(ir);
@@ -77,9 +69,32 @@ fs_visitor::visit(ir_variable *ir)
assert(reg);
hash_table_insert(this->variable_ht, reg, ir);
return;
- }
+ } else if (ir->mode == ir_var_out) {
+ reg = new(this->mem_ctx) fs_reg(this, ir->type);
+
+ if (strcmp(ir->name, "gl_FragColor") == 0) {
+ for (int i = 0; i < c->key.nr_color_regions; i++) {
+ this->outputs[i] = *reg;
+ }
+ } else if (strcmp(ir->name, "gl_FragData") == 0) {
+ for (int i = 0; i < c->key.nr_color_regions; i++) {
+ this->outputs[i] = *reg;
+ this->outputs[i].reg_offset += 4 * i;
+ }
+ } else if (strcmp(ir->name, "gl_FragDepth") == 0) {
+ this->frag_depth = ir;
+ } else {
+ assert(ir->location >= FRAG_RESULT_DATA0 &&
+ ir->location < FRAG_RESULT_DATA0 + BRW_MAX_DRAW_BUFFERS);

- if (ir->mode == ir_var_uniform) {
+ /* General color output. */
+ for (unsigned int i = 0; i < MAX2(1, ir->type->length); i++) {
+ int output = ir->location - FRAG_RESULT_DATA0 + i;
+ this->outputs[output] = *reg;
+ this->outputs[output].reg_offset += 4 * i;
+ }
+ }
+ } else if (ir->mode == ir_var_uniform) {
int param_index = c->prog_data.nr_params;

if (c->dispatch_width == 16) {
@@ -1830,10 +1845,18 @@ fs_visitor::emit_interpolation_setup_gen6()
}

void
-fs_visitor::emit_color_write(int index, int first_color_mrf, fs_reg color)
+fs_visitor::emit_color_write(int target, int index, int first_color_mrf)
{
int reg_width = c->dispatch_width / 8;
fs_inst *inst;
+ fs_reg color = outputs[target];
+ fs_reg mrf;
+
+ /* If there's no color data to be written, skip it. */
+ if (color.file == BAD_FILE)
+ return;
+
+ color.reg_offset += index;

if (c->dispatch_width == 8 || intel->gen >= 6) {
/* SIMD8 write looks like:
@@ -1853,7 +1876,7 @@ fs_visitor::emit_color_write(int index, int first_color_mrf, fs_reg color)
* m + 7: a1
*/
inst = emit(BRW_OPCODE_MOV,
- fs_reg(MRF, first_color_mrf + index * reg_width),
+ fs_reg(MRF, first_color_mrf + index * reg_width, color.type),
color);
inst->saturate = c->key.clamp_fragment_color;
} else {
@@ -1874,19 +1897,22 @@ fs_visitor::emit_color_write(int index, int first_color_mrf, fs_reg color)
* destination + 4.
*/
inst = emit(BRW_OPCODE_MOV,
- fs_reg(MRF, BRW_MRF_COMPR4 + first_color_mrf + index),
+ fs_reg(MRF, BRW_MRF_COMPR4 + first_color_mrf + index,
+ color.type),
color);
inst->saturate = c->key.clamp_fragment_color;
} else {
push_force_uncompressed();
- inst = emit(BRW_OPCODE_MOV, fs_reg(MRF, first_color_mrf + index),
+ inst = emit(BRW_OPCODE_MOV, fs_reg(MRF, first_color_mrf + index,
+ color.type),
color);
inst->saturate = c->key.clamp_fragment_color;
pop_force_uncompressed();

push_force_sechalf();
color.sechalf = true;
- inst = emit(BRW_OPCODE_MOV, fs_reg(MRF, first_color_mrf + index + 4),
+ inst = emit(BRW_OPCODE_MOV, fs_reg(MRF, first_color_mrf + index + 4,
+ color.type),
color);
inst->saturate = c->key.clamp_fragment_color;
pop_force_sechalf();
@@ -1956,27 +1982,12 @@ fs_visitor::emit_fb_writes()
nr += reg_width;
}

- fs_reg color = reg_undef;
- if (this->frag_color)
- color = *(variable_storage(this->frag_color));
- else if (this->frag_data) {
- color = *(variable_storage(this->frag_data));
- color.type = BRW_REGISTER_TYPE_F;
- }
-
for (int target = 0; target < c->key.nr_color_regions; target++) {
this->current_annotation = ralloc_asprintf(this->mem_ctx,
"FB write target %d",
target);
- if (this->frag_color || this->frag_data) {
- for (int i = 0; i < 4; i++) {
- emit_color_write(i, color_mrf, color);
- color.reg_offset++;
- }
- }
-
- if (this->frag_color)
- color.reg_offset -= 4;
+ for (int i = 0; i < 4; i++)
+ emit_color_write(target, i, color_mrf);

fs_inst *inst = emit(FS_OPCODE_FB_WRITE);
inst->target = target;
@@ -1988,13 +1999,12 @@ fs_visitor::emit_fb_writes()
}

if (c->key.nr_color_regions == 0) {
- if (c->key.alpha_test && (this->frag_color || this->frag_data)) {
+ if (c->key.alpha_test) {
/* If the alpha test is enabled but there's no color buffer,
* we still need to send alpha out the pipeline to our null
* renderbuffer.
*/
- color.reg_offset += 3;
- emit_color_write(3, color_mrf, color);
+ emit_color_write(0, 3, color_mrf);
}

fs_inst *inst = emit(FS_OPCODE_FB_WRITE);
diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp b/src/mesa/drivers/dri/i965/brw_shader.cpp
index d9d9414..f57b2e6 100644
--- a/src/mesa/drivers/dri/i965/brw_shader.cpp
+++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
@@ -170,6 +170,7 @@ brw_type_for_base_type(const struct glsl_type *type)
case GLSL_TYPE_UINT:
return BRW_REGISTER_TYPE_UD;
case GLSL_TYPE_ARRAY:
+ return brw_type_for_base_type(type->fields.array);
case GLSL_TYPE_STRUCT:
case GLSL_TYPE_SAMPLER:
/* These should be overridden with the type of the member when
--
1.7.7
Ian Romanick
2011-11-08 22:50:30 UTC
Permalink
Post by Eric Anholt
Before, I was tracking the ir_variable * found for gl_FragColor or
gl_FragData[]. Instead, when visiting those variables, set up an
array of per-render-target fs_regs to copy the output data from. This
cleans up the color emit path, while making handling of multiple
user-defined out variables easier.
Note that brw_type_for_base_type() now returns a sensible value for
arrays. This probably obsoletes a bunch of type overrides we have
laying around.
---
src/mesa/drivers/dri/i965/brw_fs.h | 8 +-
src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 80 ++++++++++++++-----------
src/mesa/drivers/dri/i965/brw_shader.cpp | 1 +
3 files changed, 50 insertions(+), 39 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
index e2ad649..854a935 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.h
+++ b/src/mesa/drivers/dri/i965/brw_fs.h
else
this->reg_null_cmp = reg_null_f;
- this->frag_color = NULL;
- this->frag_data = NULL;
this->frag_depth = NULL;
+ memset(this->outputs, 0, sizeof(this->outputs));
this->first_non_payload_grf = 0;
this->current_annotation = NULL;
void emit_if_gen6(ir_if *ir);
void emit_unspill(fs_inst *inst, fs_reg reg, uint32_t spill_offset);
- void emit_color_write(int index, int first_color_mrf, fs_reg color);
+ void emit_color_write(int target, int index, int first_color_mrf);
void emit_fb_writes();
bool try_rewrite_rhs_to_dst(ir_assignment *ir,
fs_reg dst,
int *params_remap;
struct hash_table *variable_ht;
- ir_variable *frag_color, *frag_data, *frag_depth;
+ ir_variable *frag_depth;
+ fs_reg outputs[BRW_MAX_DRAW_BUFFERS];
int first_non_payload_grf;
int urb_setup[FRAG_ATTRIB_MAX];
bool kill_emitted;
diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
index 15009dc..e28a2e4 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
@@ -58,14 +58,6 @@ fs_visitor::visit(ir_variable *ir)
if (variable_storage(ir))
return;
- if (strcmp(ir->name, "gl_FragColor") == 0) {
- this->frag_color = ir;
- } else if (strcmp(ir->name, "gl_FragData") == 0) {
- this->frag_data = ir;
- } else if (strcmp(ir->name, "gl_FragDepth") == 0) {
- this->frag_depth = ir;
- }
-
if (ir->mode == ir_var_in) {
if (!strcmp(ir->name, "gl_FragCoord")) {
reg = emit_fragcoord_interpolation(ir);
@@ -77,9 +69,32 @@ fs_visitor::visit(ir_variable *ir)
assert(reg);
hash_table_insert(this->variable_ht, reg, ir);
return;
- }
+ } else if (ir->mode == ir_var_out) {
+ reg = new(this->mem_ctx) fs_reg(this, ir->type);
+
+ if (strcmp(ir->name, "gl_FragColor") == 0) {
+ for (int i = 0; i< c->key.nr_color_regions; i++) {
+ this->outputs[i] = *reg;
+ }
+ } else if (strcmp(ir->name, "gl_FragData") == 0) {
+ for (int i = 0; i< c->key.nr_color_regions; i++) {
+ this->outputs[i] = *reg;
+ this->outputs[i].reg_offset += 4 * i;
+ }
+ } else if (strcmp(ir->name, "gl_FragDepth") == 0) {
+ this->frag_depth = ir;
+ } else {
+ assert(ir->location>= FRAG_RESULT_DATA0&&
+ ir->location< FRAG_RESULT_DATA0 + BRW_MAX_DRAW_BUFFERS);
- if (ir->mode == ir_var_uniform) {
+ /* General color output. */
+ for (unsigned int i = 0; i< MAX2(1, ir->type->length); i++) {
+ int output = ir->location - FRAG_RESULT_DATA0 + i;
+ this->outputs[output] = *reg;
+ this->outputs[output].reg_offset += 4 * i;
+ }
+ }
Instead of using strcmp, why not just use ir->location? This should
allow the gl_FragData and the general color output cases to be merged
(since they'll have the same locations).
Post by Eric Anholt
+ } else if (ir->mode == ir_var_uniform) {
int param_index = c->prog_data.nr_params;
if (c->dispatch_width == 16) {
@@ -1830,10 +1845,18 @@ fs_visitor::emit_interpolation_setup_gen6()
}
void
-fs_visitor::emit_color_write(int index, int first_color_mrf, fs_reg color)
+fs_visitor::emit_color_write(int target, int index, int first_color_mrf)
{
int reg_width = c->dispatch_width / 8;
fs_inst *inst;
+ fs_reg color = outputs[target];
+ fs_reg mrf;
+
+ /* If there's no color data to be written, skip it. */
+ if (color.file == BAD_FILE)
+ return;
+
+ color.reg_offset += index;
if (c->dispatch_width == 8 || intel->gen>= 6) {
@@ -1853,7 +1876,7 @@ fs_visitor::emit_color_write(int index, int first_color_mrf, fs_reg color)
* m + 7: a1
*/
inst = emit(BRW_OPCODE_MOV,
- fs_reg(MRF, first_color_mrf + index * reg_width),
+ fs_reg(MRF, first_color_mrf + index * reg_width, color.type),
color);
inst->saturate = c->key.clamp_fragment_color;
} else {
@@ -1874,19 +1897,22 @@ fs_visitor::emit_color_write(int index, int first_color_mrf, fs_reg color)
* destination + 4.
*/
inst = emit(BRW_OPCODE_MOV,
- fs_reg(MRF, BRW_MRF_COMPR4 + first_color_mrf + index),
+ fs_reg(MRF, BRW_MRF_COMPR4 + first_color_mrf + index,
+ color.type),
color);
inst->saturate = c->key.clamp_fragment_color;
} else {
push_force_uncompressed();
- inst = emit(BRW_OPCODE_MOV, fs_reg(MRF, first_color_mrf + index),
+ inst = emit(BRW_OPCODE_MOV, fs_reg(MRF, first_color_mrf + index,
+ color.type),
color);
inst->saturate = c->key.clamp_fragment_color;
pop_force_uncompressed();
push_force_sechalf();
color.sechalf = true;
- inst = emit(BRW_OPCODE_MOV, fs_reg(MRF, first_color_mrf + index + 4),
+ inst = emit(BRW_OPCODE_MOV, fs_reg(MRF, first_color_mrf + index + 4,
+ color.type),
color);
inst->saturate = c->key.clamp_fragment_color;
pop_force_sechalf();
@@ -1956,27 +1982,12 @@ fs_visitor::emit_fb_writes()
nr += reg_width;
}
- fs_reg color = reg_undef;
- if (this->frag_color)
- color = *(variable_storage(this->frag_color));
- else if (this->frag_data) {
- color = *(variable_storage(this->frag_data));
- color.type = BRW_REGISTER_TYPE_F;
- }
-
for (int target = 0; target< c->key.nr_color_regions; target++) {
this->current_annotation = ralloc_asprintf(this->mem_ctx,
"FB write target %d",
target);
- if (this->frag_color || this->frag_data) {
- for (int i = 0; i< 4; i++) {
- emit_color_write(i, color_mrf, color);
- color.reg_offset++;
- }
- }
-
- if (this->frag_color)
- color.reg_offset -= 4;
+ for (int i = 0; i< 4; i++)
+ emit_color_write(target, i, color_mrf);
fs_inst *inst = emit(FS_OPCODE_FB_WRITE);
inst->target = target;
@@ -1988,13 +1999,12 @@ fs_visitor::emit_fb_writes()
}
if (c->key.nr_color_regions == 0) {
- if (c->key.alpha_test&& (this->frag_color || this->frag_data)) {
+ if (c->key.alpha_test) {
/* If the alpha test is enabled but there's no color buffer,
* we still need to send alpha out the pipeline to our null
* renderbuffer.
*/
- color.reg_offset += 3;
- emit_color_write(3, color_mrf, color);
+ emit_color_write(0, 3, color_mrf);
}
fs_inst *inst = emit(FS_OPCODE_FB_WRITE);
diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp b/src/mesa/drivers/dri/i965/brw_shader.cpp
index d9d9414..f57b2e6 100644
--- a/src/mesa/drivers/dri/i965/brw_shader.cpp
+++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
@@ -170,6 +170,7 @@ brw_type_for_base_type(const struct glsl_type *type)
return BRW_REGISTER_TYPE_UD;
+ return brw_type_for_base_type(type->fields.array);
/* These should be overridden with the type of the member when
Kenneth Graunke
2011-11-09 03:21:59 UTC
Permalink
Post by Eric Anholt
Before, I was tracking the ir_variable * found for gl_FragColor or
gl_FragData[]. Instead, when visiting those variables, set up an
array of per-render-target fs_regs to copy the output data from. This
cleans up the color emit path, while making handling of multiple
user-defined out variables easier.
Note that brw_type_for_base_type() now returns a sensible value for
arrays. This probably obsoletes a bunch of type overrides we have
laying around.
---
src/mesa/drivers/dri/i965/brw_fs.h | 8 +-
src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 80 ++++++++++++++-----------
src/mesa/drivers/dri/i965/brw_shader.cpp | 1 +
3 files changed, 50 insertions(+), 39 deletions(-)
I like these changes. However, it feels like you're really doing three
separate changes in one patch. I went ahead and split it up, and will
post those shortly.

A few more comments inline.
Post by Eric Anholt
diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
index e2ad649..854a935 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.h
+++ b/src/mesa/drivers/dri/i965/brw_fs.h
[snip]
Post by Eric Anholt
@@ -77,9 +69,32 @@ fs_visitor::visit(ir_variable *ir)
assert(reg);
hash_table_insert(this->variable_ht, reg, ir);
return;
- }
+ } else if (ir->mode == ir_var_out) {
+ reg = new(this->mem_ctx) fs_reg(this, ir->type);
+
+ if (strcmp(ir->name, "gl_FragColor") == 0) {
+ for (int i = 0; i < c->key.nr_color_regions; i++) {
+ this->outputs[i] = *reg;
+ }
+ } else if (strcmp(ir->name, "gl_FragData") == 0) {
+ for (int i = 0; i < c->key.nr_color_regions; i++) {
+ this->outputs[i] = *reg;
+ this->outputs[i].reg_offset += 4 * i;
+ }
+ } else if (strcmp(ir->name, "gl_FragDepth") == 0) {
+ this->frag_depth = ir;
+ } else {
+ assert(ir->location >= FRAG_RESULT_DATA0 &&
+ ir->location < FRAG_RESULT_DATA0 + BRW_MAX_DRAW_BUFFERS);
- if (ir->mode == ir_var_uniform) {
+ /* General color output. */
+ for (unsigned int i = 0; i < MAX2(1, ir->type->length); i++) {
+ int output = ir->location - FRAG_RESULT_DATA0 + i;
+ this->outputs[output] = *reg;
+ this->outputs[output].reg_offset += 4 * i;
+ }
+ }
+ } else if (ir->mode == ir_var_uniform) {
int param_index = c->prog_data.nr_params;
if (c->dispatch_width == 16) {
I agree with Ian here---using ir->location ought to be cleaner. The
code for gl_FragData and generic FS outputs is exactly the same...

I've gone ahead and made this change too, I'll post that shortly.

[snip]
Post by Eric Anholt
@@ -1988,13 +1999,12 @@ fs_visitor::emit_fb_writes()
}
if (c->key.nr_color_regions == 0) {
- if (c->key.alpha_test && (this->frag_color || this->frag_data)) {
+ if (c->key.alpha_test) {
/* If the alpha test is enabled but there's no color buffer,
* we still need to send alpha out the pipeline to our null
* renderbuffer.
*/
- color.reg_offset += 3;
- emit_color_write(3, color_mrf, color);
+ emit_color_write(0, 3, color_mrf);
}
I'm a little concerned about removing reg_offset += 3. This seems like
a change in behavior (assigning all four instead of just alpha). Was it
wrong before/is this a bug fix? Or, is this just simpler code (since
the other three channels are ignored anyway)?

--Kenneth
Kenneth Graunke
2011-11-09 03:26:38 UTC
Permalink
From: Eric Anholt <***@anholt.net>

Previously, brw_type_for_base_type returned UD for array variables,
similar to structures. For structures, each field may have a different
type, so every field access must explicitly override the register's type
with that field's type. We chose to return UD in this case since it was
the least common, so errors would be more obvious.

For arrays, it makes far more sense to return the type corresponding to
an element of the array. This allows normal array access to work
without the hassle of explicitly overriding the register's type.

This should obsolete a bunch of type overrides throughout the code.

Signed-off-by: Eric Anholt <***@anholt.net>
Signed-off-by: Kenneth Graunke <***@whitecape.org>
---
src/mesa/drivers/dri/i965/brw_shader.cpp | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

(Feel free to change my Signed-off-by to a Reviewed-by if you prefer.)

diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp b/src/mesa/drivers/dri/i965/brw_shader.cpp
index 7679b6e..f25fab3 100644
--- a/src/mesa/drivers/dri/i965/brw_shader.cpp
+++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
@@ -170,6 +170,7 @@ brw_type_for_base_type(const struct glsl_type *type)
case GLSL_TYPE_UINT:
return BRW_REGISTER_TYPE_UD;
case GLSL_TYPE_ARRAY:
+ return brw_type_for_base_type(type->fields.array);
case GLSL_TYPE_STRUCT:
case GLSL_TYPE_SAMPLER:
/* These should be overridden with the type of the member when
--
1.7.7.2
Kenneth Graunke
2011-11-09 03:26:39 UTC
Permalink
From: Eric Anholt <***@anholt.net>

When rendering to integer color buffers, we need to be careful to use
MRFs of the correct type when emitting color writes.

Signed-off-by: Eric Anholt <***@anholt.net>
Signed-off-by: Kenneth Graunke <***@whitecape.org>
---
src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 11 +++++++----
1 files changed, 7 insertions(+), 4 deletions(-)

(Feel free to change my Signed-off-by to a Reviewed-by if you prefer.)

diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
index 15009dc..3e2feaf 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
@@ -1853,7 +1853,7 @@ fs_visitor::emit_color_write(int index, int first_color_mrf, fs_reg color)
* m + 7: a1
*/
inst = emit(BRW_OPCODE_MOV,
- fs_reg(MRF, first_color_mrf + index * reg_width),
+ fs_reg(MRF, first_color_mrf + index * reg_width, color.type),
color);
inst->saturate = c->key.clamp_fragment_color;
} else {
@@ -1874,19 +1874,22 @@ fs_visitor::emit_color_write(int index, int first_color_mrf, fs_reg color)
* destination + 4.
*/
inst = emit(BRW_OPCODE_MOV,
- fs_reg(MRF, BRW_MRF_COMPR4 + first_color_mrf + index),
+ fs_reg(MRF, BRW_MRF_COMPR4 + first_color_mrf + index,
+ color.type),
color);
inst->saturate = c->key.clamp_fragment_color;
} else {
push_force_uncompressed();
- inst = emit(BRW_OPCODE_MOV, fs_reg(MRF, first_color_mrf + index),
+ inst = emit(BRW_OPCODE_MOV, fs_reg(MRF, first_color_mrf + index,
+ color.type),
color);
inst->saturate = c->key.clamp_fragment_color;
pop_force_uncompressed();

push_force_sechalf();
color.sechalf = true;
- inst = emit(BRW_OPCODE_MOV, fs_reg(MRF, first_color_mrf + index + 4),
+ inst = emit(BRW_OPCODE_MOV, fs_reg(MRF, first_color_mrf + index + 4,
+ color.type),
color);
inst->saturate = c->key.clamp_fragment_color;
pop_force_sechalf();
--
1.7.7.2
Kenneth Graunke
2011-11-09 03:26:40 UTC
Permalink
From: Eric Anholt <***@anholt.net>

Before, I was tracking the ir_variable * found for gl_FragColor or
gl_FragData[]. Instead, when visiting those variables, set up an
array of per-render-target fs_regs to copy the output data from. This
cleans up the color emit path, while making handling of multiple
user-defined out variables easier.

Signed-off-by: Eric Anholt <***@anholt.net>
Signed-off-by: Kenneth Graunke <***@whitecape.org>
---
src/mesa/drivers/dri/i965/brw_fs.h | 8 ++--
src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 69 ++++++++++++++------------
2 files changed, 42 insertions(+), 35 deletions(-)

(Feel free to change my Signed-off-by to a Reviewed-by if you prefer.)

This is the same as Eric's original patch, just split. You might want to
take v2 of this, which also incorporates Ian's feedback.

diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
index 3e45030..de31644 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.h
+++ b/src/mesa/drivers/dri/i965/brw_fs.h
@@ -376,9 +376,8 @@ public:
else
this->reg_null_cmp = reg_null_f;

- this->frag_color = NULL;
- this->frag_data = NULL;
this->frag_depth = NULL;
+ memset(this->outputs, 0, sizeof(this->outputs));
this->first_non_payload_grf = 0;

this->current_annotation = NULL;
@@ -533,7 +532,7 @@ public:
void emit_if_gen6(ir_if *ir);
void emit_unspill(fs_inst *inst, fs_reg reg, uint32_t spill_offset);

- void emit_color_write(int index, int first_color_mrf, fs_reg color);
+ void emit_color_write(int target, int index, int first_color_mrf);
void emit_fb_writes();
bool try_rewrite_rhs_to_dst(ir_assignment *ir,
fs_reg dst,
@@ -581,7 +580,8 @@ public:
int *params_remap;

struct hash_table *variable_ht;
- ir_variable *frag_color, *frag_data, *frag_depth;
+ ir_variable *frag_depth;
+ fs_reg outputs[BRW_MAX_DRAW_BUFFERS];
int first_non_payload_grf;
int urb_setup[FRAG_ATTRIB_MAX];
bool kill_emitted;
diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
index 3e2feaf..e28a2e4 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
@@ -58,14 +58,6 @@ fs_visitor::visit(ir_variable *ir)
if (variable_storage(ir))
return;

- if (strcmp(ir->name, "gl_FragColor") == 0) {
- this->frag_color = ir;
- } else if (strcmp(ir->name, "gl_FragData") == 0) {
- this->frag_data = ir;
- } else if (strcmp(ir->name, "gl_FragDepth") == 0) {
- this->frag_depth = ir;
- }
-
if (ir->mode == ir_var_in) {
if (!strcmp(ir->name, "gl_FragCoord")) {
reg = emit_fragcoord_interpolation(ir);
@@ -77,9 +69,32 @@ fs_visitor::visit(ir_variable *ir)
assert(reg);
hash_table_insert(this->variable_ht, reg, ir);
return;
- }
+ } else if (ir->mode == ir_var_out) {
+ reg = new(this->mem_ctx) fs_reg(this, ir->type);

- if (ir->mode == ir_var_uniform) {
+ if (strcmp(ir->name, "gl_FragColor") == 0) {
+ for (int i = 0; i < c->key.nr_color_regions; i++) {
+ this->outputs[i] = *reg;
+ }
+ } else if (strcmp(ir->name, "gl_FragData") == 0) {
+ for (int i = 0; i < c->key.nr_color_regions; i++) {
+ this->outputs[i] = *reg;
+ this->outputs[i].reg_offset += 4 * i;
+ }
+ } else if (strcmp(ir->name, "gl_FragDepth") == 0) {
+ this->frag_depth = ir;
+ } else {
+ assert(ir->location >= FRAG_RESULT_DATA0 &&
+ ir->location < FRAG_RESULT_DATA0 + BRW_MAX_DRAW_BUFFERS);
+
+ /* General color output. */
+ for (unsigned int i = 0; i < MAX2(1, ir->type->length); i++) {
+ int output = ir->location - FRAG_RESULT_DATA0 + i;
+ this->outputs[output] = *reg;
+ this->outputs[output].reg_offset += 4 * i;
+ }
+ }
+ } else if (ir->mode == ir_var_uniform) {
int param_index = c->prog_data.nr_params;

if (c->dispatch_width == 16) {
@@ -1830,10 +1845,18 @@ fs_visitor::emit_interpolation_setup_gen6()
}

void
-fs_visitor::emit_color_write(int index, int first_color_mrf, fs_reg color)
+fs_visitor::emit_color_write(int target, int index, int first_color_mrf)
{
int reg_width = c->dispatch_width / 8;
fs_inst *inst;
+ fs_reg color = outputs[target];
+ fs_reg mrf;
+
+ /* If there's no color data to be written, skip it. */
+ if (color.file == BAD_FILE)
+ return;
+
+ color.reg_offset += index;

if (c->dispatch_width == 8 || intel->gen >= 6) {
/* SIMD8 write looks like:
@@ -1959,27 +1982,12 @@ fs_visitor::emit_fb_writes()
nr += reg_width;
}

- fs_reg color = reg_undef;
- if (this->frag_color)
- color = *(variable_storage(this->frag_color));
- else if (this->frag_data) {
- color = *(variable_storage(this->frag_data));
- color.type = BRW_REGISTER_TYPE_F;
- }
-
for (int target = 0; target < c->key.nr_color_regions; target++) {
this->current_annotation = ralloc_asprintf(this->mem_ctx,
"FB write target %d",
target);
- if (this->frag_color || this->frag_data) {
- for (int i = 0; i < 4; i++) {
- emit_color_write(i, color_mrf, color);
- color.reg_offset++;
- }
- }
-
- if (this->frag_color)
- color.reg_offset -= 4;
+ for (int i = 0; i < 4; i++)
+ emit_color_write(target, i, color_mrf);

fs_inst *inst = emit(FS_OPCODE_FB_WRITE);
inst->target = target;
@@ -1991,13 +1999,12 @@ fs_visitor::emit_fb_writes()
}

if (c->key.nr_color_regions == 0) {
- if (c->key.alpha_test && (this->frag_color || this->frag_data)) {
+ if (c->key.alpha_test) {
/* If the alpha test is enabled but there's no color buffer,
* we still need to send alpha out the pipeline to our null
* renderbuffer.
*/
- color.reg_offset += 3;
- emit_color_write(3, color_mrf, color);
+ emit_color_write(0, 3, color_mrf);
}

fs_inst *inst = emit(FS_OPCODE_FB_WRITE);
--
1.7.7.2
Kenneth Graunke
2011-11-09 03:27:46 UTC
Permalink
From: Eric Anholt <***@anholt.net>

Before, I was tracking the ir_variable * found for gl_FragColor or
gl_FragData[]. Instead, when visiting those variables, set up an
array of per-render-target fs_regs to copy the output data from. This
cleans up the color emit path, while making handling of multiple
user-defined out variables easier.

Signed-off-by: Eric Anholt <***@anholt.net>
Signed-off-by: Kenneth Graunke <***@whitecape.org>
---
src/mesa/drivers/dri/i965/brw_fs.h | 8 ++--
src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 66 ++++++++++++++------------
2 files changed, 39 insertions(+), 35 deletions(-)

This version incorporates Ian's feedback, using ir->location and coalescing
the gl_FragData and generic FS output handling.

diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
index 3e45030..de31644 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.h
+++ b/src/mesa/drivers/dri/i965/brw_fs.h
@@ -376,9 +376,8 @@ public:
else
this->reg_null_cmp = reg_null_f;

- this->frag_color = NULL;
- this->frag_data = NULL;
this->frag_depth = NULL;
+ memset(this->outputs, 0, sizeof(this->outputs));
this->first_non_payload_grf = 0;

this->current_annotation = NULL;
@@ -533,7 +532,7 @@ public:
void emit_if_gen6(ir_if *ir);
void emit_unspill(fs_inst *inst, fs_reg reg, uint32_t spill_offset);

- void emit_color_write(int index, int first_color_mrf, fs_reg color);
+ void emit_color_write(int target, int index, int first_color_mrf);
void emit_fb_writes();
bool try_rewrite_rhs_to_dst(ir_assignment *ir,
fs_reg dst,
@@ -581,7 +580,8 @@ public:
int *params_remap;

struct hash_table *variable_ht;
- ir_variable *frag_color, *frag_data, *frag_depth;
+ ir_variable *frag_depth;
+ fs_reg outputs[BRW_MAX_DRAW_BUFFERS];
int first_non_payload_grf;
int urb_setup[FRAG_ATTRIB_MAX];
bool kill_emitted;
diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
index 3e2feaf..a76ba96 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
@@ -58,14 +58,6 @@ fs_visitor::visit(ir_variable *ir)
if (variable_storage(ir))
return;

- if (strcmp(ir->name, "gl_FragColor") == 0) {
- this->frag_color = ir;
- } else if (strcmp(ir->name, "gl_FragData") == 0) {
- this->frag_data = ir;
- } else if (strcmp(ir->name, "gl_FragDepth") == 0) {
- this->frag_depth = ir;
- }
-
if (ir->mode == ir_var_in) {
if (!strcmp(ir->name, "gl_FragCoord")) {
reg = emit_fragcoord_interpolation(ir);
@@ -77,9 +69,29 @@ fs_visitor::visit(ir_variable *ir)
assert(reg);
hash_table_insert(this->variable_ht, reg, ir);
return;
- }
+ } else if (ir->mode == ir_var_out) {
+ reg = new(this->mem_ctx) fs_reg(this, ir->type);

- if (ir->mode == ir_var_uniform) {
+ if (ir->location == FRAG_RESULT_COLOR) {
+ /* Writing gl_FragColor outputs to all color regions. */
+ for (int i = 0; i < c->key.nr_color_regions; i++) {
+ this->outputs[i] = *reg;
+ }
+ } else if (ir->location == FRAG_RESULT_DEPTH) {
+ this->frag_depth = ir;
+ } else {
+ /* gl_FragData or a user-defined FS output */
+ assert(ir->location >= FRAG_RESULT_DATA0 &&
+ ir->location < FRAG_RESULT_DATA0 + BRW_MAX_DRAW_BUFFERS);
+
+ /* General color output. */
+ for (unsigned int i = 0; i < MAX2(1, ir->type->length); i++) {
+ int output = ir->location - FRAG_RESULT_DATA0 + i;
+ this->outputs[output] = *reg;
+ this->outputs[output].reg_offset += 4 * i;
+ }
+ }
+ } else if (ir->mode == ir_var_uniform) {
int param_index = c->prog_data.nr_params;

if (c->dispatch_width == 16) {
@@ -1830,10 +1842,18 @@ fs_visitor::emit_interpolation_setup_gen6()
}

void
-fs_visitor::emit_color_write(int index, int first_color_mrf, fs_reg color)
+fs_visitor::emit_color_write(int target, int index, int first_color_mrf)
{
int reg_width = c->dispatch_width / 8;
fs_inst *inst;
+ fs_reg color = outputs[target];
+ fs_reg mrf;
+
+ /* If there's no color data to be written, skip it. */
+ if (color.file == BAD_FILE)
+ return;
+
+ color.reg_offset += index;

if (c->dispatch_width == 8 || intel->gen >= 6) {
/* SIMD8 write looks like:
@@ -1959,27 +1979,12 @@ fs_visitor::emit_fb_writes()
nr += reg_width;
}

- fs_reg color = reg_undef;
- if (this->frag_color)
- color = *(variable_storage(this->frag_color));
- else if (this->frag_data) {
- color = *(variable_storage(this->frag_data));
- color.type = BRW_REGISTER_TYPE_F;
- }
-
for (int target = 0; target < c->key.nr_color_regions; target++) {
this->current_annotation = ralloc_asprintf(this->mem_ctx,
"FB write target %d",
target);
- if (this->frag_color || this->frag_data) {
- for (int i = 0; i < 4; i++) {
- emit_color_write(i, color_mrf, color);
- color.reg_offset++;
- }
- }
-
- if (this->frag_color)
- color.reg_offset -= 4;
+ for (int i = 0; i < 4; i++)
+ emit_color_write(target, i, color_mrf);

fs_inst *inst = emit(FS_OPCODE_FB_WRITE);
inst->target = target;
@@ -1991,13 +1996,12 @@ fs_visitor::emit_fb_writes()
}

if (c->key.nr_color_regions == 0) {
- if (c->key.alpha_test && (this->frag_color || this->frag_data)) {
+ if (c->key.alpha_test) {
/* If the alpha test is enabled but there's no color buffer,
* we still need to send alpha out the pipeline to our null
* renderbuffer.
*/
- color.reg_offset += 3;
- emit_color_write(3, color_mrf, color);
+ emit_color_write(0, 3, color_mrf);
}

fs_inst *inst = emit(FS_OPCODE_FB_WRITE);
--
1.7.7.2
Eric Anholt
2011-11-09 19:52:59 UTC
Permalink
Post by Kenneth Graunke
Post by Eric Anholt
Before, I was tracking the ir_variable * found for gl_FragColor or
gl_FragData[]. Instead, when visiting those variables, set up an
array of per-render-target fs_regs to copy the output data from. This
cleans up the color emit path, while making handling of multiple
user-defined out variables easier.
Note that brw_type_for_base_type() now returns a sensible value for
arrays. This probably obsoletes a bunch of type overrides we have
laying around.
[snip]
Post by Eric Anholt
@@ -1988,13 +1999,12 @@ fs_visitor::emit_fb_writes()
}
if (c->key.nr_color_regions == 0) {
- if (c->key.alpha_test && (this->frag_color || this->frag_data)) {
+ if (c->key.alpha_test) {
/* If the alpha test is enabled but there's no color buffer,
* we still need to send alpha out the pipeline to our null
* renderbuffer.
*/
- color.reg_offset += 3;
- emit_color_write(3, color_mrf, color);
+ emit_color_write(0, 3, color_mrf);
}
I'm a little concerned about removing reg_offset += 3. This seems like
a change in behavior (assigning all four instead of just alpha). Was it
wrong before/is this a bug fix? Or, is this just simpler code (since
the other three channels are ignored anyway)?
Even simpler explanation: The reg_offset increment by index value got
moved into emit_color_write() -- note a similar change above at the
other emit_color_write().

Eric Anholt
2011-11-04 22:01:29 UTC
Permalink
This requires using a new fragment shader to get the integer color
output, and a new vertex shader because #version has to match between
the two.
---
src/mesa/drivers/common/meta.c | 108 ++++++++++++++++++++++++++++++++++++++--
1 files changed, 104 insertions(+), 4 deletions(-)

diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
index 3e55334..fcff0ae 100644
--- a/src/mesa/drivers/common/meta.c
+++ b/src/mesa/drivers/common/meta.c
@@ -182,7 +182,6 @@ struct save_state
GLboolean Lighting;
};

-
/**
* Temporary texture used for glBlitFramebuffer, glDrawPixels, etc.
* This is currently shared by all the meta ops. But we could create a
@@ -221,6 +220,9 @@ struct clear_state
GLuint VBO;
GLuint ShaderProg;
GLint ColorLocation;
+
+ GLuint IntegerShaderProg;
+ GLint IntegerColorLocation;
};


@@ -310,6 +312,67 @@ struct gl_meta_state
struct drawtex_state DrawTex; /**< For _mesa_meta_DrawTex() */
};

+static GLuint
+compile_shader_with_debug(struct gl_context *ctx, GLenum target, const GLcharARB *source)
+{
+ GLuint shader;
+ GLint ok, size;
+ GLchar *info;
+
+ shader = _mesa_CreateShaderObjectARB(target);
+ _mesa_ShaderSourceARB(shader, 1, &source, NULL);
+ _mesa_CompileShaderARB(shader);
+
+ _mesa_GetShaderiv(shader, GL_COMPILE_STATUS, &ok);
+ if (ok)
+ return shader;
+
+ _mesa_GetShaderiv(shader, GL_INFO_LOG_LENGTH, &size);
+ if (size == 0)
+ return 0;
+
+ info = malloc(size);
+ if (!info)
+ return 0;
+
+ _mesa_GetProgramInfoLog(shader, size, NULL, info);
+ _mesa_problem(ctx,
+ "meta program compile failed:\n%s\n"
+ "source:\n%s\n",
+ info, source);
+
+ free(info);
+
+ return 0;
+}
+
+static GLuint
+link_program_with_debug(struct gl_context *ctx, GLuint program)
+{
+ GLint ok, size;
+ GLchar *info;
+
+ _mesa_LinkProgramARB(program);
+
+ _mesa_GetProgramiv(program, GL_LINK_STATUS, &ok);
+ if (ok)
+ return program;
+
+ _mesa_GetProgramiv(program, GL_INFO_LOG_LENGTH, &size);
+ if (size == 0)
+ return 0;
+
+ info = malloc(size);
+ if (!info)
+ return 0;
+
+ _mesa_GetProgramInfoLog(program, size, NULL, info);
+ _mesa_problem(ctx, "meta program link failed:\n%s", info);
+
+ free(info);
+
+ return 0;
+}

/**
* Initialize meta-ops for a context.
@@ -1646,6 +1709,22 @@ meta_glsl_clear_init(struct gl_context *ctx, struct clear_state *clear)
"{\n"
" gl_FragColor = color;\n"
"}\n";
+ const char *vs_int_source =
+ "#version 130\n"
+ "attribute vec4 position;\n"
+ "void main()\n"
+ "{\n"
+ " gl_Position = position;\n"
+ "}\n";
+ const char *fs_int_source =
+ "#version 130\n"
+ "uniform ivec4 color;\n"
+ "out ivec4 out_color;\n"
+ "\n"
+ "void main()\n"
+ "{\n"
+ " out_color = color;\n"
+ "}\n";
GLuint vs, fs;

if (clear->ArrayObj != 0)
@@ -1679,6 +1758,21 @@ meta_glsl_clear_init(struct gl_context *ctx, struct clear_state *clear)

clear->ColorLocation = _mesa_GetUniformLocationARB(clear->ShaderProg,
"color");
+
+ if (ctx->Const.GLSLVersion >= 130) {
+ vs = compile_shader_with_debug(ctx, GL_VERTEX_SHADER, vs_int_source);
+ fs = compile_shader_with_debug(ctx, GL_FRAGMENT_SHADER, fs_int_source);
+
+ clear->IntegerShaderProg = _mesa_CreateProgramObjectARB();
+ _mesa_AttachShader(clear->IntegerShaderProg, fs);
+ _mesa_AttachShader(clear->IntegerShaderProg, vs);
+ _mesa_BindAttribLocationARB(clear->IntegerShaderProg, 0, "position");
+
+ link_program_with_debug(ctx, clear->IntegerShaderProg);
+
+ clear->IntegerColorLocation =
+ _mesa_GetUniformLocationARB(clear->IntegerShaderProg, "color");
+ }
}

/**
@@ -1722,9 +1816,15 @@ _mesa_meta_glsl_Clear(struct gl_context *ctx, GLbitfield buffers)

meta_glsl_clear_init(ctx, clear);

- _mesa_UseProgramObjectARB(clear->ShaderProg);
- _mesa_Uniform4fvARB(clear->ColorLocation, 1,
- ctx->Color.ClearColor.f);
+ if (fb->_IntegerColor) {
+ _mesa_UseProgramObjectARB(clear->IntegerShaderProg);
+ _mesa_Uniform4ivARB(clear->IntegerColorLocation, 1,
+ ctx->Color.ClearColor.i);
+ } else {
+ _mesa_UseProgramObjectARB(clear->ShaderProg);
+ _mesa_Uniform4fvARB(clear->ColorLocation, 1,
+ ctx->Color.ClearColor.f);
+ }

_mesa_BindVertexArray(clear->ArrayObj);
_mesa_BindBufferARB(GL_ARRAY_BUFFER_ARB, clear->VBO);
--
1.7.7
Kenneth Graunke
2011-11-09 03:46:13 UTC
Permalink
Post by Eric Anholt
This requires using a new fragment shader to get the integer color
output, and a new vertex shader because #version has to match between
the two.
Hm. The extra VS is rather unfortunate (it's exactly the same other
than the #version), but I suppose necessary.

One comment...

You don't appear to have any code to associate "out_color" with a
location; I suppose it's implicitly 0 since it's the only output. Is it
worth setting this explicitly?

I'll accept "No, it's fine" as an answer. :)

Reviewed-by: Kenneth Graunke <***@whitecape.org>
Eric Anholt
2011-11-09 19:46:08 UTC
Permalink
Post by Kenneth Graunke
Post by Eric Anholt
This requires using a new fragment shader to get the integer color
output, and a new vertex shader because #version has to match between
the two.
Hm. The extra VS is rather unfortunate (it's exactly the same other
than the #version), but I suppose necessary.
One comment...
You don't appear to have any code to associate "out_color" with a
location; I suppose it's implicitly 0 since it's the only output. Is it
worth setting this explicitly?
I'll accept "No, it's fine" as an answer. :)
Given that I had to look up the spec saying that assignment starts from
0, yes, it does merit a comment explaining why this works.
Eric Anholt
2011-11-04 22:01:30 UTC
Permalink
This is the inverse operation to _mesa_pack_rgba_span_int. The 16-bit
code isn't done because of lack of testing and not being sure how sign
extension/clamping should be handled between, say, 16-bit int and
32-bit int or uint.
---
src/mesa/main/format_unpack.c | 120 +++++++++++++++++++++++++++++++++++++++++
src/mesa/main/format_unpack.h | 8 +++
2 files changed, 128 insertions(+), 0 deletions(-)

diff --git a/src/mesa/main/format_unpack.c b/src/mesa/main/format_unpack.c
index eaa33df..525bbcb 100644
--- a/src/mesa/main/format_unpack.c
+++ b/src/mesa/main/format_unpack.c
@@ -1258,7 +1258,127 @@ _mesa_unpack_rgba_row(gl_format format, GLuint n,
}
}

+static void
+unpack_int_rgba_RGBA_UINT32(const GLuint *src, GLuint dst[][4], GLuint n)
+{
+ memcpy(dst, src, n * 4 * sizeof(GLuint));
+}
+
+static void
+unpack_int_rgba_RGB_UINT32(const GLuint *src, GLuint dst[][4], GLuint n)
+{
+ unsigned int i;
+
+ for (i = 0; i < n; i++) {
+ dst[i][0] = src[i * 3 + 0];
+ dst[i][1] = src[i * 3 + 1];
+ dst[i][2] = src[i * 3 + 2];
+ dst[i][3] = 1;
+ }
+}
+
+static void
+unpack_int_rgba_RG_UINT32(const GLuint *src, GLuint dst[][4], GLuint n)
+{
+ unsigned int i;
+
+ for (i = 0; i < n; i++) {
+ dst[i][0] = src[i * 2 + 0];
+ dst[i][1] = src[i * 2 + 1];
+ dst[i][2] = 0;
+ dst[i][3] = 1;
+ }
+}
+
+static void
+unpack_int_rgba_R_UINT32(const GLuint *src, GLuint dst[][4], GLuint n)
+{
+ unsigned int i;
+
+ for (i = 0; i < n; i++) {
+ dst[i][0] = src[i];
+ dst[i][1] = 0;
+ dst[i][2] = 0;
+ dst[i][3] = 1;
+ }
+}
+
+static void
+unpack_int_rgba_LUMINANCE_UINT32(const GLuint *src, GLuint dst[][4], GLuint n)
+{
+ unsigned int i;
+
+ for (i = 0; i < n; i++) {
+ dst[i][0] = dst[i][1] = dst[i][2] = src[i];
+ dst[i][3] = 1;
+ }
+}

+static void
+unpack_int_rgba_LUMINANCE_ALPHA_UINT32(const GLuint *src, GLuint dst[][4], GLuint n)
+{
+ unsigned int i;
+
+ for (i = 0; i < n; i++) {
+ dst[i][0] = dst[i][1] = dst[i][2] = src[i * 2 + 0];
+ dst[i][3] = src[i * 2 + 1];
+ }
+}
+
+static void
+unpack_int_rgba_INTENSITY_UINT32(const GLuint *src, GLuint dst[][4], GLuint n)
+{
+ unsigned int i;
+
+ for (i = 0; i < n; i++) {
+ dst[i][0] = dst[i][1] = dst[i][2] = dst[i][3] = src[i];
+ }
+}
+
+void
+_mesa_unpack_int_rgba_row(gl_format format, GLuint n,
+ const void *src, GLuint dst[][4])
+{
+ switch (format) {
+ /* Since there won't be any sign extension happening, there's no need to
+ * make separate paths for 32-bit-to-32-bit integer unpack.
+ */
+ case MESA_FORMAT_RGBA_UINT32:
+ case MESA_FORMAT_RGBA_INT32:
+ unpack_int_rgba_RGBA_UINT32(src, dst, n);
+ break;
+ case MESA_FORMAT_RGB_UINT32:
+ case MESA_FORMAT_RGB_INT32:
+ unpack_int_rgba_RGB_UINT32(src, dst, n);
+ break;
+ case MESA_FORMAT_RG_UINT32:
+ case MESA_FORMAT_RG_INT32:
+ unpack_int_rgba_RG_UINT32(src, dst, n);
+ break;
+ case MESA_FORMAT_R_UINT32:
+ case MESA_FORMAT_R_INT32:
+ unpack_int_rgba_R_UINT32(src, dst, n);
+ break;
+
+ case MESA_FORMAT_LUMINANCE_UINT32:
+ case MESA_FORMAT_LUMINANCE_INT32:
+ unpack_int_rgba_LUMINANCE_UINT32(src, dst, n);
+ break;
+ case MESA_FORMAT_LUMINANCE_ALPHA_UINT32:
+ case MESA_FORMAT_LUMINANCE_ALPHA_INT32:
+ unpack_int_rgba_LUMINANCE_ALPHA_UINT32(src, dst, n);
+ break;
+ case MESA_FORMAT_INTENSITY_UINT32:
+ case MESA_FORMAT_INTENSITY_INT32:
+ unpack_int_rgba_INTENSITY_UINT32(src, dst, n);
+ break;
+
+ default:
+ _mesa_problem(NULL, "%s: bad format %s", __FUNCTION__,
+ _mesa_get_format_name(format));
+ return;
+ }
+}

/**
* Unpack a 2D rect of pixels returning float RGBA colors.
diff --git a/src/mesa/main/format_unpack.h b/src/mesa/main/format_unpack.h
index a8a829c..0d13a2d 100644
--- a/src/mesa/main/format_unpack.h
+++ b/src/mesa/main/format_unpack.h
@@ -29,12 +29,20 @@ _mesa_unpack_rgba_row(gl_format format, GLuint n,
const void *src, GLfloat dst[][4]);


+void
+_mesa_unpack_int_rgba_row(gl_format format, GLuint n,
+ const void *src, GLuint dst[][4]);
+
extern void
_mesa_unpack_rgba_block(gl_format format,
const void *src, GLint srcRowStride,
GLfloat dst[][4], GLint dstRowStride,
GLuint x, GLuint y, GLuint width, GLuint height);

+extern void
+_mesa_unpack_uint_rgba_row(gl_format format, GLuint n,
+ const void *src, GLuint dst[][4]);
+

extern void
_mesa_unpack_float_z_row(gl_format format, GLuint n,
--
1.7.7
Eric Anholt
2011-11-04 22:01:28 UTC
Permalink
We're missing support for the software paths still, but basic
rendering is working.
---
src/mesa/drivers/dri/intel/intel_span.c | 7 ++++++-
src/mesa/drivers/dri/intel/intel_tex_format.c | 6 ++++++
2 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/src/mesa/drivers/dri/intel/intel_span.c b/src/mesa/drivers/dri/intel/intel_span.c
index 604962d..e66b0f6 100644
--- a/src/mesa/drivers/dri/intel/intel_span.c
+++ b/src/mesa/drivers/dri/intel/intel_span.c
@@ -421,7 +421,12 @@ static span_init_func intel_span_init_funcs[MESA_FORMAT_COUNT] =
bool
intel_span_supports_format(gl_format format)
{
- return intel_span_init_funcs[format] != NULL;
+ /* Rendering to/from integer textures will be done using MapRenderbuffer,
+ * rather than coding up new paths through GetRow/PutRow(), so claim support
+ * for those formats in here for now.
+ */
+ return (intel_span_init_funcs[format] != NULL ||
+ _mesa_is_format_integer_color(format));
}

/**
diff --git a/src/mesa/drivers/dri/intel/intel_tex_format.c b/src/mesa/drivers/dri/intel/intel_tex_format.c
index 6890a69..caef5b2 100644
--- a/src/mesa/drivers/dri/intel/intel_tex_format.c
+++ b/src/mesa/drivers/dri/intel/intel_tex_format.c
@@ -9,6 +9,12 @@
GLenum
intel_mesa_format_to_rb_datatype(gl_format format)
{
+ /* These formats won't be going through the GetRow/PutRow() interfaces, so
+ * just return a type.
+ */
+ if (_mesa_is_format_integer_color(format))
+ return GL_UNSIGNED_INT;
+
switch (format) {
case MESA_FORMAT_ARGB8888:
case MESA_FORMAT_XRGB8888:
--
1.7.7
Kenneth Graunke
2011-11-09 03:33:07 UTC
Permalink
Post by Eric Anholt
We're missing support for the software paths still, but basic
rendering is working.
Yeah, we'll need to finish killing the software paths. Glad to see GPU
rendering working!

Comments below.
Post by Eric Anholt
---
src/mesa/drivers/dri/intel/intel_span.c | 7 ++++++-
src/mesa/drivers/dri/intel/intel_tex_format.c | 6 ++++++
2 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/src/mesa/drivers/dri/intel/intel_span.c b/src/mesa/drivers/dri/intel/intel_span.c
index 604962d..e66b0f6 100644
--- a/src/mesa/drivers/dri/intel/intel_span.c
+++ b/src/mesa/drivers/dri/intel/intel_span.c
@@ -421,7 +421,12 @@ static span_init_func intel_span_init_funcs[MESA_FORMAT_COUNT] =
bool
intel_span_supports_format(gl_format format)
{
- return intel_span_init_funcs[format] != NULL;
+ /* Rendering to/from integer textures will be done using MapRenderbuffer,
+ * rather than coding up new paths through GetRow/PutRow(), so claim support
+ * for those formats in here for now.
+ */
+ return (intel_span_init_funcs[format] != NULL ||
+ _mesa_is_format_integer_color(format));
}
/**
diff --git a/src/mesa/drivers/dri/intel/intel_tex_format.c b/src/mesa/drivers/dri/intel/intel_tex_format.c
index 6890a69..caef5b2 100644
--- a/src/mesa/drivers/dri/intel/intel_tex_format.c
+++ b/src/mesa/drivers/dri/intel/intel_tex_format.c
@@ -9,6 +9,12 @@
GLenum
intel_mesa_format_to_rb_datatype(gl_format format)
{
+ /* These formats won't be going through the GetRow/PutRow() interfaces, so
+ * just return a type.
+ */
+ if (_mesa_is_format_integer_color(format))
+ return GL_UNSIGNED_INT;
+
switch (format) {
The key piece of missing knowledge is that intel_renderbuffer::DataType
is only used for spans rendering. Assuming that's really the case, this
patch is:

Reviewed-by: Kenneth Graunke <***@whitecape.org>
Eric Anholt
2011-11-04 22:01:31 UTC
Permalink
With this change, i965 passes
GL_EXT_texture_integer/fbo_integer_precision_clear
---
src/mesa/swrast/s_readpix.c | 19 +++++++++++++------
1 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/src/mesa/swrast/s_readpix.c b/src/mesa/swrast/s_readpix.c
index 50422db..54f42db 100644
--- a/src/mesa/swrast/s_readpix.c
+++ b/src/mesa/swrast/s_readpix.c
@@ -236,7 +236,10 @@ slow_read_rgba_pixels( struct gl_context *ctx,
GLbitfield transferOps )
{
struct gl_renderbuffer *rb = ctx->ReadBuffer->_ColorReadBuffer;
- GLfloat rgba[MAX_WIDTH][4];
+ union {
+ float f[MAX_WIDTH][4];
+ unsigned int i[MAX_WIDTH][4];
+ } rgba;
GLubyte *dst, *map;
int dstStride, stride, j;

@@ -248,11 +251,15 @@ slow_read_rgba_pixels( struct gl_context *ctx,
&map, &stride);

for (j = 0; j < height; j++) {
- _mesa_unpack_rgba_row(_mesa_get_srgb_format_linear(rb->Format),
- width, map, rgba);
- _mesa_pack_rgba_span_float(ctx, width, rgba, format, type, dst,
- packing, transferOps);
-
+ if (_mesa_is_integer_format(format)) {
+ _mesa_unpack_int_rgba_row(rb->Format, width, map, rgba.i);
+ _mesa_pack_rgba_span_int(ctx, width, rgba.i, format, type, dst);
+ } else {
+ _mesa_unpack_rgba_row(_mesa_get_srgb_format_linear(rb->Format),
+ width, map, rgba.f);
+ _mesa_pack_rgba_span_float(ctx, width, rgba.f, format, type, dst,
+ packing, transferOps);
+ }
dst += dstStride;
map += stride;
}
--
1.7.7
Kenneth Graunke
2011-11-09 03:54:57 UTC
Permalink
Post by Eric Anholt
Here's a patch series to get GL_EXT_texture_integer partially working
on i965, hidden under the GL 3.0 override. There's a bunch of support
I think isn't finished yet, mostly because testing is in a bad state.
For the series:
Reviewed-by: Kenneth Graunke <***@whitecape.org>

I replied with suggestions to a few of the patches, which I hope are
helpful, but I wouldn't object to the series going in as-is either.
Eric Anholt
2011-11-09 19:49:18 UTC
Permalink
Post by Kenneth Graunke
Post by Eric Anholt
Here's a patch series to get GL_EXT_texture_integer partially working
on i965, hidden under the GL 3.0 override. There's a bunch of support
I think isn't finished yet, mostly because testing is in a bad state.
I replied with suggestions to a few of the patches, which I hope are
helpful, but I wouldn't object to the series going in as-is either.
Thanks! I liked the split of the user-defined outs, so I pulled that
in.
Continue reading on narkive:
Loading...