[Mesa-dev] [PATCH] draw: implement TGSI_PROPERTY_VS_WINDOW_SPACE_POSITION
Marek Olšák
maraeo at gmail.com
Mon Dec 8 11:19:55 PST 2014
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 <sroland at 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 <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