Am 27.06.2013 09:41, schrieb Jose Fonseca: > ----- Original Message ----- >> From: Roland Scheidegger <srol...@vmware.com> >> >> This was just ignored (unless for some reason like unfilled polys draw was >> handling this). > > Patch looks good. > >> I'm not convinced of that code, putting the float for the clamp in the key >> isn't really a good idea. > > Indeed. > >> Then again the other floats for depth bias are >> already in there too anyway (should probably have a jit_context for the >> setup function), so this is just a quick fix. >> Also, the "minimum resolvable depth difference" used isn't really right as it >> should be calculated according to the z values of the current primitive >> and not be a constant (of course, this only makes a difference for float >> depth buffers), at least for d3d10, so depth biasing is still not quite >> right. >> --- >> src/gallium/drivers/llvmpipe/lp_state_setup.c | 20 +++++++++++++++++++- >> src/gallium/drivers/llvmpipe/lp_state_setup.h | 3 ++- >> src/gallium/drivers/llvmpipe/lp_state_surface.c | 2 ++ >> 3 files changed, 23 insertions(+), 2 deletions(-) >> >> diff --git a/src/gallium/drivers/llvmpipe/lp_state_setup.c >> b/src/gallium/drivers/llvmpipe/lp_state_setup.c >> index ed68b98..2988bed 100644 >> --- a/src/gallium/drivers/llvmpipe/lp_state_setup.c >> +++ b/src/gallium/drivers/llvmpipe/lp_state_setup.c >> @@ -244,6 +244,7 @@ lp_do_offset_tri(struct gallivm_state *gallivm, >> { >> LLVMBuilderRef b = gallivm->builder; >> struct lp_build_context bld; >> + struct lp_build_context flt_scalar_bld; >> LLVMValueRef zoffset, mult; >> LLVMValueRef z0_new, z1_new, z2_new; >> LLVMValueRef dzdxdzdy, dzdx, dzdy, dzxyz20, dyzzx01, dyzzx01_dzxyz20, >> dzx01_dyz20; >> @@ -298,6 +299,18 @@ lp_do_offset_tri(struct gallivm_state *gallivm, >> lp_build_const_float(gallivm, >> key->pgon_offset_units), >> mult, "zoffset"); >> >> + lp_build_context_init(&flt_scalar_bld, gallivm, lp_type_float_vec(32, >> 32)); >> + if (key->pgon_offset_clamp > 0) { >> + zoffset = lp_build_min(&flt_scalar_bld, >> + lp_build_const_float(gallivm, >> key->pgon_offset_clamp), >> + zoffset); >> + } >> + else if (key->pgon_offset_clamp < 0) { >> + zoffset = lp_build_max(&flt_scalar_bld, >> + lp_build_const_float(gallivm, >> key->pgon_offset_clamp), >> + zoffset); >> + } >> + >> /* yuck */ >> shuffles[0] = twoi; >> shuffles[1] = lp_build_const_int32(gallivm, 6); >> @@ -312,6 +325,10 @@ lp_do_offset_tri(struct gallivm_state *gallivm, >> zoffset = vec4f_from_scalar(gallivm, zoffset, ""); >> >> /* clamp and do offset */ >> + /* >> + * XXX I suspect the clamp (is that even right to always clamp to fixed >> 0.0/1.0?) >> + * should really be per fragment? >> + */ > > yes. this doesn't look right. Should probably be a FIXME too. Ok. Draw actually has a similar comment though I believe it is a bit wrong there (because it is saying offset should be per-fragment too which I think isn't true at all, just the clamp though I'm unsure where this comes from in the first place).
Roland > >> z0z1z2 = lp_build_clamp(&bld, LLVMBuildFAdd(b, z0z1z2, zoffset, ""), >> bld.zero, bld.one); >> >> /* insert into args->a0.z, a1.z, a2.z: >> @@ -810,7 +827,7 @@ lp_make_setup_variant_key(struct llvmpipe_context *lp, >> key->pixel_center_half = lp->rasterizer->half_pixel_center; >> key->twoside = lp->rasterizer->light_twoside; >> key->size = Offset(struct lp_setup_variant_key, >> - inputs[key->num_inputs]); >> + inputs[key->num_inputs]); >> >> key->color_slot = lp->color_slot [0]; >> key->bcolor_slot = lp->bcolor_slot[0]; >> @@ -823,6 +840,7 @@ lp_make_setup_variant_key(struct llvmpipe_context *lp, >> >> key->pgon_offset_units = (float) (lp->rasterizer->offset_units * >> lp->mrd); >> key->pgon_offset_scale = lp->rasterizer->offset_scale; >> + key->pgon_offset_clamp = lp->rasterizer->offset_clamp; >> key->pad = 0; >> memcpy(key->inputs, fs->inputs, key->num_inputs * sizeof key->inputs[0]); >> for (i = 0; i < key->num_inputs; i++) { >> diff --git a/src/gallium/drivers/llvmpipe/lp_state_setup.h >> b/src/gallium/drivers/llvmpipe/lp_state_setup.h >> index 73d40a5..c2a2c7f 100644 >> --- a/src/gallium/drivers/llvmpipe/lp_state_setup.h >> +++ b/src/gallium/drivers/llvmpipe/lp_state_setup.h >> @@ -14,7 +14,7 @@ struct lp_setup_variant_list_item >> }; >> >> >> -struct lp_setup_variant_key { >> +struct lp_setup_variant_key { >> unsigned size:16; >> unsigned num_inputs:8; >> int color_slot:8; >> @@ -29,6 +29,7 @@ struct lp_setup_variant_key { >> >> float pgon_offset_units; >> float pgon_offset_scale; >> + float pgon_offset_clamp; >> struct lp_shader_input inputs[PIPE_MAX_SHADER_INPUTS]; >> }; >> >> diff --git a/src/gallium/drivers/llvmpipe/lp_state_surface.c >> b/src/gallium/drivers/llvmpipe/lp_state_surface.c >> index 375ceb2..e6aac31 100644 >> --- a/src/gallium/drivers/llvmpipe/lp_state_surface.c >> +++ b/src/gallium/drivers/llvmpipe/lp_state_surface.c >> @@ -65,6 +65,8 @@ llvmpipe_set_framebuffer_state(struct pipe_context *pipe, >> } >> >> /* Tell draw module how deep the Z/depth buffer is */ >> + /* FIXME: mrd constant isn't right should use a value derived from >> + * current primitive not a constant (for float depth buffers) */ >> if (lp->framebuffer.zsbuf) { >> int depth_bits; >> double mrd; >> -- >> 1.7.9.5 >> > > Jose > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev