From patchwork Sat Jun 30 18:50:11 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Olivier Galibert X-Patchwork-Id: 1134751 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by patchwork1.kernel.org (Postfix) with ESMTP id 8BAE13FE77 for ; Sat, 30 Jun 2012 19:04:52 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 6CC379EB11 for ; Sat, 30 Jun 2012 12:04:52 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from sasl.smtp.pobox.com (a-pb-sasl-sd.pobox.com [74.115.168.62]) by gabe.freedesktop.org (Postfix) with ESMTP id C5C799E8D8 for ; Sat, 30 Jun 2012 12:00:00 -0700 (PDT) Received: from sasl.smtp.pobox.com (unknown [127.0.0.1]) by b-pb-sasl-sd.pobox.com (Postfix) with ESMTP id A90AAB0E7; Sat, 30 Jun 2012 14:50:25 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=pobox.com; h=from:to:cc :subject:date:message-id:in-reply-to:references; s=sasl; bh=grHk Naro3HJ9FSbRuPfpkAbqsN8=; b=nC2+crLhJEB8yLxWUuHSeT0CsCte2fvjWNgC 7sf6jegb4K7Jq30kfVAFhGTr8KxJGYfMIAy8imias1VuJICpeZOLcJCc8D6dFpwP mlc++QcxdV22QxkGyBtHedmoiREK5vYE0UJptLFZQS3LtbwefazeuvJQpFKRoLpg VPimlzA= DomainKey-Signature: a=rsa-sha1; c=nofws; d=pobox.com; h=from:to:cc :subject:date:message-id:in-reply-to:references; q=dns; s=sasl; b= KcvfYY6XQrBXcxTL+q0ZjaOk+1QCCpduEiEfRSe8I8+i0Aw2WH1s4wNI+4Rl2Xxr n5iWyugGKLb9vQbRpV6PZc7x5tvx3A1FlhA8Q4SVeBgKJY+xj0fUJtl96+gaSZEk vQwNGv7VVWl3xCFxUyPnyF9yg2nbYvdNFJL01DcYXRc= Received: from b-pb-sasl-sd. (unknown [127.0.0.1]) by b-pb-sasl-sd.pobox.com (Postfix) with ESMTP id 9FD53B0E6; Sat, 30 Jun 2012 14:50:25 -0400 (EDT) Received: from kervella.org (unknown [82.234.121.82]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by b-pb-sasl-sd.pobox.com (Postfix) with ESMTPSA id 804BAB0E4; Sat, 30 Jun 2012 14:50:23 -0400 (EDT) From: Olivier Galibert To: intel-gfx@lists.freedesktop.org, mesa-dev@lists.freedesktop.org Date: Sat, 30 Jun 2012 20:50:11 +0200 Message-Id: <1341082215-22933-2-git-send-email-galibert@pobox.com> X-Mailer: git-send-email 1.7.10.rc3.1.gb306 In-Reply-To: <1341082215-22933-1-git-send-email-galibert@pobox.com> References: <1341082215-22933-1-git-send-email-galibert@pobox.com> X-Pobox-Relay-ID: 73D22ECE-C2E4-11E1-893E-FA6787E41631-92059326!b-pb-sasl-sd.pobox.com Cc: Olivier Galibert Subject: [Intel-gfx] [PATCH 1/5] intel gen4/5: fix GL_VERTEX_PROGRAM_TWO_SIDE. X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org There was... confusion about which register goes where. With that patch urb_setup is in line with the vue setup, even when these annoying backcolor slots are used. And in addition the stray mov into lalaland is avoided when only one of the front/back slots is used and the backface is looking at you. The code instead picks whatever slot was written to by the vertex shader. That makes most of the generated piglit tests useless to test the backface selection though. Signed-off-by: Olivier Galibert --- src/mesa/drivers/dri/i965/brw_fs.cpp | 18 +++++- src/mesa/drivers/dri/i965/brw_sf.c | 3 +- src/mesa/drivers/dri/i965/brw_sf_emit.c | 93 +++++++++++++++++------------- src/mesa/drivers/dri/i965/brw_wm_pass2.c | 19 +++++- 4 files changed, 89 insertions(+), 44 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp index 6cef08a..710f2ff 100644 --- a/src/mesa/drivers/dri/i965/brw_fs.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp @@ -721,8 +721,24 @@ fs_visitor::calculate_urb_setup() if (c->key.vp_outputs_written & BITFIELD64_BIT(i)) { int fp_index = _mesa_vert_result_to_frag_attrib((gl_vert_result) i); + /* Special case: two-sided vertex option, vertex program + * only writes to the back color. Map it to the + * associated front color location. + */ + if (i >= VERT_RESULT_BFC0 && i <= VERT_RESULT_BFC1 && + ctx->VertexProgram._TwoSideEnabled && + urb_setup[i - VERT_RESULT_BFC0 + FRAG_ATTRIB_COL0] == -1) + fp_index = i - VERT_RESULT_BFC0 + FRAG_ATTRIB_COL0; + + /* The back color slot is skipped when the front color is + * also written to. In addition, some slots can be + * written in the vertex shader and not read in the + * fragment shader. So the register number must always be + * incremented, mapped or not. + */ if (fp_index >= 0) - urb_setup[fp_index] = urb_next++; + urb_setup[fp_index] = urb_next; + urb_next++; } } diff --git a/src/mesa/drivers/dri/i965/brw_sf.c b/src/mesa/drivers/dri/i965/brw_sf.c index 23a874a..7867ab5 100644 --- a/src/mesa/drivers/dri/i965/brw_sf.c +++ b/src/mesa/drivers/dri/i965/brw_sf.c @@ -192,7 +192,8 @@ brw_upload_sf_prog(struct brw_context *brw) /* _NEW_LIGHT */ key.do_flat_shading = (ctx->Light.ShadeModel == GL_FLAT); - key.do_twoside_color = (ctx->Light.Enabled && ctx->Light.Model.TwoSide); + key.do_twoside_color = (ctx->Light.Enabled && ctx->Light.Model.TwoSide) || + ctx->VertexProgram._TwoSideEnabled; /* _NEW_POLYGON */ if (key.do_twoside_color) { diff --git a/src/mesa/drivers/dri/i965/brw_sf_emit.c b/src/mesa/drivers/dri/i965/brw_sf_emit.c index ff6383b..9d8aa38 100644 --- a/src/mesa/drivers/dri/i965/brw_sf_emit.c +++ b/src/mesa/drivers/dri/i965/brw_sf_emit.c @@ -79,24 +79,9 @@ have_attr(struct brw_sf_compile *c, GLuint attr) /*********************************************************************** * Twoside lighting */ -static void copy_bfc( struct brw_sf_compile *c, - struct brw_reg vert ) -{ - struct brw_compile *p = &c->func; - GLuint i; - - for (i = 0; i < 2; i++) { - if (have_attr(c, VERT_RESULT_COL0+i) && - have_attr(c, VERT_RESULT_BFC0+i)) - brw_MOV(p, - get_vert_result(c, vert, VERT_RESULT_COL0+i), - get_vert_result(c, vert, VERT_RESULT_BFC0+i)); - } -} - - static void do_twoside_color( struct brw_sf_compile *c ) { + GLuint i, need_0, need_1; struct brw_compile *p = &c->func; GLuint backface_conditional = c->key.frontface_ccw ? BRW_CONDITIONAL_G : BRW_CONDITIONAL_L; @@ -105,12 +90,14 @@ static void do_twoside_color( struct brw_sf_compile *c ) if (c->key.primitive == SF_UNFILLED_TRIS) return; - /* XXX: What happens if BFC isn't present? This could only happen - * for user-supplied vertex programs, as t_vp_build.c always does - * the right thing. + /* If the vertex shader provides both front and backface color, do + * the selection. Otherwise the generated code will pick up + * whichever there is. */ - if (!(have_attr(c, VERT_RESULT_COL0) && have_attr(c, VERT_RESULT_BFC0)) && - !(have_attr(c, VERT_RESULT_COL1) && have_attr(c, VERT_RESULT_BFC1))) + need_0 = have_attr(c, VERT_RESULT_COL0) && have_attr(c, VERT_RESULT_BFC0); + need_1 = have_attr(c, VERT_RESULT_COL1) && have_attr(c, VERT_RESULT_BFC1); + + if (!need_0 && !need_1) return; /* Need to use BRW_EXECUTE_4 and also do an 4-wide compare in order @@ -121,12 +108,15 @@ static void do_twoside_color( struct brw_sf_compile *c ) brw_push_insn_state(p); brw_CMP(p, vec4(brw_null_reg()), backface_conditional, c->det, brw_imm_f(0)); brw_IF(p, BRW_EXECUTE_4); - { - switch (c->nr_verts) { - case 3: copy_bfc(c, c->vert[2]); - case 2: copy_bfc(c, c->vert[1]); - case 1: copy_bfc(c, c->vert[0]); - } + for (i=0; inr_verts; i++) { + if (need_0) + brw_MOV(p, + get_vert_result(c, c->vert[i], VERT_RESULT_COL0), + get_vert_result(c, c->vert[i], VERT_RESULT_BFC0)); + if (need_1) + brw_MOV(p, + get_vert_result(c, c->vert[i], VERT_RESULT_COL1), + get_vert_result(c, c->vert[i], VERT_RESULT_BFC1)); } brw_ENDIF(p); brw_pop_insn_state(p); @@ -139,20 +129,27 @@ static void do_twoside_color( struct brw_sf_compile *c ) */ #define VERT_RESULT_COLOR_BITS (BITFIELD64_BIT(VERT_RESULT_COL0) | \ - BITFIELD64_BIT(VERT_RESULT_COL1)) + BITFIELD64_BIT(VERT_RESULT_COL1)) static void copy_colors( struct brw_sf_compile *c, struct brw_reg dst, - struct brw_reg src) + struct brw_reg src, + int allow_twoside) { struct brw_compile *p = &c->func; GLuint i; for (i = VERT_RESULT_COL0; i <= VERT_RESULT_COL1; i++) { - if (have_attr(c,i)) + if (have_attr(c,i)) { brw_MOV(p, get_vert_result(c, dst, i), get_vert_result(c, src, i)); + + } else if(allow_twoside && have_attr(c, i - VERT_RESULT_COL0 + VERT_RESULT_BFC0)) { + brw_MOV(p, + get_vert_result(c, dst, i - VERT_RESULT_COL0 + VERT_RESULT_BFC0), + get_vert_result(c, src, i - VERT_RESULT_COL0 + VERT_RESULT_BFC0)); + } } } @@ -167,9 +164,19 @@ static void do_flatshade_triangle( struct brw_sf_compile *c ) struct brw_compile *p = &c->func; struct intel_context *intel = &p->brw->intel; struct brw_reg ip = brw_ip_reg(); - GLuint nr = _mesa_bitcount_64(c->key.attrs & VERT_RESULT_COLOR_BITS); GLuint jmpi = 1; + GLuint nr; + + if (c->key.do_twoside_color) { + nr = ((c->key.attrs & (BITFIELD64_BIT(VERT_RESULT_COL0) | BITFIELD64_BIT(VERT_RESULT_BFC0))) != 0) + + ((c->key.attrs & (BITFIELD64_BIT(VERT_RESULT_COL1) | BITFIELD64_BIT(VERT_RESULT_BFC1))) != 0); + + } else { + nr = ((c->key.attrs & BITFIELD64_BIT(VERT_RESULT_COL0)) != 0) + + ((c->key.attrs & BITFIELD64_BIT(VERT_RESULT_COL1)) != 0); + } + if (!nr) return; @@ -186,16 +193,16 @@ static void do_flatshade_triangle( struct brw_sf_compile *c ) brw_MUL(p, c->pv, c->pv, brw_imm_d(jmpi*(nr*2+1))); brw_JMPI(p, ip, ip, c->pv); - copy_colors(c, c->vert[1], c->vert[0]); - copy_colors(c, c->vert[2], c->vert[0]); + copy_colors(c, c->vert[1], c->vert[0], c->key.do_twoside_color); + copy_colors(c, c->vert[2], c->vert[0], c->key.do_twoside_color); brw_JMPI(p, ip, ip, brw_imm_d(jmpi*(nr*4+1))); - copy_colors(c, c->vert[0], c->vert[1]); - copy_colors(c, c->vert[2], c->vert[1]); + copy_colors(c, c->vert[0], c->vert[1], c->key.do_twoside_color); + copy_colors(c, c->vert[2], c->vert[1], c->key.do_twoside_color); brw_JMPI(p, ip, ip, brw_imm_d(jmpi*nr*2)); - copy_colors(c, c->vert[0], c->vert[2]); - copy_colors(c, c->vert[1], c->vert[2]); + copy_colors(c, c->vert[0], c->vert[2], c->key.do_twoside_color); + copy_colors(c, c->vert[1], c->vert[2], c->key.do_twoside_color); brw_pop_insn_state(p); } @@ -224,10 +231,10 @@ static void do_flatshade_line( struct brw_sf_compile *c ) brw_MUL(p, c->pv, c->pv, brw_imm_d(jmpi*(nr+1))); brw_JMPI(p, ip, ip, c->pv); - copy_colors(c, c->vert[1], c->vert[0]); + copy_colors(c, c->vert[1], c->vert[0], 0); brw_JMPI(p, ip, ip, brw_imm_ud(jmpi*nr)); - copy_colors(c, c->vert[0], c->vert[1]); + copy_colors(c, c->vert[0], c->vert[1], 0); brw_pop_insn_state(p); } @@ -337,13 +344,17 @@ calculate_masks(struct brw_sf_compile *c, if (c->key.do_flat_shading) persp_mask = c->key.attrs & ~(BITFIELD64_BIT(VERT_RESULT_HPOS) | BITFIELD64_BIT(VERT_RESULT_COL0) | - BITFIELD64_BIT(VERT_RESULT_COL1)); + BITFIELD64_BIT(VERT_RESULT_COL1) | + BITFIELD64_BIT(VERT_RESULT_BFC0) | + BITFIELD64_BIT(VERT_RESULT_BFC1)); else persp_mask = c->key.attrs & ~(BITFIELD64_BIT(VERT_RESULT_HPOS)); if (c->key.do_flat_shading) linear_mask = c->key.attrs & ~(BITFIELD64_BIT(VERT_RESULT_COL0) | - BITFIELD64_BIT(VERT_RESULT_COL1)); + BITFIELD64_BIT(VERT_RESULT_COL1) | + BITFIELD64_BIT(VERT_RESULT_BFC0) | + BITFIELD64_BIT(VERT_RESULT_BFC1)); else linear_mask = c->key.attrs; diff --git a/src/mesa/drivers/dri/i965/brw_wm_pass2.c b/src/mesa/drivers/dri/i965/brw_wm_pass2.c index 27c0a94..48143f3 100644 --- a/src/mesa/drivers/dri/i965/brw_wm_pass2.c +++ b/src/mesa/drivers/dri/i965/brw_wm_pass2.c @@ -96,9 +96,26 @@ static void init_registers( struct brw_wm_compile *c ) if (c->key.vp_outputs_written & BITFIELD64_BIT(j)) { int fp_index = _mesa_vert_result_to_frag_attrib(j); + /* Special case: two-sided vertex option, vertex program + * only writes to the back color. Map it to the + * associated front color location. + */ + if (j >= VERT_RESULT_BFC0 && j <= VERT_RESULT_BFC1 && + intel->ctx.VertexProgram._TwoSideEnabled && + !(c->key.vp_outputs_written & BITFIELD64_BIT(j - VERT_RESULT_BFC0 + VERT_RESULT_COL0))) + fp_index = j - VERT_RESULT_BFC0 + FRAG_ATTRIB_COL0; + nr_interp_regs++; + + /* The back color slot is skipped when the front color is + * also written to. In addition, some slots can be + * written in the vertex shader and not read in the + * fragment shader. So the register number must always be + * incremented, mapped or not. + */ if (fp_index >= 0) - prealloc_reg(c, &c->payload.input_interp[fp_index], i++); + prealloc_reg(c, &c->payload.input_interp[fp_index], i); + i++; } } assert(nr_interp_regs >= 1);