Am 13.05.2015 um 16:32 schrieb Brian Paul: > On 05/12/2015 08:36 PM, srol...@vmware.com wrote: >> From: Roland Scheidegger <srol...@vmware.com> >> >> All the functionality was pretty much there, just not tested. >> Trivially fix up the missing pieces (take target info from view not >> resource), and add some missing bits for cubes. >> Also add some minimal debug validation to detect uninitialized target >> values >> in the view... >> >> 49 new piglits, 47 pass, 2 fail (both related to fake multisampling, >> not texture_view itself). No other piglit changes. >> --- >> src/gallium/auxiliary/gallivm/lp_bld_sample.c | 2 +- >> src/gallium/drivers/llvmpipe/lp_screen.c | 3 ++- >> src/gallium/drivers/llvmpipe/lp_setup.c | 36 >> ++++++++++++++++++++++--- >> src/gallium/drivers/llvmpipe/lp_state_sampler.c | 36 >> ++++++++++++++++++++++--- >> 4 files changed, 67 insertions(+), 10 deletions(-) >> >> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_sample.c >> b/src/gallium/auxiliary/gallivm/lp_bld_sample.c >> index 5b22045..4befb3a 100644 >> --- a/src/gallium/auxiliary/gallivm/lp_bld_sample.c >> +++ b/src/gallium/auxiliary/gallivm/lp_bld_sample.c >> @@ -113,7 +113,7 @@ lp_sampler_static_texture_state(struct >> lp_static_texture_state *state, >> state->swizzle_b = view->swizzle_b; >> state->swizzle_a = view->swizzle_a; >> >> - state->target = texture->target; >> + state->target = view->target; >> state->pot_width = util_is_power_of_two(texture->width0); >> state->pot_height = util_is_power_of_two(texture->height0); >> state->pot_depth = util_is_power_of_two(texture->depth0); >> diff --git a/src/gallium/drivers/llvmpipe/lp_screen.c >> b/src/gallium/drivers/llvmpipe/lp_screen.c >> index f4ba596..e001e53 100644 >> --- a/src/gallium/drivers/llvmpipe/lp_screen.c >> +++ b/src/gallium/drivers/llvmpipe/lp_screen.c >> @@ -258,8 +258,9 @@ llvmpipe_get_param(struct pipe_screen *screen, >> enum pipe_cap param) >> case PIPE_CAP_TGSI_VS_WINDOW_SPACE_POSITION: >> return 1; >> case PIPE_CAP_TGSI_FS_FINE_DERIVATIVE: >> - case PIPE_CAP_SAMPLER_VIEW_TARGET: >> return 0; >> + case PIPE_CAP_SAMPLER_VIEW_TARGET: >> + return 1; >> case PIPE_CAP_FAKE_SW_MSAA: >> return 1; >> case PIPE_CAP_CONDITIONAL_RENDER_INVERTED: >> diff --git a/src/gallium/drivers/llvmpipe/lp_setup.c >> b/src/gallium/drivers/llvmpipe/lp_setup.c >> index 96cc77c..4c57fab 100644 >> --- a/src/gallium/drivers/llvmpipe/lp_setup.c >> +++ b/src/gallium/drivers/llvmpipe/lp_setup.c >> @@ -811,6 +811,32 @@ lp_setup_set_fragment_sampler_views(struct >> lp_setup_context *setup, >> */ >> pipe_resource_reference(&setup->fs.current_tex[i], res); >> >> +#ifdef DEBUG >> + /* this is possibly too lenient */ >> + if (view->target != res->target) { >> + if (view->target == PIPE_TEXTURE_1D) >> + assert(res->target == PIPE_TEXTURE_1D_ARRAY); >> + else if (view->target == PIPE_TEXTURE_1D_ARRAY) >> + assert(res->target == PIPE_TEXTURE_1D); >> + else if (view->target == PIPE_TEXTURE_2D) >> + assert(res->target == PIPE_TEXTURE_2D_ARRAY || >> + res->target == PIPE_TEXTURE_CUBE || >> + res->target == PIPE_TEXTURE_CUBE_ARRAY); >> + else if (view->target == PIPE_TEXTURE_2D_ARRAY) >> + assert(res->target == PIPE_TEXTURE_2D || >> + res->target == PIPE_TEXTURE_CUBE || >> + res->target == PIPE_TEXTURE_CUBE_ARRAY); >> + else if (view->target == PIPE_TEXTURE_CUBE) >> + assert(res->target == PIPE_TEXTURE_CUBE_ARRAY || >> + res->target == PIPE_TEXTURE_2D_ARRAY); >> + else if (view->target == PIPE_TEXTURE_CUBE_ARRAY) >> + assert(res->target == PIPE_TEXTURE_CUBE || >> + res->target == PIPE_TEXTURE_2D_ARRAY); >> + else >> + assert(0); >> + } >> +#endif >> + >> if (!lp_tex->dt) { >> /* regular texture - setup array of mipmap level offsets */ >> int j; >> @@ -854,9 +880,10 @@ lp_setup_set_fragment_sampler_views(struct >> lp_setup_context *setup, >> jit_tex->img_stride[j] = lp_tex->img_stride[j]; >> } >> >> - if (res->target == PIPE_TEXTURE_1D_ARRAY || >> - res->target == PIPE_TEXTURE_2D_ARRAY || >> - res->target == PIPE_TEXTURE_CUBE_ARRAY) { >> + if (view->target == PIPE_TEXTURE_1D_ARRAY || >> + view->target == PIPE_TEXTURE_2D_ARRAY || >> + view->target == PIPE_TEXTURE_CUBE || >> + view->target == PIPE_TEXTURE_CUBE_ARRAY) { >> /* >> * For array textures, we don't have >> first_layer, instead >> * adjust last_layer (stored as depth) plus the >> mip level offsets >> @@ -868,7 +895,8 @@ lp_setup_set_fragment_sampler_views(struct >> lp_setup_context *setup, >> jit_tex->mip_offsets[j] += >> view->u.tex.first_layer * >> >> lp_tex->img_stride[j]; >> } >> - if (res->target == PIPE_TEXTURE_CUBE_ARRAY) { >> + if (view->target == PIPE_TEXTURE_CUBE || >> + view->target == PIPE_TEXTURE_CUBE_ARRAY) { >> assert(jit_tex->depth % 6 == 0); >> } >> assert(view->u.tex.first_layer <= >> view->u.tex.last_layer); >> diff --git a/src/gallium/drivers/llvmpipe/lp_state_sampler.c >> b/src/gallium/drivers/llvmpipe/lp_state_sampler.c >> index 21da629..a9d0cd1 100644 >> --- a/src/gallium/drivers/llvmpipe/lp_state_sampler.c >> +++ b/src/gallium/drivers/llvmpipe/lp_state_sampler.c >> @@ -228,6 +228,32 @@ prepare_shader_sampling( >> */ >> pipe_resource_reference(&mapped_tex[i], tex); >> >> +#ifdef DEBUG >> + /* this is possibly too lenient */ >> + if (view->target != tex->target) { >> + if (view->target == PIPE_TEXTURE_1D) >> + assert(tex->target == PIPE_TEXTURE_1D_ARRAY); >> + else if (view->target == PIPE_TEXTURE_1D_ARRAY) >> + assert(tex->target == PIPE_TEXTURE_1D); >> + else if (view->target == PIPE_TEXTURE_2D) >> + assert(tex->target == PIPE_TEXTURE_2D_ARRAY || >> + tex->target == PIPE_TEXTURE_CUBE || >> + tex->target == PIPE_TEXTURE_CUBE_ARRAY); >> + else if (view->target == PIPE_TEXTURE_2D_ARRAY) >> + assert(tex->target == PIPE_TEXTURE_2D || >> + tex->target == PIPE_TEXTURE_CUBE || >> + tex->target == PIPE_TEXTURE_CUBE_ARRAY); >> + else if (view->target == PIPE_TEXTURE_CUBE) >> + assert(tex->target == PIPE_TEXTURE_CUBE_ARRAY || >> + tex->target == PIPE_TEXTURE_2D_ARRAY); >> + else if (view->target == PIPE_TEXTURE_CUBE_ARRAY) >> + assert(tex->target == PIPE_TEXTURE_CUBE || >> + tex->target == PIPE_TEXTURE_2D_ARRAY); >> + else >> + assert(0); >> + } >> +#endif > > How about putting these assertions into a function to avoid duplication > with the other ones earlier? Are they also useful for softpipe? I can put it into some util helper and use it in softpipe too. (The whole reason I put it there is to catch errors like in the 1/3 patch of the series - this caused very weird failures from crashes to misrenderings in about 50 tests but without seemingly any connection to texture_view just spread out all over piglit...). Though actually now that I think about it is in the wrong place, it should be in sampler view creation. I'll redo that.
Roland > > Otherwise, > > Reviewed-by: Brian Paul <bri...@vmware.com> > > >> + >> if (!lp_tex->dt) { >> /* regular texture - setup array of mipmap level offsets */ >> struct pipe_resource *res = view->texture; >> @@ -245,15 +271,17 @@ prepare_shader_sampling( >> row_stride[j] = lp_tex->row_stride[j]; >> img_stride[j] = lp_tex->img_stride[j]; >> } >> - if (res->target == PIPE_TEXTURE_1D_ARRAY || >> - res->target == PIPE_TEXTURE_2D_ARRAY || >> - res->target == PIPE_TEXTURE_CUBE_ARRAY) { >> + if (view->target == PIPE_TEXTURE_1D_ARRAY || >> + view->target == PIPE_TEXTURE_2D_ARRAY || >> + view->target == PIPE_TEXTURE_CUBE || >> + view->target == PIPE_TEXTURE_CUBE_ARRAY) { >> num_layers = view->u.tex.last_layer - >> view->u.tex.first_layer + 1; >> for (j = first_level; j <= last_level; j++) { >> mip_offsets[j] += view->u.tex.first_layer * >> lp_tex->img_stride[j]; >> } >> - if (res->target == PIPE_TEXTURE_CUBE_ARRAY) { >> + if (view->target == PIPE_TEXTURE_CUBE || >> + view->target == PIPE_TEXTURE_CUBE_ARRAY) { >> assert(num_layers % 6 == 0); >> } >> assert(view->u.tex.first_layer <= >> view->u.tex.last_layer); >> > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev