[Mesa-dev] [PATCH] i965: Implement support for ARB_clip_control.

Kenneth Graunke kenneth at whitecape.org
Tue Mar 31 14:31:41 PDT 2015


On Tuesday, March 31, 2015 08:33:24 AM Mathias.Froehlich at gmx.net wrote:
> From: Mathias Fröhlich <Mathias.Froehlich at gmx.net>
> 
> Hi,
> 
> The patch aims to implement ARB_clip_control on intel chips.
> I hope to have found all places to cover all supported chipsets.
> I have done some limited testing on an Ivybridge Mobile and
> a GM45 Express chipset.
> Please review.
> 
> Thanks
> 
> Mathias
> 
> Switch between the two clip space definitions already available
> in hardware. Update winding order dependent state according
> to the clip control state.
> This change did not introduce new piglit quick.test regressions on
> an Ivybridge Mobile and a GM45 Express chipset.
> Also it enables and passes the clip-control and clip-control-depth-precision
> tests on these two chipsets.
> 
> Signed-off-by: Mathias Froehlich <Mathias.Froehlich at web.de>

Wow, Gen4 even :)  This looks good to me - and using _FrontBit does
simplify things quite a bit.  Thanks!

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

I ran some Intel internal tests at this (which barely even function, so
I don't put a lot of stock in them)...it seems like there might be some
bad interactions with multisampling.  But I'm not seeing anything
obviously missing.  Most of the tests look good.

> ---
>  docs/GL3.txt                                 | 2 +-
>  docs/relnotes/10.6.0.html                    | 1 +
>  src/mesa/drivers/dri/i965/brw_clip.c         | 7 ++-----
>  src/mesa/drivers/dri/i965/brw_clip_state.c   | 5 ++++-
>  src/mesa/drivers/dri/i965/brw_sf.c           | 2 +-
>  src/mesa/drivers/dri/i965/brw_sf_state.c     | 6 +++---
>  src/mesa/drivers/dri/i965/gen6_clip_state.c  | 7 +++++--
>  src/mesa/drivers/dri/i965/gen6_sf_state.c    | 2 +-
>  src/mesa/drivers/dri/i965/gen7_sf_state.c    | 2 +-
>  src/mesa/drivers/dri/i965/gen8_sf_state.c    | 2 +-
>  src/mesa/drivers/dri/i965/intel_extensions.c | 1 +
>  11 files changed, 21 insertions(+), 16 deletions(-)
> 
> diff --git a/docs/GL3.txt b/docs/GL3.txt
> index 3614260..6cc0087 100644
> --- a/docs/GL3.txt
> +++ b/docs/GL3.txt
> @@ -188,7 +188,7 @@ GL 4.4, GLSL 4.40:
>  GL 4.5, GLSL 4.50:
>  
>    GL_ARB_ES3_1_compatibility                           not started
> -  GL_ARB_clip_control                                  DONE (nv50, nvc0, r600, radeonsi, llvmpipe, softpipe)
> +  GL_ARB_clip_control                                  DONE (i965, nv50, nvc0, r600, radeonsi, llvmpipe, softpipe)
>    GL_ARB_conditional_render_inverted                   DONE (i965, nv50, nvc0, llvmpipe, softpipe)
>    GL_ARB_cull_distance                                 not started
>    GL_ARB_derivative_control                            DONE (i965, nv50, nvc0, r600)
> diff --git a/docs/relnotes/10.6.0.html b/docs/relnotes/10.6.0.html
> index 00aaaa5..c62d0d8 100644
> --- a/docs/relnotes/10.6.0.html
> +++ b/docs/relnotes/10.6.0.html
> @@ -50,6 +50,7 @@ Note: some of the new features are only available with certain drivers.
>  <li>GL_ARB_instanced_arrays on freedreno</li>
>  <li>GL_ARB_pipeline_statistics_query on i965, nv50, nvc0, r600, radeonsi, softpipe</li>
>  <li>GL_ARB_draw_indirect, GL_ARB_multi_draw_indirect on r600</li>
> +<li>GL_ARB_clip_control on i965</li>
>  </ul>
>  
>  <h2>Bug fixes</h2>
> diff --git a/src/mesa/drivers/dri/i965/brw_clip.c b/src/mesa/drivers/dri/i965/brw_clip.c
> index 3fef38c..de78f46 100644
> --- a/src/mesa/drivers/dri/i965/brw_clip.c
> +++ b/src/mesa/drivers/dri/i965/brw_clip.c
> @@ -224,8 +224,7 @@ brw_upload_clip_prog(struct brw_context *brw)
>  	       key.offset_factor = ctx->Polygon.OffsetFactor * ctx->DrawBuffer->_MRD;
>  	    }
>  
> -	    switch (ctx->Polygon.FrontFace) {
> -	    case GL_CCW:
> +	    if (!ctx->Polygon._FrontBit) {
>  	       key.fill_ccw = fill_front;
>  	       key.fill_cw = fill_back;
>  	       key.offset_ccw = offset_front;
> @@ -233,8 +232,7 @@ brw_upload_clip_prog(struct brw_context *brw)
>  	       if (ctx->Light.Model.TwoSide &&
>  		   key.fill_cw != CLIP_CULL)
>  		  key.copy_bfc_cw = 1;
> -	       break;
> -	    case GL_CW:
> +	    } else {
>  	       key.fill_cw = fill_front;
>  	       key.fill_ccw = fill_back;
>  	       key.offset_cw = offset_front;
> @@ -242,7 +240,6 @@ brw_upload_clip_prog(struct brw_context *brw)
>  	       if (ctx->Light.Model.TwoSide &&
>  		   key.fill_ccw != CLIP_CULL)
>  		  key.copy_bfc_ccw = 1;
> -	       break;
>  	    }
>  	 }
>        }
> diff --git a/src/mesa/drivers/dri/i965/brw_clip_state.c b/src/mesa/drivers/dri/i965/brw_clip_state.c
> index 09a2523..385b8a4 100644
> --- a/src/mesa/drivers/dri/i965/brw_clip_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_clip_state.c
> @@ -147,7 +147,10 @@ brw_upload_clip_unit(struct brw_context *brw)
>        clip->clip5.viewport_z_clip_enable = 1;
>     clip->clip5.viewport_xy_clip_enable = 1;
>     clip->clip5.vertex_position_space = BRW_CLIP_NDCSPACE;
> -   clip->clip5.api_mode = BRW_CLIP_API_OGL;
> +   if (ctx->Transform.ClipDepthMode == GL_ZERO_TO_ONE)
> +      clip->clip5.api_mode = BRW_CLIP_API_DX;
> +   else
> +      clip->clip5.api_mode = BRW_CLIP_API_OGL;
>     clip->clip5.clip_mode = brw->clip.prog_data->clip_mode;
>  
>     if (brw->is_g4x)
> diff --git a/src/mesa/drivers/dri/i965/brw_sf.c b/src/mesa/drivers/dri/i965/brw_sf.c
> index a41a4ad..d5395de 100644
> --- a/src/mesa/drivers/dri/i965/brw_sf.c
> +++ b/src/mesa/drivers/dri/i965/brw_sf.c
> @@ -204,7 +204,7 @@ brw_upload_sf_prog(struct brw_context *brw)
>         * face orientation, just as we invert the viewport in
>         * sf_unit_create_from_key().
>         */
> -      key.frontface_ccw = (ctx->Polygon.FrontFace == GL_CCW) != render_to_fbo;
> +      key.frontface_ccw = ctx->Polygon._FrontBit == render_to_fbo;
>     }
>  
>     if (!brw_search_cache(&brw->cache, BRW_CACHE_SF_PROG,
> diff --git a/src/mesa/drivers/dri/i965/brw_sf_state.c b/src/mesa/drivers/dri/i965/brw_sf_state.c
> index 75d6451..d9b9213 100644
> --- a/src/mesa/drivers/dri/i965/brw_sf_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_sf_state.c
> @@ -182,10 +182,10 @@ static void upload_sf_unit( struct brw_context *brw )
>        sf->sf6.scissor = 1;
>  
>     /* _NEW_POLYGON */
> -   if (ctx->Polygon.FrontFace == GL_CCW)
> -      sf->sf5.front_winding = BRW_FRONTWINDING_CCW;
> -   else
> +   if (ctx->Polygon._FrontBit)
>        sf->sf5.front_winding = BRW_FRONTWINDING_CW;
> +   else
> +      sf->sf5.front_winding = BRW_FRONTWINDING_CCW;
>  
>     /* _NEW_BUFFERS
>      * The viewport is inverted for rendering to a FBO, and that inverts
> diff --git a/src/mesa/drivers/dri/i965/gen6_clip_state.c b/src/mesa/drivers/dri/i965/gen6_clip_state.c
> index e8c16ca..aaf90df 100644
> --- a/src/mesa/drivers/dri/i965/gen6_clip_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_clip_state.c
> @@ -54,7 +54,7 @@ upload_clip_state(struct brw_context *brw)
>  
>     if (brw->gen == 7) {
>        /* _NEW_POLYGON */
> -      if ((ctx->Polygon.FrontFace == GL_CCW) ^ _mesa_is_user_fbo(fb))
> +      if (ctx->Polygon._FrontBit == _mesa_is_user_fbo(fb))
>           dw1 |= GEN7_CLIP_WINDING_CCW;
>  
>        if (ctx->Polygon.CullFlag) {
> @@ -95,6 +95,10 @@ upload_clip_state(struct brw_context *brw)
>     /* _NEW_TRANSFORM */
>     dw2 |= (ctx->Transform.ClipPlanesEnabled <<
>             GEN6_USER_CLIP_CLIP_DISTANCES_SHIFT);
> +   if (ctx->Transform.ClipDepthMode == GL_ZERO_TO_ONE)
> +      dw2 |= GEN6_CLIP_API_D3D;
> +   else
> +      dw2 |= GEN6_CLIP_API_OGL;
>  
>     dw2 |= GEN6_CLIP_GB_TEST;
>  
> @@ -170,7 +174,6 @@ upload_clip_state(struct brw_context *brw)
>     OUT_BATCH(_3DSTATE_CLIP << 16 | (4 - 2));
>     OUT_BATCH(dw1);
>     OUT_BATCH(enable |
> -	     GEN6_CLIP_API_OGL |
>  	     GEN6_CLIP_MODE_NORMAL |
>  	     GEN6_CLIP_XY_TEST |
>  	     dw2);
> diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c b/src/mesa/drivers/dri/i965/gen6_sf_state.c
> index f9d8d27..ea5c47a 100644
> --- a/src/mesa/drivers/dri/i965/gen6_sf_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c
> @@ -290,7 +290,7 @@ upload_sf_state(struct brw_context *brw)
>     dw4 = 0;
>  
>     /* _NEW_POLYGON */
> -   if ((ctx->Polygon.FrontFace == GL_CCW) ^ render_to_fbo)
> +   if (ctx->Polygon._FrontBit == render_to_fbo)
>        dw2 |= GEN6_SF_WINDING_CCW;
>  
>     if (ctx->Polygon.OffsetFill)
> diff --git a/src/mesa/drivers/dri/i965/gen7_sf_state.c b/src/mesa/drivers/dri/i965/gen7_sf_state.c
> index c9815b0..69853e6 100644
> --- a/src/mesa/drivers/dri/i965/gen7_sf_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_sf_state.c
> @@ -120,7 +120,7 @@ upload_sf_state(struct brw_context *brw)
>     dw1 |= (brw_depthbuffer_format(brw) << GEN7_SF_DEPTH_BUFFER_SURFACE_FORMAT_SHIFT);
>  
>     /* _NEW_POLYGON */
> -   if ((ctx->Polygon.FrontFace == GL_CCW) ^ render_to_fbo)
> +   if (ctx->Polygon._FrontBit == render_to_fbo)
>        dw1 |= GEN6_SF_WINDING_CCW;
>  
>     if (ctx->Polygon.OffsetFill)
> diff --git a/src/mesa/drivers/dri/i965/gen8_sf_state.c b/src/mesa/drivers/dri/i965/gen8_sf_state.c
> index 27116f7..52a21b6 100644
> --- a/src/mesa/drivers/dri/i965/gen8_sf_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_sf_state.c
> @@ -229,7 +229,7 @@ upload_raster(struct brw_context *brw)
>     bool render_to_fbo = _mesa_is_user_fbo(brw->ctx.DrawBuffer);
>  
>     /* _NEW_POLYGON */
> -   if ((ctx->Polygon.FrontFace == GL_CCW) ^ render_to_fbo)
> +   if (ctx->Polygon._FrontBit == render_to_fbo)
>        dw1 |= GEN8_RASTER_FRONT_WINDING_CCW;
>  
>     if (ctx->Polygon.CullFlag) {
> diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c b/src/mesa/drivers/dri/i965/intel_extensions.c
> index 608bfac9..48064e1 100644
> --- a/src/mesa/drivers/dri/i965/intel_extensions.c
> +++ b/src/mesa/drivers/dri/i965/intel_extensions.c
> @@ -178,6 +178,7 @@ intelInitExtensions(struct gl_context *ctx)
>  
>     ctx->Extensions.ARB_buffer_storage = true;
>     ctx->Extensions.ARB_clear_texture = true;
> +   ctx->Extensions.ARB_clip_control = true;
>     ctx->Extensions.ARB_copy_image = true;
>     ctx->Extensions.ARB_depth_buffer_float = true;
>     ctx->Extensions.ARB_depth_clamp = true;
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150331/65963669/attachment.sig>


More information about the mesa-dev mailing list