It's actually quite nice in our hardware drivers. The viewport transformation enable bit and the computation of 1/w bit now lives in the VS state. The clip enable bits have to be derived from both the VS and the rasterizer state on most/all drivers anyway (based on whether ClipDistances or ClipVertex is used or none of them - yeah we have 3 ways of doing clipping), so the clip disable bit is just part of that state now.
Marek On Mon, Dec 8, 2014 at 7:25 PM, Roland Scheidegger <srol...@vmware.com> wrote: > Am 06.12.2014 um 18:24 schrieb Marek Olšák: >> Ping. >> >> On Mon, Nov 17, 2014 at 10:43 PM, Marek Olšák <mar...@gmail.com> wrote: >>> From: Marek Olšák <marek.ol...@amd.com> >>> >>> Required by Nine. Tested with util_run_tests. >>> It's added to softpipe, llvmpipe, and r300g/swtcl. >>> --- >>> src/gallium/auxiliary/draw/draw_context.c | 40 >>> ++++++++++++++++++---- >>> src/gallium/auxiliary/draw/draw_llvm.c | 2 +- >>> src/gallium/auxiliary/draw/draw_private.h | 4 +++ >>> .../auxiliary/draw/draw_pt_fetch_shade_emit.c | 2 +- >>> .../auxiliary/draw/draw_pt_fetch_shade_pipeline.c | 2 +- >>> .../draw/draw_pt_fetch_shade_pipeline_llvm.c | 2 +- >>> src/gallium/auxiliary/draw/draw_vs.c | 2 ++ >>> src/gallium/drivers/llvmpipe/lp_screen.c | 2 ++ >>> src/gallium/drivers/r300/r300_screen.c | 2 +- >>> src/gallium/drivers/softpipe/sp_screen.c | 2 ++ >>> 10 files changed, 49 insertions(+), 11 deletions(-) >>> >>> diff --git a/src/gallium/auxiliary/draw/draw_context.c >>> b/src/gallium/auxiliary/draw/draw_context.c >>> index 2b640b6..d473cfc 100644 >>> --- a/src/gallium/auxiliary/draw/draw_context.c >>> +++ b/src/gallium/auxiliary/draw/draw_context.c >>> @@ -267,21 +267,48 @@ void draw_set_zs_format(struct draw_context *draw, >>> enum pipe_format format) >>> } >>> >>> >>> -static void update_clip_flags( struct draw_context *draw ) >>> +static bool >>> +draw_is_vs_window_space(struct draw_context *draw) >>> { >>> - draw->clip_xy = !draw->driver.bypass_clip_xy; >>> + if (draw->vs.vertex_shader) { >>> + struct tgsi_shader_info *info = &draw->vs.vertex_shader->info; >>> + >>> + return info->properties[TGSI_PROPERTY_VS_WINDOW_SPACE_POSITION] != 0; >>> + } >>> + return false; >>> +} >>> + >>> + >>> +void >>> +draw_update_clip_flags(struct draw_context *draw) >>> +{ >>> + bool window_space = draw_is_vs_window_space(draw); >>> + >>> + draw->clip_xy = !draw->driver.bypass_clip_xy && !window_space; >>> draw->guard_band_xy = (!draw->driver.bypass_clip_xy && >>> draw->driver.guard_band_xy); >>> draw->clip_z = (!draw->driver.bypass_clip_z && >>> - draw->rasterizer && draw->rasterizer->depth_clip); >>> + draw->rasterizer && draw->rasterizer->depth_clip) && >>> + !window_space; >>> draw->clip_user = draw->rasterizer && >>> - draw->rasterizer->clip_plane_enable != 0; >>> + draw->rasterizer->clip_plane_enable != 0 && >>> + !window_space; >>> draw->guard_band_points_xy = draw->guard_band_xy || >>> (draw->driver.bypass_clip_points && >>> (draw->rasterizer && >>> draw->rasterizer->point_tri_clip)); >>> } >>> >>> + >>> +void >>> +draw_update_viewport_flags(struct draw_context *draw) >>> +{ >>> + bool window_space = draw_is_vs_window_space(draw); >>> + >>> + draw->bypass_viewport = window_space || draw->identity_viewport; >>> +} >>> + >>> + >>> /** >>> * Register new primitive rasterization/rendering state. >>> * This causes the drawing pipeline to be rebuilt. >>> @@ -295,7 +322,7 @@ void draw_set_rasterizer_state( struct draw_context >>> *draw, >>> >>> draw->rasterizer = raster; >>> draw->rast_handle = rast_handle; >>> - update_clip_flags(draw); >>> + draw_update_clip_flags(draw); >>> } >>> } >>> >>> @@ -322,7 +349,7 @@ void draw_set_driver_clipping( struct draw_context >>> *draw, >>> draw->driver.bypass_clip_z = bypass_clip_z; >>> draw->driver.guard_band_xy = guard_band_xy; >>> draw->driver.bypass_clip_points = bypass_clip_points; >>> - update_clip_flags(draw); >>> + draw_update_clip_flags(draw); >>> } >>> >>> >>> @@ -376,6 +403,7 @@ void draw_set_viewport_states( struct draw_context >>> *draw, >>> viewport->translate[0] == 0.0f && >>> viewport->translate[1] == 0.0f && >>> viewport->translate[2] == 0.0f); >>> + draw_update_viewport_flags(draw); >>> } >>> >>> >>> diff --git a/src/gallium/auxiliary/draw/draw_llvm.c >>> b/src/gallium/auxiliary/draw/draw_llvm.c >>> index 3a1b057..fbbe08b 100644 >>> --- a/src/gallium/auxiliary/draw/draw_llvm.c >>> +++ b/src/gallium/auxiliary/draw/draw_llvm.c >>> @@ -1831,7 +1831,7 @@ draw_llvm_make_variant_key(struct draw_llvm *llvm, >>> char *store) >>> key->clip_xy = llvm->draw->clip_xy; >>> key->clip_z = llvm->draw->clip_z; >>> key->clip_user = llvm->draw->clip_user; >>> - key->bypass_viewport = llvm->draw->identity_viewport; >>> + key->bypass_viewport = llvm->draw->bypass_viewport; >>> key->clip_halfz = llvm->draw->rasterizer->clip_halfz; >>> key->need_edgeflags = (llvm->draw->vs.edgeflag_output ? TRUE : FALSE); >>> key->ucp_enable = llvm->draw->rasterizer->clip_plane_enable; >>> diff --git a/src/gallium/auxiliary/draw/draw_private.h >>> b/src/gallium/auxiliary/draw/draw_private.h >>> index d8dc2ab..8d4e1cd 100644 >>> --- a/src/gallium/auxiliary/draw/draw_private.h >>> +++ b/src/gallium/auxiliary/draw/draw_private.h >>> @@ -252,6 +252,7 @@ struct draw_context >>> >>> struct pipe_viewport_state viewports[PIPE_MAX_VIEWPORTS]; >>> boolean identity_viewport; >>> + boolean bypass_viewport; >>> >>> /** Vertex shader state */ >>> struct { >>> @@ -478,6 +479,9 @@ void >>> draw_stats_clipper_primitives(struct draw_context *draw, >>> const struct draw_prim_info *prim_info); >>> >>> +void draw_update_clip_flags(struct draw_context *draw); >>> +void draw_update_viewport_flags(struct draw_context *draw); >>> + >>> /** >>> * Return index i from the index buffer. >>> * If the index buffer would overflow we return the >>> diff --git a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_emit.c >>> b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_emit.c >>> index c675321..50f438c 100644 >>> --- a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_emit.c >>> +++ b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_emit.c >>> @@ -95,7 +95,7 @@ fse_prepare(struct draw_pt_middle_end *middle, >>> fse->key.nr_elements = MAX2(fse->key.nr_outputs, /* outputs - >>> translate to hw format */ >>> fse->key.nr_inputs); /* inputs - fetch >>> from api format */ >>> >>> - fse->key.viewport = !draw->identity_viewport; >>> + fse->key.viewport = !draw->bypass_viewport; >>> fse->key.clip = draw->clip_xy || draw->clip_z || draw->clip_user; >>> fse->key.const_vbuffers = 0; >>> >>> diff --git a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline.c >>> b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline.c >>> index f518dee..5af845f 100644 >>> --- a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline.c >>> +++ b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline.c >>> @@ -117,7 +117,7 @@ fetch_pipeline_prepare(struct draw_pt_middle_end >>> *middle, >>> draw->clip_user, >>> point_clip ? draw->guard_band_points_xy : >>> draw->guard_band_xy, >>> - draw->identity_viewport, >>> + draw->bypass_viewport, >>> draw->rasterizer->clip_halfz, >>> (draw->vs.edgeflag_output ? TRUE : FALSE) ); >>> >>> diff --git a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c >>> b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c >>> index 49341ff..06bd4e9 100644 >>> --- a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c >>> +++ b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c >>> @@ -162,7 +162,7 @@ llvm_middle_end_prepare( struct draw_pt_middle_end >>> *middle, >>> draw->clip_user, >>> point_clip ? draw->guard_band_points_xy : >>> draw->guard_band_xy, >>> - draw->identity_viewport, >>> + draw->bypass_viewport, >>> draw->rasterizer->clip_halfz, >>> (draw->vs.edgeflag_output ? TRUE : FALSE) ); >>> >>> diff --git a/src/gallium/auxiliary/draw/draw_vs.c >>> b/src/gallium/auxiliary/draw/draw_vs.c >>> index dc50870..73334d7 100644 >>> --- a/src/gallium/auxiliary/draw/draw_vs.c >>> +++ b/src/gallium/auxiliary/draw/draw_vs.c >>> @@ -120,6 +120,8 @@ draw_bind_vertex_shader(struct draw_context *draw, >>> draw->vs.clipdistance_output[0] = dvs->clipdistance_output[0]; >>> draw->vs.clipdistance_output[1] = dvs->clipdistance_output[1]; >>> dvs->prepare( dvs, draw ); >>> + draw_update_clip_flags(draw); >>> + draw_update_viewport_flags(draw); > I think technically you're also supposed to call these when you hit the > else clause below (going from a window space vs to no vs is supposed to > change the key). Doubt it will make any difference in practice, though... > >>> } >>> else { >>> draw->vs.vertex_shader = NULL; >>> diff --git a/src/gallium/drivers/llvmpipe/lp_screen.c >>> b/src/gallium/drivers/llvmpipe/lp_screen.c >>> index cec0fcb..bea2dc7 100644 >>> --- a/src/gallium/drivers/llvmpipe/lp_screen.c >>> +++ b/src/gallium/drivers/llvmpipe/lp_screen.c >>> @@ -251,7 +251,9 @@ llvmpipe_get_param(struct pipe_screen *screen, enum >>> pipe_cap param) >>> case PIPE_CAP_TEXTURE_QUERY_LOD: >>> case PIPE_CAP_SAMPLE_SHADING: >>> case PIPE_CAP_TEXTURE_GATHER_OFFSETS: >>> + return 0; >>> 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; >>> diff --git a/src/gallium/drivers/r300/r300_screen.c >>> b/src/gallium/drivers/r300/r300_screen.c >>> index 47616f6..e653eab 100644 >>> --- a/src/gallium/drivers/r300/r300_screen.c >>> +++ b/src/gallium/drivers/r300/r300_screen.c >>> @@ -177,7 +177,6 @@ static int r300_get_param(struct pipe_screen* pscreen, >>> enum pipe_cap param) >>> case PIPE_CAP_FAKE_SW_MSAA: >>> case PIPE_CAP_SAMPLE_SHADING: >>> case PIPE_CAP_TEXTURE_GATHER_OFFSETS: >>> - case PIPE_CAP_TGSI_VS_WINDOW_SPACE_POSITION: >>> case PIPE_CAP_DRAW_INDIRECT: >>> case PIPE_CAP_TGSI_FS_FINE_DERIVATIVE: >>> case PIPE_CAP_CONDITIONAL_RENDER_INVERTED: >>> @@ -187,6 +186,7 @@ static int r300_get_param(struct pipe_screen* pscreen, >>> enum pipe_cap param) >>> /* SWTCL-only features. */ >>> case PIPE_CAP_PRIMITIVE_RESTART: >>> case PIPE_CAP_USER_VERTEX_BUFFERS: >>> + case PIPE_CAP_TGSI_VS_WINDOW_SPACE_POSITION: >>> return !r300screen->caps.has_tcl; >>> >>> /* HWTCL-only features / limitations. */ >>> diff --git a/src/gallium/drivers/softpipe/sp_screen.c >>> b/src/gallium/drivers/softpipe/sp_screen.c >>> index 47126ef..57cd9b6 100644 >>> --- a/src/gallium/drivers/softpipe/sp_screen.c >>> +++ b/src/gallium/drivers/softpipe/sp_screen.c >>> @@ -196,7 +196,9 @@ softpipe_get_param(struct pipe_screen *screen, enum >>> pipe_cap param) >>> case PIPE_CAP_TEXTURE_QUERY_LOD: >>> case PIPE_CAP_SAMPLE_SHADING: >>> case PIPE_CAP_TEXTURE_GATHER_OFFSETS: >>> + return 0; >>> 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; >>> -- >>> 2.1.0 >>> > > Other than that, looks ok to me. I wonder though if adding this shader > property was really worth it, makes the logic of these bits even less > intuitive (as there's now 3 sources for them, rasterizer bits, shader > property bits, drivers wanting to disable them - and in case of the > latter drivers would still need to check for the shader property on > their own if they actually do their own clipping). > > Roland > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev