On Thu, Aug 23, 2018 at 8:18 PM, Sagar Ghuge <sagar.gh...@intel.com> wrote:
> Enable _mesa_PushAttrib() and _mesa_PopAttrib() to handle
> GL_DEPTH_CLAMP_NEAR_AMD and GL_DEPTH_CLAMP_FAR_AMD tokens.
>
> Remove DepthClamp, because DepthClampNear + DepthClampFar replaces it,
> as suggested by Marek Olsak.
>
> Driver that enables AMD_depth_clamp_separate will only ever look at
> DepthClampNear and DepthClampFar, as suggested by Ian Romanick.
>
> v2: 1) Remove unnecessary parentheses (Marek Olsak)
> 2) if AMD_depth_clamp_separate is unsupported, TEST_AND_UPDATE
> GL_DEPTH_CLAMP only (Marek Olsak)
> 3) Clamp against near and far plane separately (Marek Olsak)
> 4) Clip point separately for near and far Z clipping plane (Marek
> Olsak)
>
> v3: Clamp raster position zw to the range [min(n,f), 0] for near plane
> and [0, max(n,f)] for far plane (Marek Olsak)
>
> Signed-off-by: Sagar Ghuge <sagar.gh...@intel.com>
> Reviewed-by: Ian Romanick <ian.d.roman...@intel.com>
> ---
> src/mesa/drivers/dri/i965/genX_state_upload.c | 11 ++--
> src/mesa/main/attrib.c | 40 ++++++++++---
> src/mesa/main/enable.c | 9 ++-
> src/mesa/main/get.c | 4 ++
> src/mesa/main/get_hash_params.py | 2 +-
> src/mesa/main/mtypes.h | 1 -
> src/mesa/main/rastpos.c | 59 ++++++++++++++++---
> src/mesa/state_tracker/st_atom_rasterizer.c | 3 +-
> src/mesa/state_tracker/st_cb_drawpixels.c | 3 +-
> src/mesa/swrast/s_span.c | 2 +-
> src/mesa/tnl/t_vb_program.c | 6 +-
> src/mesa/tnl/t_vb_vertex.c | 8 ++-
> 12 files changed, 115 insertions(+), 33 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c
> b/src/mesa/drivers/dri/i965/genX_state_upload.c
> index c051848985..dc54cb67af 100644
> --- a/src/mesa/drivers/dri/i965/genX_state_upload.c
> +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
> @@ -1399,7 +1399,8 @@ genX(upload_clip_state)(struct brw_context *brw)
> clip.ScreenSpaceViewportYMax = 1;
>
> clip.ViewportXYClipTestEnable = true;
> - clip.ViewportZClipTestEnable = !ctx->Transform.DepthClamp;
> + clip.ViewportZClipTestEnable = !(ctx->Transform.DepthClampNear &&
> + ctx->Transform.DepthClampFar);
>
> /* _NEW_TRANSFORM */
> if (GEN_GEN == 5 || GEN_IS_G4X) {
> @@ -1493,7 +1494,8 @@ genX(upload_clip_state)(struct brw_context *brw)
> clip.UserClipDistanceCullTestEnableBitmask =
> brw_vue_prog_data(brw->vs.base.prog_data)->cull_distance_mask;
>
> - clip.ViewportZClipTestEnable = !ctx->Transform.DepthClamp;
> + clip.ViewportZClipTestEnable = !(ctx->Transform.DepthClampNear &&
> + ctx->Transform.DepthClampFar);
> #endif
>
> /* _NEW_LIGHT */
> @@ -2338,7 +2340,7 @@ genX(upload_cc_viewport)(struct brw_context *brw)
> for (unsigned i = 0; i < viewport_count; i++) {
> /* _NEW_VIEWPORT | _NEW_TRANSFORM */
> const struct gl_viewport_attrib *vp = &ctx->ViewportArray[i];
> - if (ctx->Transform.DepthClamp) {
> + if (ctx->Transform.DepthClampNear && ctx->Transform.DepthClampFar) {
> ccv.MinimumDepth = MIN2(vp->Near, vp->Far);
> ccv.MaximumDepth = MAX2(vp->Near, vp->Far);
> } else {
> @@ -4605,7 +4607,8 @@ genX(upload_raster)(struct brw_context *brw)
> raster.ScissorRectangleEnable = ctx->Scissor.EnableFlags;
>
> /* _NEW_TRANSFORM */
> - if (!ctx->Transform.DepthClamp) {
> + if (!(ctx->Transform.DepthClampNear &&
> + ctx->Transform.DepthClampFar)) {
> #if GEN_GEN >= 9
> raster.ViewportZFarClipTestEnable = true;
> raster.ViewportZNearClipTestEnable = true;
> diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c
> index cbe93ab6fa..a46fec73fd 100644
> --- a/src/mesa/main/attrib.c
> +++ b/src/mesa/main/attrib.c
> @@ -72,7 +72,8 @@ struct gl_enable_attrib
> GLbitfield ClipPlanes;
> GLboolean ColorMaterial;
> GLboolean CullFace;
> - GLboolean DepthClamp;
> + GLboolean DepthClampNear;
> + GLboolean DepthClampFar;
> GLboolean DepthTest;
> GLboolean Dither;
> GLboolean Fog;
> @@ -336,7 +337,8 @@ _mesa_PushAttrib(GLbitfield mask)
> attr->ClipPlanes = ctx->Transform.ClipPlanesEnabled;
> attr->ColorMaterial = ctx->Light.ColorMaterialEnabled;
> attr->CullFace = ctx->Polygon.CullFlag;
> - attr->DepthClamp = ctx->Transform.DepthClamp;
> + attr->DepthClampNear = ctx->Transform.DepthClampNear;
> + attr->DepthClampFar = ctx->Transform.DepthClampFar;
> attr->DepthTest = ctx->Depth.Test;
> attr->Dither = ctx->Color.DitherFlag;
> attr->Fog = ctx->Fog.Enabled;
> @@ -627,8 +629,18 @@ pop_enable_group(struct gl_context *ctx, const struct
> gl_enable_attrib *enable)
> TEST_AND_UPDATE(ctx->Light.ColorMaterialEnabled, enable->ColorMaterial,
> GL_COLOR_MATERIAL);
> TEST_AND_UPDATE(ctx->Polygon.CullFlag, enable->CullFace, GL_CULL_FACE);
> - TEST_AND_UPDATE(ctx->Transform.DepthClamp, enable->DepthClamp,
> - GL_DEPTH_CLAMP);
> +
> + if (!ctx->Extensions.AMD_depth_clamp_separate) {
> + TEST_AND_UPDATE(ctx->Transform.DepthClampNear &&
> ctx->Transform.DepthClampFar,
> + enable->DepthClampNear && enable->DepthClampFar,
> + GL_DEPTH_CLAMP);
> + } else {
> + TEST_AND_UPDATE(ctx->Transform.DepthClampNear, enable->DepthClampNear,
> + GL_DEPTH_CLAMP_NEAR_AMD);
> + TEST_AND_UPDATE(ctx->Transform.DepthClampFar, enable->DepthClampFar,
> + GL_DEPTH_CLAMP_FAR_AMD);
> + }
> +
> TEST_AND_UPDATE(ctx->Depth.Test, enable->DepthTest, GL_DEPTH_TEST);
> TEST_AND_UPDATE(ctx->Color.DitherFlag, enable->Dither, GL_DITHER);
> TEST_AND_UPDATE(ctx->Fog.Enabled, enable->Fog, GL_FOG);
> @@ -1433,9 +1445,23 @@ _mesa_PopAttrib(void)
> if (xform->RescaleNormals != ctx->Transform.RescaleNormals)
> _mesa_set_enable(ctx, GL_RESCALE_NORMAL_EXT,
> ctx->Transform.RescaleNormals);
> - if (xform->DepthClamp != ctx->Transform.DepthClamp)
> - _mesa_set_enable(ctx, GL_DEPTH_CLAMP,
> - ctx->Transform.DepthClamp);
> +
> + if (!ctx->Extensions.AMD_depth_clamp_separate) {
> + if (xform->DepthClampNear != ctx->Transform.DepthClampNear
> &&
> + xform->DepthClampFar != ctx->Transform.DepthClampFar) {
> + _mesa_set_enable(ctx, GL_DEPTH_CLAMP,
> + ctx->Transform.DepthClampNear &&
> + ctx->Transform.DepthClampFar);
> + }
> + } else {
> + if (xform->DepthClampNear != ctx->Transform.DepthClampNear)
> + _mesa_set_enable(ctx, GL_DEPTH_CLAMP_NEAR_AMD,
> + ctx->Transform.DepthClampNear);
> + if (xform->DepthClampFar != ctx->Transform.DepthClampFar)
> + _mesa_set_enable(ctx, GL_DEPTH_CLAMP_FAR_AMD,
> + ctx->Transform.DepthClampFar);
> + }
> +
> if (ctx->Extensions.ARB_clip_control)
> _mesa_ClipControl(xform->ClipOrigin, xform->ClipDepthMode);
> }
> diff --git a/src/mesa/main/enable.c b/src/mesa/main/enable.c
> index d1b2f3a962..30d275075d 100644
> --- a/src/mesa/main/enable.c
> +++ b/src/mesa/main/enable.c
> @@ -1007,12 +1007,14 @@ _mesa_set_enable(struct gl_context *ctx, GLenum cap,
> GLboolean state)
> if (!_mesa_is_desktop_gl(ctx))
> goto invalid_enum_error;
> CHECK_EXTENSION(ARB_depth_clamp, cap);
> - if (ctx->Transform.DepthClamp == state)
> + if (ctx->Transform.DepthClampNear == state &&
> + ctx->Transform.DepthClampFar == state)
> return;
> FLUSH_VERTICES(ctx, ctx->DriverFlags.NewDepthClamp ? 0 :
> _NEW_TRANSFORM);
> ctx->NewDriverState |= ctx->DriverFlags.NewDepthClamp;
> - ctx->Transform.DepthClamp = state;
> + ctx->Transform.DepthClampNear = state;
> + ctx->Transform.DepthClampFar = state;
> break;
>
> case GL_FRAGMENT_SHADER_ATI:
> @@ -1684,7 +1686,8 @@ _mesa_IsEnabled( GLenum cap )
> if (!_mesa_is_desktop_gl(ctx))
> goto invalid_enum_error;
> CHECK_EXTENSION(ARB_depth_clamp);
> - return ctx->Transform.DepthClamp;
> + return ctx->Transform.DepthClampNear ||
> + ctx->Transform.DepthClampFar;
>
> case GL_FRAGMENT_SHADER_ATI:
> if (ctx->API != API_OPENGL_COMPAT)
> diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c
> index f870b217db..16625e92e2 100644
> --- a/src/mesa/main/get.c
> +++ b/src/mesa/main/get.c
> @@ -698,6 +698,10 @@ find_custom_value(struct gl_context *ctx, const struct
> value_desc *d, union valu
> v->value_int_4[3] = GET_COLORMASK_BIT(ctx->Color.ColorMask, 0, 3);
> break;
>
> + case GL_DEPTH_CLAMP:
> + v->value_bool = ctx->Transform.DepthClampNear ||
> ctx->Transform.DepthClampFar;
> + break;
> +
> case GL_EDGE_FLAG:
> v->value_bool = ctx->Current.Attrib[VERT_ATTRIB_EDGEFLAG][0] == 1.0F;
> break;
> diff --git a/src/mesa/main/get_hash_params.py
> b/src/mesa/main/get_hash_params.py
> index 5c672a3312..ae1fc6f062 100644
> --- a/src/mesa/main/get_hash_params.py
> +++ b/src/mesa/main/get_hash_params.py
> @@ -903,7 +903,7 @@ descriptor=[
> [ "DEPTH_BOUNDS_EXT", "CONTEXT_FLOAT2(Depth.BoundsMin),
> extra_EXT_depth_bounds_test" ],
>
> # GL_ARB_depth_clamp
> - [ "DEPTH_CLAMP", "CONTEXT_BOOL(Transform.DepthClamp),
> extra_ARB_depth_clamp" ],
> + [ "DEPTH_CLAMP", "LOC_CUSTOM, TYPE_BOOLEAN, 0, extra_ARB_depth_clamp" ],
>
> # GL_ATI_fragment_shader
> [ "FRAGMENT_SHADER_ATI", "CONTEXT_BOOL(ATIFragmentShader.Enabled),
> extra_ATI_fragment_shader" ],
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 3df657cc5d..eb4ccc1804 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -1282,7 +1282,6 @@ struct gl_transform_attrib
> GLboolean Normalize; /**< Normalize all
> normals? */
> GLboolean RescaleNormals; /**< GL_EXT_rescale_normal */
> GLboolean RasterPositionUnclipped; /**< GL_IBM_rasterpos_clip */
> - GLboolean DepthClamp; /**< GL_ARB_depth_clamp */
> GLboolean DepthClampNear; /**<
> GL_AMD_depth_clamp_separate */
> GLboolean DepthClampFar; /**<
> GL_AMD_depth_clamp_separate */
> /** GL_ARB_clip_control */
> diff --git a/src/mesa/main/rastpos.c b/src/mesa/main/rastpos.c
> index 1ca83c78b0..6796460639 100644
> --- a/src/mesa/main/rastpos.c
> +++ b/src/mesa/main/rastpos.c
> @@ -61,16 +61,35 @@ viewclip_point_xy( const GLfloat v[] )
>
>
> /**
> - * Clip a point against the far/near Z clipping planes.
> + * Clip a point against the near Z clipping planes.
> *
> * \param v vertex vector describing the point to clip.
> *
> * \return zero if outside view volume, or one if inside.
> */
> static GLuint
> -viewclip_point_z( const GLfloat v[] )
> +viewclip_point_near_z( const GLfloat v[] )
> {
> - if (v[2] > v[3] || v[2] < -v[3] ) {
> + if (v[2] < -v[3]) {
> + return 0;
> + }
> + else {
> + return 1;
> + }
> +}
> +
> +
> +/**
> + * Clip a point against the far Z clipping planes.
> + *
> + * \param v vertex vector describing the point to clip.
> + *
> + * \return zero if outside view volume, or one if inside.
> + */
> +static GLuint
> +viewclip_point_far_z( const GLfloat v[] )
> +{
> + if (v[2] > v[3]) {
> return 0;
> }
> else {
> @@ -389,8 +408,14 @@ _mesa_RasterPos(struct gl_context *ctx, const GLfloat
> vObj[4])
> TRANSFORM_POINT( clip, ctx->ProjectionMatrixStack.Top->m, eye );
>
> /* clip to view volume. */
> - if (!ctx->Transform.DepthClamp) {
> - if (viewclip_point_z(clip) == 0) {
> + if (!ctx->Transform.DepthClampNear) {
> + if (viewclip_point_near_z(clip) == 0) {
> + ctx->Current.RasterPosValid = GL_FALSE;
> + return;
> + }
> + }
> + if (!ctx->Transform.DepthClampFar) {
> + if (viewclip_point_far_z(clip) == 0) {
> ctx->Current.RasterPosValid = GL_FALSE;
> return;
> }
> @@ -420,10 +445,26 @@ _mesa_RasterPos(struct gl_context *ctx, const GLfloat
> vObj[4])
> ctx->Current.RasterPos[2] = ndc[2] * scale[2] + translate[2];
> ctx->Current.RasterPos[3] = clip[3];
>
> - if (ctx->Transform.DepthClamp) {
> - ctx->Current.RasterPos[3] = CLAMP(ctx->Current.RasterPos[3],
> - ctx->ViewportArray[0].Near,
> - ctx->ViewportArray[0].Far);
> + if (ctx->Transform.DepthClampNear &&
> + ctx->Transform.DepthClampFar) {
> + ctx->Current.RasterPos[3] = CLAMP(ctx->Current.RasterPos[3],
> + ctx->ViewportArray[0].Near,
> + ctx->ViewportArray[0].Far);
> + } else {
> + /* Clamp against near and far plane separately */
> + if (ctx->Transform.DepthClampNear) {
> + ctx->Current.RasterPos[3] =
> + CLAMP(ctx->Current.RasterPos[3],
> + MIN2(ctx->ViewportArray[0].Near,
> + ctx->ViewportArray[0].Far), 0);
> + }
> +
> + if (ctx->Transform.DepthClampFar) {
> + ctx->Current.RasterPos[3] =
> + CLAMP(ctx->Current.RasterPos[3],
> + 0, MAX2(ctx->ViewportArray[0].Near,
> + ctx->ViewportArray[0].Far));
> + }
I would just use:
MAX2(ctx->Current.RasterPos[3], ctx->ViewportArray[0].Near)
MIN2(ctx->Current.RasterPos[3], ctx->ViewportArray[0].Far)
for the near and far plane clamping, respectively. With that changed
and assuming you agree:
Reviewed-by: Marek Olšák <marek.ol...@amd.com>
Marek
> }
>
> /* compute raster distance */
> diff --git a/src/mesa/state_tracker/st_atom_rasterizer.c
> b/src/mesa/state_tracker/st_atom_rasterizer.c
> index 0383b8ac4a..1f66b9d059 100644
> --- a/src/mesa/state_tracker/st_atom_rasterizer.c
> +++ b/src/mesa/state_tracker/st_atom_rasterizer.c
> @@ -294,7 +294,8 @@ st_update_rasterizer(struct st_context *st)
> }
>
> /* _NEW_TRANSFORM */
> - raster->depth_clip = !ctx->Transform.DepthClamp;
> + raster->depth_clip = !(ctx->Transform.DepthClampNear &&
> + ctx->Transform.DepthClampFar);
> raster->clip_plane_enable = ctx->Transform.ClipPlanesEnabled;
> raster->clip_halfz = (ctx->Transform.ClipDepthMode == GL_ZERO_TO_ONE);
>
> diff --git a/src/mesa/state_tracker/st_cb_drawpixels.c
> b/src/mesa/state_tracker/st_cb_drawpixels.c
> index b8895684bc..67bbb35850 100644
> --- a/src/mesa/state_tracker/st_cb_drawpixels.c
> +++ b/src/mesa/state_tracker/st_cb_drawpixels.c
> @@ -677,7 +677,8 @@ draw_textured_quad(struct gl_context *ctx, GLint x, GLint
> y, GLfloat z,
> ctx->Color._ClampFragmentColor;
> rasterizer.half_pixel_center = 1;
> rasterizer.bottom_edge_rule = 1;
> - rasterizer.depth_clip = !ctx->Transform.DepthClamp;
> + rasterizer.depth_clip = !(ctx->Transform.DepthClampNear &&
> + ctx->Transform.DepthClampFar);
> rasterizer.scissor = ctx->Scissor.EnableFlags;
> cso_set_rasterizer(cso, &rasterizer);
> }
> diff --git a/src/mesa/swrast/s_span.c b/src/mesa/swrast/s_span.c
> index 87b72e8197..f50b549a97 100644
> --- a/src/mesa/swrast/s_span.c
> +++ b/src/mesa/swrast/s_span.c
> @@ -1219,7 +1219,7 @@ _swrast_write_rgba_span( struct gl_context *ctx, SWspan
> *span)
> if (!(span->arrayMask & SPAN_Z))
> _swrast_span_interpolate_z(ctx, span);
>
> - if (ctx->Transform.DepthClamp)
> + if (ctx->Transform.DepthClampNear && ctx->Transform.DepthClampFar)
> _swrast_depth_clamp_span(ctx, span);
>
> if (_mesa_stencil_is_enabled(ctx)) {
> diff --git a/src/mesa/tnl/t_vb_program.c b/src/mesa/tnl/t_vb_program.c
> index 19be5eed63..8d8aca614e 100644
> --- a/src/mesa/tnl/t_vb_program.c
> +++ b/src/mesa/tnl/t_vb_program.c
> @@ -143,7 +143,8 @@ do_ndc_cliptest(struct gl_context *ctx, struct
> vp_stage_data *store)
> store->clipmask,
> &store->ormask,
> &store->andmask,
> - !ctx->Transform.DepthClamp );
> + !(ctx->Transform.DepthClampNear &&
> + ctx->Transform.DepthClampFar)
> );
> }
> else {
> VB->NdcPtr = NULL;
> @@ -152,7 +153,8 @@ do_ndc_cliptest(struct gl_context *ctx, struct
> vp_stage_data *store)
> store->clipmask,
> &store->ormask,
> &store->andmask,
> - !ctx->Transform.DepthClamp );
> + !(ctx->Transform.DepthClampNear &&
> + ctx->Transform.DepthClampFar)
> );
> }
>
> if (store->andmask) {
> diff --git a/src/mesa/tnl/t_vb_vertex.c b/src/mesa/tnl/t_vb_vertex.c
> index 71a32b4952..4fb3659a35 100644
> --- a/src/mesa/tnl/t_vb_vertex.c
> +++ b/src/mesa/tnl/t_vb_vertex.c
> @@ -123,7 +123,7 @@ tnl_clip_prepare(struct gl_context *ctx)
> /* Neither the x86 nor sparc asm cliptest functions have been updated
> * for ARB_depth_clamp, so force the C paths.
> */
> - if (ctx->Transform.DepthClamp) {
> + if (ctx->Transform.DepthClampNear && ctx->Transform.DepthClampFar) {
> static GLboolean c_funcs_installed = GL_FALSE;
> if (!c_funcs_installed) {
> init_c_cliptest();
> @@ -191,7 +191,8 @@ static GLboolean run_vertex_stage( struct gl_context *ctx,
> store->clipmask,
> &store->ormask,
> &store->andmask,
> - !ctx->Transform.DepthClamp );
> + !(ctx->Transform.DepthClampNear &&
> + ctx->Transform.DepthClampFar)
> );
> }
> else {
> VB->NdcPtr = NULL;
> @@ -200,7 +201,8 @@ static GLboolean run_vertex_stage( struct gl_context *ctx,
> store->clipmask,
> &store->ormask,
> &store->andmask,
> - !ctx->Transform.DepthClamp );
> + !(ctx->Transform.DepthClampNear &&
> + ctx->Transform.DepthClampFar)
> );
> }
>
> if (store->andmask)
> --
> 2.17.1
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
