On Thu, Jan 1, 2015 at 4:58 AM, Ilia Mirkin <imir...@alum.mit.edu> wrote:
> On Thu, Jan 1, 2015 at 4:44 AM, Kenneth Graunke <kenn...@whitecape.org> wrote:
>> On Wednesday, December 31, 2014 02:38:12 AM Ilia Mirkin wrote:
>>> Nothing enables the extension yet, but the values are now available.
>>> The spec calls for it to only be exposed for GL 3.3+, which is core-only
>>> in mesa. Restrict it as such so that drivers enabling the extension
>>> don't end up accidentally exposing the function in compat contexts.
>>>
>>> Signed-off-by: Ilia Mirkin <imir...@alum.mit.edu>
>>> ---
>>> src/mesa/drivers/dri/nouveau/nouveau_state.c | 2 +-
>>> src/mesa/drivers/dri/r200/r200_state.c | 2 +-
>>> src/mesa/drivers/dri/radeon/radeon_state.c | 2 +-
>>> src/mesa/main/dd.h | 2 +-
>>> src/mesa/main/extensions.c | 1 +
>>> src/mesa/main/get.c | 1 +
>>> src/mesa/main/get_hash_params.py | 4 ++++
>>> src/mesa/main/mtypes.h | 2 ++
>>> src/mesa/main/polygon.c | 28
>>> ++++++++++++++++++++++++++--
>>> 9 files changed, 38 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/src/mesa/drivers/dri/nouveau/nouveau_state.c
>>> b/src/mesa/drivers/dri/nouveau/nouveau_state.c
>>> index db4915f..3aad10e 100644
>>> --- a/src/mesa/drivers/dri/nouveau/nouveau_state.c
>>> +++ b/src/mesa/drivers/dri/nouveau/nouveau_state.c
>>> @@ -319,7 +319,7 @@ nouveau_polygon_mode(struct gl_context *ctx, GLenum
>>> face, GLenum mode)
>>> }
>>>
>>> static void
>>> -nouveau_polygon_offset(struct gl_context *ctx, GLfloat factor, GLfloat
>>> units)
>>> +nouveau_polygon_offset(struct gl_context *ctx, GLfloat factor, GLfloat
>>> units, GLfloat clamp)
>>> {
>>> context_dirty(ctx, POLYGON_OFFSET);
>>> }
>>> diff --git a/src/mesa/drivers/dri/r200/r200_state.c
>>> b/src/mesa/drivers/dri/r200/r200_state.c
>>> index 2ad8439..930ead8 100644
>>> --- a/src/mesa/drivers/dri/r200/r200_state.c
>>> +++ b/src/mesa/drivers/dri/r200/r200_state.c
>>> @@ -712,7 +712,7 @@ static void r200ColorMask( struct gl_context *ctx,
>>> */
>>>
>>> static void r200PolygonOffset( struct gl_context *ctx,
>>> - GLfloat factor, GLfloat units )
>>> + GLfloat factor, GLfloat units, GLfloat clamp )
>>> {
>>> r200ContextPtr rmesa = R200_CONTEXT(ctx);
>>> const GLfloat depthScale = 1.0F / ctx->DrawBuffer->_DepthMaxF;
>>> diff --git a/src/mesa/drivers/dri/radeon/radeon_state.c
>>> b/src/mesa/drivers/dri/radeon/radeon_state.c
>>> index 0f4d84f..726b06d 100644
>>> --- a/src/mesa/drivers/dri/radeon/radeon_state.c
>>> +++ b/src/mesa/drivers/dri/radeon/radeon_state.c
>>> @@ -520,7 +520,7 @@ static void radeonColorMask( struct gl_context *ctx,
>>> */
>>>
>>> static void radeonPolygonOffset( struct gl_context *ctx,
>>> - GLfloat factor, GLfloat units )
>>> + GLfloat factor, GLfloat units, GLfloat clamp
>>> )
>>> {
>>> r100ContextPtr rmesa = R100_CONTEXT(ctx);
>>> const GLfloat depthScale = 1.0F / ctx->DrawBuffer->_DepthMaxF;
>>> diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h
>>> index 2f40915..6ba0959 100644
>>> --- a/src/mesa/main/dd.h
>>> +++ b/src/mesa/main/dd.h
>>> @@ -563,7 +563,7 @@ struct dd_function_table {
>>> /** Select a polygon rasterization mode */
>>> void (*PolygonMode)(struct gl_context *ctx, GLenum face, GLenum mode);
>>> /** Set the scale and units used to calculate depth values */
>>> - void (*PolygonOffset)(struct gl_context *ctx, GLfloat factor, GLfloat
>>> units);
>>> + void (*PolygonOffset)(struct gl_context *ctx, GLfloat factor, GLfloat
>>> units, GLfloat clamp);
>>> /** Set the polygon stippling pattern */
>>> void (*PolygonStipple)(struct gl_context *ctx, const GLubyte *mask );
>>> /* Specifies the current buffer for reading */
>>> diff --git a/src/mesa/main/extensions.c b/src/mesa/main/extensions.c
>>> index 0df04c2..bffa6b2 100644
>>> --- a/src/mesa/main/extensions.c
>>> +++ b/src/mesa/main/extensions.c
>>> @@ -231,6 +231,7 @@ static const struct extension extension_table[] = {
>>> { "GL_EXT_pixel_buffer_object",
>>> o(EXT_pixel_buffer_object), GL, 2004 },
>>> { "GL_EXT_point_parameters",
>>> o(EXT_point_parameters), GLL, 1997 },
>>> { "GL_EXT_polygon_offset", o(dummy_true),
>>> GLL, 1995 },
>>> + { "GL_EXT_polygon_offset_clamp",
>>> o(EXT_polygon_offset_clamp), GLC, 2014 },
>>> { "GL_EXT_provoking_vertex",
>>> o(EXT_provoking_vertex), GL, 2009 },
>>> { "GL_EXT_rescale_normal", o(dummy_true),
>>> GLL, 1997 },
>>> { "GL_EXT_secondary_color", o(dummy_true),
>>> GLL, 1999 },
>>> diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c
>>> index 6091efc..3f9d745 100644
>>> --- a/src/mesa/main/get.c
>>> +++ b/src/mesa/main/get.c
>>> @@ -392,6 +392,7 @@ EXTRA_EXT2(ARB_transform_feedback3, ARB_gpu_shader5);
>>> EXTRA_EXT(INTEL_performance_query);
>>> EXTRA_EXT(ARB_explicit_uniform_location);
>>> EXTRA_EXT(ARB_clip_control);
>>> +EXTRA_EXT(EXT_polygon_offset_clamp);
>>>
>>> static const int
>>> extra_ARB_color_buffer_float_or_glcore[] = {
>>> diff --git a/src/mesa/main/get_hash_params.py
>>> b/src/mesa/main/get_hash_params.py
>>> index 09a61ac..299ad24 100644
>>> --- a/src/mesa/main/get_hash_params.py
>>> +++ b/src/mesa/main/get_hash_params.py
>>> @@ -811,6 +811,10 @@ descriptor=[
>>> [ "VIEWPORT_BOUNDS_RANGE", "CONTEXT_FLOAT2(Const.ViewportBounds),
>>> extra_ARB_viewport_array" ],
>>> [ "LAYER_PROVOKING_VERTEX", "CONTEXT_ENUM(Light.ProvokingVertex),
>>> extra_ARB_viewport_array" ],
>>> [ "VIEWPORT_INDEX_PROVOKING_VERTEX",
>>> "CONTEXT_ENUM(Light.ProvokingVertex), extra_ARB_viewport_array" ],
>>> +
>>> +# GL_EXT_polygon_offset_clamp
>>> + [ "POLYGON_OFFSET_CLAMP_EXT", "CONTEXT_FLOAT(Polygon.OffsetUnits),
>>> extra_EXT_polygon_offset_clamp" ],
>>> +
>>> ]}
>>>
>>> ]
>>
>> You want CONTEXT_FLOAT(Polygon.OffsetClamp) here, not OffsetUnits.
>>
>> With that fixed, patches 1-3 are:
>> Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
>
> Thanks! I actually caught that, and a few other things last night...
> updated patches at https://github.com/imirkin/mesa/commits/master
>
> Things like dispatch_test failing, a missing return in the "is this
> supported" check. (I think that's it.)
>
>>
>> Patch 4 looks fine too, but I haven't verified that every Gallium driver
>> exposing core profile actually has the feature wired up.
>
> grep shows that they all seem to do it... perhaps I missed one.
>
>>
>> There are a couple of existing PolygonOffset piglit tests - perhaps those
>> could be extended to handle this? In the meantime, I dug out some of Intel's
>> tests, hacked both the tests and Mesa to let me run them in core profile,
>> and they all pass. So I suspect this is working fine.
>
> Those tests are way too complex for me to comprehend... I have an idea
> for something relatively simple -- draw a sloped quad, and a flat one
> strictly above it, then use clamps and a high slope factor to control
> whether the sloped quad appears above or below the flat one. That
> seems like it could work, right?
I've posted a piglit test which seems to pass on llvmpipe and nvc0
after a lot of fiddling (still learning how depth works). Haven't
tested it on my SNB yet.
Any opinion on whether I should just get rid of the extension bool and
start it off with dummy_false and then flip it to dummy_true at the
end of the series? Seems like all the core-supporting drivers support
it, and I suspect that will be true long-term. The only driver I'm
aware of that's in any danger of acquiring core context support is
freedreno on a3xx and a4xx. a4xx is a DX11-capable gpu, so should have
it. a3xx is unclear, but it's also pretty far away from core contexts.
There's also a potential counter-argument of making the extension
available in all GL contexts despite the extension text requiring GL
3.3. In that case we definitely need the per-driver enable (and I'd
have to propagate it as a gallium cap). I'm not sure why the extension
text requires GL 3.3 -- perhaps it's as a proxy for "DX10 GPU", in
which case it may make sense to expose it in GL 3.0 compat contexts on
those GPU's...
-ilia
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
