[Mesa-dev] [PATCH] draw: implement TGSI_PROPERTY_VS_WINDOW_SPACE_POSITION
Roland Scheidegger
sroland at vmware.com
Mon Dec 8 10:25:24 PST 2014
Am 06.12.2014 um 18:24 schrieb Marek Olšák:
> Ping.
>
> On Mon, Nov 17, 2014 at 10:43 PM, Marek Olšák <maraeo at gmail.com> wrote:
>> From: Marek Olšák <marek.olsak at 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
More information about the mesa-dev
mailing list