From 5e6d21afa4efe033e27b980cc26266c3db6d9021 Mon Sep 17 00:00:00 2001 From: Brian Paul Date: Thu, 20 Aug 2009 10:25:42 -0600 Subject: [PATCH 01/12] tgsi: added tgsi_full_instruction::Flags field Users of the parser can make use of this. --- src/gallium/auxiliary/tgsi/tgsi_build.c | 2 ++ src/gallium/auxiliary/tgsi/tgsi_parse.h | 1 + 2 files changed, 3 insertions(+) diff --git a/src/gallium/auxiliary/tgsi/tgsi_build.c b/src/gallium/auxiliary/tgsi/tgsi_build.c index d272533d63c..94d9c638e1c 100644 --- a/src/gallium/auxiliary/tgsi/tgsi_build.c +++ b/src/gallium/auxiliary/tgsi/tgsi_build.c @@ -477,6 +477,8 @@ tgsi_default_full_instruction( void ) full_instruction.FullSrcRegisters[i] = tgsi_default_full_src_register(); } + full_instruction.Flags = 0x0; + return full_instruction; } diff --git a/src/gallium/auxiliary/tgsi/tgsi_parse.h b/src/gallium/auxiliary/tgsi/tgsi_parse.h index a289e26e3ac..5268c029006 100644 --- a/src/gallium/auxiliary/tgsi/tgsi_parse.h +++ b/src/gallium/auxiliary/tgsi/tgsi_parse.h @@ -91,6 +91,7 @@ struct tgsi_full_instruction struct tgsi_instruction_ext_texture InstructionExtTexture; struct tgsi_full_dst_register FullDstRegisters[TGSI_FULL_MAX_DST_REGISTERS]; struct tgsi_full_src_register FullSrcRegisters[TGSI_FULL_MAX_SRC_REGISTERS]; + uint Flags; /**< user-defined usage */ }; union tgsi_full_token From 4c7c294ffff2a66f4585faa41ea9810527ea1e92 Mon Sep 17 00:00:00 2001 From: Brian Paul Date: Thu, 20 Aug 2009 10:28:22 -0600 Subject: [PATCH 02/12] tgsi: handle SOA dependencies for MOV/SWZ SOA dependencies can happen when a register is used both as a source and destination and the source is swizzled. For example: MOV T, T.yxwz; would expand into: MOV t0, t1; MOV t1, t0; MOV t2, t3; MOV t3, t2; The second instruction will produce the wrong result since we wrote to t0 in the first instruction. We need to use an intermediate temporary to fix this. This will take more work to fix for all TGSI instructions. This seems to happen with MOV instructions more than anything else so fix that case now and warn on others. Fixes piglit glsl-vs-loop test (when not using SSE). See bug 23317. --- src/gallium/auxiliary/tgsi/tgsi_exec.c | 50 ++++++++++++++++++-------- src/gallium/auxiliary/tgsi/tgsi_exec.h | 4 +++ 2 files changed, 40 insertions(+), 14 deletions(-) diff --git a/src/gallium/auxiliary/tgsi/tgsi_exec.c b/src/gallium/auxiliary/tgsi/tgsi_exec.c index 5cb322a5fa2..259894e72e0 100644 --- a/src/gallium/auxiliary/tgsi/tgsi_exec.c +++ b/src/gallium/auxiliary/tgsi/tgsi_exec.c @@ -62,6 +62,9 @@ #define FAST_MATH 1 +/** for tgsi_full_instruction::Flags */ +#define SOA_DEPENDENCY_FLAG 0x1 + #define TILE_TOP_LEFT 0 #define TILE_TOP_RIGHT 1 #define TILE_BOTTOM_LEFT 2 @@ -182,7 +185,7 @@ print_temp(const struct tgsi_exec_machine *mach, uint index) * MOV t3, t2; * The second instruction will have the wrong value for t0 if executed as-is. */ -static boolean +boolean tgsi_check_soa_dependencies(const struct tgsi_full_instruction *inst) { uint i, chan; @@ -328,20 +331,25 @@ tgsi_exec_machine_bind_shader( * sizeof(struct tgsi_full_instruction)); maxInstructions += 10; } + + if (tgsi_check_soa_dependencies(&parse.FullToken.FullInstruction)) { + uint opcode = parse.FullToken.FullInstruction.Instruction.Opcode; + parse.FullToken.FullInstruction.Flags = SOA_DEPENDENCY_FLAG; + /* XXX we only handle SOA dependencies properly for MOV/SWZ + * at this time! + */ + if (opcode != TGSI_OPCODE_MOV && opcode != TGSI_OPCODE_SWZ) { + debug_printf("Warning: SOA dependency in instruction" + " is not handled:\n"); + tgsi_dump_instruction(&parse.FullToken.FullInstruction, + numInstructions); + } + } + memcpy(instructions + numInstructions, &parse.FullToken.FullInstruction, sizeof(instructions[0])); -#if 0 - if (tgsi_check_soa_dependencies(&parse.FullToken.FullInstruction)) { - debug_printf("SOA dependency in instruction:\n"); - tgsi_dump_instruction(&parse.FullToken.FullInstruction, - numInstructions); - } -#else - (void) tgsi_check_soa_dependencies; -#endif - numInstructions++; break; @@ -2018,9 +2026,23 @@ exec_instruction( case TGSI_OPCODE_MOV: case TGSI_OPCODE_SWZ: - FOR_EACH_ENABLED_CHANNEL( *inst, chan_index ) { - FETCH( &r[0], 0, chan_index ); - STORE( &r[0], 0, chan_index ); + if (inst->Flags & SOA_DEPENDENCY_FLAG) { + /* Do all fetches into temp regs, then do all stores to avoid + * intermediate/accidental clobbering. This could be done all the + * time for MOV but for other instructions we'll need more temps... + */ + FOR_EACH_ENABLED_CHANNEL( *inst, chan_index ) { + FETCH( &r[chan_index], 0, chan_index ); + } + FOR_EACH_ENABLED_CHANNEL( *inst, chan_index ) { + STORE( &r[chan_index], 0, chan_index ); + } + } + else { + FOR_EACH_ENABLED_CHANNEL( *inst, chan_index ) { + FETCH( &r[0], 0, chan_index ); + STORE( &r[0], 0, chan_index ); + } } break; diff --git a/src/gallium/auxiliary/tgsi/tgsi_exec.h b/src/gallium/auxiliary/tgsi/tgsi_exec.h index da22baad3ef..182e5e228b5 100644 --- a/src/gallium/auxiliary/tgsi/tgsi_exec.h +++ b/src/gallium/auxiliary/tgsi/tgsi_exec.h @@ -272,6 +272,10 @@ void tgsi_exec_machine_free_data(struct tgsi_exec_machine *mach); +boolean +tgsi_check_soa_dependencies(const struct tgsi_full_instruction *inst); + + static INLINE void tgsi_set_kill_mask(struct tgsi_exec_machine *mach, unsigned mask) { From ce723d8d8b011f2efaea6588c42b6d11ee2e7115 Mon Sep 17 00:00:00 2001 From: Brian Paul Date: Thu, 20 Aug 2009 10:34:45 -0600 Subject: [PATCH 03/12] tgsi: check for SOA dependencies in SSE and PPC code generators Fall back to interpreter for now. This doesn't happen very often. --- src/gallium/auxiliary/tgsi/tgsi_ppc.c | 4 ++++ src/gallium/auxiliary/tgsi/tgsi_sse2.c | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/src/gallium/auxiliary/tgsi/tgsi_ppc.c b/src/gallium/auxiliary/tgsi/tgsi_ppc.c index 2f8b0c4df0f..8466d9bc229 100644 --- a/src/gallium/auxiliary/tgsi/tgsi_ppc.c +++ b/src/gallium/auxiliary/tgsi/tgsi_ppc.c @@ -1112,6 +1112,10 @@ emit_instruction(struct gen_context *gen, if (inst->Instruction.Saturate != TGSI_SAT_NONE) return 0; + /* need to use extra temps to fix SOA dependencies : */ + if (tgsi_check_soa_dependencies(inst)) + return FALSE; + switch (inst->Instruction.Opcode) { case TGSI_OPCODE_MOV: case TGSI_OPCODE_SWZ: diff --git a/src/gallium/auxiliary/tgsi/tgsi_sse2.c b/src/gallium/auxiliary/tgsi/tgsi_sse2.c index 571f98ae35e..a13368c80c8 100644 --- a/src/gallium/auxiliary/tgsi/tgsi_sse2.c +++ b/src/gallium/auxiliary/tgsi/tgsi_sse2.c @@ -1506,6 +1506,10 @@ emit_instruction( if (inst->Instruction.Saturate != TGSI_SAT_NONE) return FALSE; + /* need to use extra temps to fix SOA dependencies : */ + if (tgsi_check_soa_dependencies(inst)) + return FALSE; + switch (inst->Instruction.Opcode) { case TGSI_OPCODE_ARL: FOR_EACH_DST0_ENABLED_CHANNEL( *inst, chan_index ) { From 1aba1baa622116759bfedca87f37e527c0111d16 Mon Sep 17 00:00:00 2001 From: Brian Paul Date: Fri, 21 Aug 2009 10:24:50 -0600 Subject: [PATCH 04/12] st/mesa: flush bitmap cache if Z value changes When adding a new bitmap to the cache we have to check if the Z value is changing and flush first if it is. This is a modified version of a patch from Justin Dou --- src/mesa/state_tracker/st_cb_bitmap.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/mesa/state_tracker/st_cb_bitmap.c b/src/mesa/state_tracker/st_cb_bitmap.c index 8709633557c..ccf972f8055 100644 --- a/src/mesa/state_tracker/st_cb_bitmap.c +++ b/src/mesa/state_tracker/st_cb_bitmap.c @@ -94,6 +94,9 @@ struct bitmap_cache GLfloat color[4]; + /** Bitmap's Z position */ + GLfloat zpos; + struct pipe_texture *texture; struct pipe_transfer *trans; @@ -104,6 +107,8 @@ struct bitmap_cache }; +/** Epsilon for Z comparisons */ +#define Z_EPSILON 1e-06 /** @@ -538,9 +543,7 @@ draw_bitmap_quad(GLcontext *ctx, GLint x, GLint y, GLfloat z, } /* draw textured quad */ - offset = setup_bitmap_vertex_data(st, x, y, width, height, - ctx->Current.RasterPos[2], - color); + offset = setup_bitmap_vertex_data(st, x, y, width, height, z, color); util_draw_vertex_buffer(pipe, st->bitmap.vbuf, offset, PIPE_PRIM_TRIANGLE_FAN, @@ -647,7 +650,7 @@ st_flush_bitmap_cache(struct st_context *st) draw_bitmap_quad(st->ctx, cache->xpos, cache->ypos, - st->ctx->Current.RasterPos[2], + cache->zpos, BITMAP_CACHE_WIDTH, BITMAP_CACHE_HEIGHT, cache->texture, cache->color); @@ -687,6 +690,7 @@ accum_bitmap(struct st_context *st, { struct bitmap_cache *cache = st->bitmap.cache; int px = -999, py; + const GLfloat z = st->ctx->Current.RasterPos[2]; if (width > BITMAP_CACHE_WIDTH || height > BITMAP_CACHE_HEIGHT) @@ -697,7 +701,8 @@ accum_bitmap(struct st_context *st, py = y - cache->ypos; if (px < 0 || px + width > BITMAP_CACHE_WIDTH || py < 0 || py + height > BITMAP_CACHE_HEIGHT || - !TEST_EQ_4V(st->ctx->Current.RasterColor, cache->color)) { + !TEST_EQ_4V(st->ctx->Current.RasterColor, cache->color) || + ((fabs(z - cache->zpos) > Z_EPSILON))) { /* This bitmap would extend beyond cache bounds, or the bitmap * color is changing * so flush and continue. @@ -712,6 +717,7 @@ accum_bitmap(struct st_context *st, py = (BITMAP_CACHE_HEIGHT - height) / 2; cache->xpos = x; cache->ypos = y - py; + cache->zpos = z; cache->empty = GL_FALSE; COPY_4FV(cache->color, st->ctx->Current.RasterColor); } From f785b35b47926e052571386227eff58db7c084e2 Mon Sep 17 00:00:00 2001 From: Vinson Lee Date: Mon, 24 Aug 2009 11:43:02 -0600 Subject: [PATCH 05/12] glsl: Silence gcc uninitialized variable warning. --- src/mesa/shader/slang/slang_builtin.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mesa/shader/slang/slang_builtin.c b/src/mesa/shader/slang/slang_builtin.c index 289d94644f0..4e67241f3b2 100644 --- a/src/mesa/shader/slang/slang_builtin.c +++ b/src/mesa/shader/slang/slang_builtin.c @@ -436,7 +436,7 @@ emit_statevars(const char *name, int array_len, struct gl_program_parameter_list *paramList) { if (type->type == SLANG_SPEC_ARRAY) { - GLint i, pos; + GLint i, pos = -1; assert(array_len > 0); if (strcmp(name, "gl_ClipPlane") == 0) { tokens[0] = STATE_CLIPPLANE; From b9b04872d526ed7955f647542399e110ace0325c Mon Sep 17 00:00:00 2001 From: Brian Paul Date: Mon, 24 Aug 2009 12:43:57 -0600 Subject: [PATCH 06/12] vbo: fix divide by zero exception Fixes bug 23489. --- src/mesa/vbo/vbo_exec_draw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mesa/vbo/vbo_exec_draw.c b/src/mesa/vbo/vbo_exec_draw.c index c53a4eee00c..98177a41750 100644 --- a/src/mesa/vbo/vbo_exec_draw.c +++ b/src/mesa/vbo/vbo_exec_draw.c @@ -383,7 +383,7 @@ void vbo_exec_vtx_flush( struct vbo_exec_context *exec, } - if (unmap) + if (unmap || exec->vtx.vertex_size == 0) exec->vtx.max_vert = 0; else exec->vtx.max_vert = ((VBO_VERT_BUFFER_SIZE - exec->vtx.buffer_used) / From b5ecbbe636dd0d2094921c5401e4268694c405ce Mon Sep 17 00:00:00 2001 From: Brian Paul Date: Mon, 24 Aug 2009 12:58:47 -0600 Subject: [PATCH 07/12] xlib: fix single buffer window resize bug When a single-buffered window was resized the new window size was never detected. This fix that, but there's still a bug which causes window contents corruption for certain window sizes... --- src/gallium/winsys/xlib/xlib_softpipe.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/gallium/winsys/xlib/xlib_softpipe.c b/src/gallium/winsys/xlib/xlib_softpipe.c index 44b8464518a..2a08b829085 100644 --- a/src/gallium/winsys/xlib/xlib_softpipe.c +++ b/src/gallium/winsys/xlib/xlib_softpipe.c @@ -303,6 +303,7 @@ xm_flush_frontbuffer(struct pipe_winsys *pws, */ XMesaContext xmctx = (XMesaContext) context_private; xlib_softpipe_display_surface(xmctx->xm_buffer, surf); + xmesa_check_and_update_buffer_size(xmctx, xmctx->xm_buffer); } From 96f7b422426f8f13f4cedbe280e5dede6b358f2a Mon Sep 17 00:00:00 2001 From: Brian Paul Date: Mon, 24 Aug 2009 13:02:33 -0600 Subject: [PATCH 08/12] docs: recent 7.5.1 bug fixes --- docs/relnotes-7.5.1.html | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/relnotes-7.5.1.html b/docs/relnotes-7.5.1.html index 1da086de3b5..77f2dd29b35 100644 --- a/docs/relnotes-7.5.1.html +++ b/docs/relnotes-7.5.1.html @@ -50,6 +50,8 @@ tbd
  • Fixed minor GLX memory leaks.
  • Fixed some texture env / fragment program state bugs.
  • Fixed some Gallium glBlitFramebuffer() bugs +
  • Empty glBegin/glEnd() pair could cause divide by zero (bug 23489) +
  • Fixed Gallium glBitmap() Z position bug From bf7e4b10cbed496a12c8be17531c9cb7da1be177 Mon Sep 17 00:00:00 2001 From: Brian Paul Date: Mon, 24 Aug 2009 13:56:01 -0600 Subject: [PATCH 09/12] ARB prog: Set error instead of falling through with incorrect value If a fragment program only parameter was queried of a vertex program (e.g., GL_MAX_PROGRAM_TEX_INDIRECTIONS_ARB) no error would be set and a random value would be returned. This caused 'glxinfo -l' to show the same values for GL_MAX_PROGRAM_ALU_INSTRUCTIONS_ARB, GL_MAX_PROGRAM_TEX_INSTRUCTIONS_ARB, GL_MAX_PROGRAM_TEX_INDIRECTIONS_ARB, GL_MAX_PROGRAM_NATIVE_ALU_INSTRUCTIONS_ARB, GL_MAX_PROGRAM_NATIVE_TEX_INSTRUCTIONS_ARB, GL_MAX_PROGRAM_NATIVE_TEX_INDIRECTIONS_ARB as for GL_MAX_PROGRAM_ENV_PARAMETERS_ARB. This is confusing and incorrect. (cherry picked from master, commit 4bccd693a72a0b42dffc849034263a68e779ca91) --- src/mesa/shader/arbprogram.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/mesa/shader/arbprogram.c b/src/mesa/shader/arbprogram.c index 981565ab8f1..a0331054bb4 100644 --- a/src/mesa/shader/arbprogram.c +++ b/src/mesa/shader/arbprogram.c @@ -985,6 +985,9 @@ _mesa_GetProgramivARB(GLenum target, GLenum pname, GLint *params) _mesa_error(ctx, GL_INVALID_ENUM, "glGetProgramivARB(pname)"); return; } + } else { + _mesa_error(ctx, GL_INVALID_ENUM, "glGetProgramivARB(pname)"); + return; } } From 04d170794a22d93d58afeb5d0930e06f85964f9a Mon Sep 17 00:00:00 2001 From: Brian Paul Date: Wed, 26 Aug 2009 11:39:24 -0600 Subject: [PATCH 10/12] glsl: fix bug in sampler array indexing Need to add the 'offset' parameter when indexing the parameter array. Before, if we were setting arrays of samplers, we were actually only setting the 0th sampler's value. Because of how progs/glsl/samplers.c is constructed, this wasn't showing up as a failure in the samplers_array output. --- src/mesa/shader/shader_api.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/mesa/shader/shader_api.c b/src/mesa/shader/shader_api.c index 14da974a873..95b7490ca97 100644 --- a/src/mesa/shader/shader_api.c +++ b/src/mesa/shader/shader_api.c @@ -1624,7 +1624,6 @@ set_program_uniform(GLcontext *ctx, struct gl_program *program, if (param->Type == PROGRAM_SAMPLER) { /* This controls which texture unit which is used by a sampler */ - GLuint texUnit, sampler; GLint i; /* data type for setting samplers must be int */ @@ -1639,8 +1638,9 @@ set_program_uniform(GLcontext *ctx, struct gl_program *program, * common thing... */ for (i = 0; i < count; i++) { - sampler = (GLuint) program->Parameters->ParameterValues[index + i][0]; - texUnit = ((GLuint *) values)[i]; + GLuint sampler = + (GLuint) program->Parameters->ParameterValues[index + offset + i][0]; + GLuint texUnit = ((GLuint *) values)[i]; /* check that the sampler (tex unit index) is legal */ if (texUnit >= ctx->Const.MaxTextureImageUnits) { @@ -1651,6 +1651,10 @@ set_program_uniform(GLcontext *ctx, struct gl_program *program, /* This maps a sampler to a texture unit: */ if (sampler < MAX_SAMPLERS) { +#if 0 + _mesa_printf("Set program %p sampler %d '%s' to unit %u\n", + program, sampler, param->Name, texUnit); +#endif program->SamplerUnits[sampler] = texUnit; } } From f6d34c20585ae9b4fb07ec2f2850f04dc9a9bc29 Mon Sep 17 00:00:00 2001 From: Brian Paul Date: Wed, 26 Aug 2009 11:53:25 -0600 Subject: [PATCH 11/12] progs/glsl: change samplers.c to better test sampler/texture indexing Now the left half is yellow and the right half is red, with the gradients going in opposite directions. --- progs/glsl/samplers.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/progs/glsl/samplers.c b/progs/glsl/samplers.c index 113e5bbeff1..87dad5d8575 100644 --- a/progs/glsl/samplers.c +++ b/progs/glsl/samplers.c @@ -211,10 +211,18 @@ InitTextures(void) for (y = 0; y < stripeSize; y++) { for (x = 0; x < size; x++) { GLint k = 4 * ((ypos + y) * size + x); - texImage[k + 0] = intensity; - texImage[k + 1] = intensity; - texImage[k + 2] = 0; - texImage[k + 3] = 255; + if (x < size / 2) { + texImage[k + 0] = intensity; + texImage[k + 1] = intensity; + texImage[k + 2] = 0; + texImage[k + 3] = 255; + } + else { + texImage[k + 0] = 255 - intensity; + texImage[k + 1] = 0; + texImage[k + 2] = 0; + texImage[k + 3] = 255; + } } } From 488b3c4d1bc3d830477180759a42dbaf8f5801b0 Mon Sep 17 00:00:00 2001 From: Brian Paul Date: Wed, 26 Aug 2009 11:55:15 -0600 Subject: [PATCH 12/12] progs/glsl: add special Makefile rule for samplers_array --- progs/glsl/Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/progs/glsl/Makefile b/progs/glsl/Makefile index eedd866c957..6af7a665cf1 100644 --- a/progs/glsl/Makefile +++ b/progs/glsl/Makefile @@ -32,6 +32,7 @@ DEMO_SOURCES = \ pointcoord.c \ points.c \ samplers.c \ + samplers_array.c \ shadow_sampler.c \ skinning.c \ texaaline.c \ @@ -188,7 +189,8 @@ samplers.o: $(UTIL_HEADERS) samplers: samplers.o $(UTIL_OBJS) -samplers_array.o: $(UTIL_HEADERS) +samplers_array.o: samplers.c $(UTIL_HEADERS) + $(APP_CC) $(CFLAGS) -DSAMPLERS_ARRAY $< -c -o $@ samplers_array: samplers_array.o $(UTIL_OBJS)