Looks good to me too! Reviewed-by: Matthew McClure <mcclu...@vmware.com>
Matthew ----- Original Message ----- From: "Roland Scheidegger" <srol...@vmware.com> To: "Zack Rusin" <za...@vmware.com>, mesa-dev@lists.freedesktop.org Cc: jfons...@vmware.com, mcclu...@vmware.com Sent: Tuesday, March 4, 2014 8:57:30 AM Subject: Re: [PATCH] draw/llvm: fix generation of the VS with GS present Am 04.03.2014 05:08, schrieb Zack Rusin: > draw_current_shader_* functions return a final output when considering > both the geometry shader and the vertex shader. But when code generating > vertex shader we can not be using output slots from the geometry shader > because, obviously, those can be completely different. This fixes a > number of very non-obvious crashes. > A side-effect of this bug was that sometimes the vertex shading code > could save some random outputs as position/clip when the geometry > shader was writing them and vertex shader had different outputs at > those slots (sometimes writing garbage and sometimes something correct). > --- > src/gallium/auxiliary/draw/draw_llvm.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/src/gallium/auxiliary/draw/draw_llvm.c > b/src/gallium/auxiliary/draw/draw_llvm.c > index 0bbb680..53d13f3 100644 > --- a/src/gallium/auxiliary/draw/draw_llvm.c > +++ b/src/gallium/auxiliary/draw/draw_llvm.c > @@ -1104,7 +1104,7 @@ generate_viewport(struct draw_llvm_variant *variant, > int i; > struct gallivm_state *gallivm = variant->gallivm; > struct lp_type f32_type = vs_type; > - const unsigned pos = > draw_current_shader_position_output(variant->llvm->draw); > + const unsigned pos = variant->llvm->draw->vs.position_output; > LLVMTypeRef vs_type_llvm = lp_build_vec_type(gallivm, vs_type); > LLVMValueRef out3 = LLVMBuildLoad(builder, outputs[pos][3], ""); /*w0 w1 > .. wn*/ > LLVMValueRef const1 = lp_build_const_vec(gallivm, f32_type, 1.0); > /*1.0 1.0 1.0 1.0*/ > @@ -1173,14 +1173,14 @@ generate_clipmask(struct draw_llvm *llvm, > LLVMValueRef plane1, planes, plane_ptr, sum; > struct lp_type f32_type = vs_type; > struct lp_type i32_type = lp_int_type(vs_type); > - const unsigned pos = draw_current_shader_position_output(llvm->draw); > - const unsigned cv = draw_current_shader_clipvertex_output(llvm->draw); > + const unsigned pos = llvm->draw->vs.position_output; > + const unsigned cv = llvm->draw->vs.clipvertex_output; > int num_written_clipdistance = > llvm->draw->vs.vertex_shader->info.num_written_clipdistance; > bool have_cd = false; > unsigned cd[2]; > > - cd[0] = draw_current_shader_clipdistance_output(llvm->draw, 0); > - cd[1] = draw_current_shader_clipdistance_output(llvm->draw, 1); > + cd[0] = llvm->draw->vs.clipdistance_output[0]; > + cd[1] = llvm->draw->vs.clipdistance_output[1]; > > if (cd[0] != pos || cd[1] != pos) > have_cd = true; > @@ -1551,8 +1551,8 @@ draw_llvm_generate(struct draw_llvm *llvm, struct > draw_llvm_variant *variant, > key->clip_z || > key->clip_user); > LLVMValueRef variant_func; > - const unsigned pos = draw_current_shader_position_output(llvm->draw); > - const unsigned cv = draw_current_shader_clipvertex_output(llvm->draw); > + const unsigned pos = llvm->draw->vs.position_output; > + const unsigned cv = llvm->draw->vs.clipvertex_output; > boolean have_clipdist = FALSE; > struct lp_bld_tgsi_system_values system_values; > > Both patches make sense to me, good catch! Roland _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
