Hi,
I got some time to revisit the issue from December regarding quads not
following the first provoking vertex convention.
The attached patch adds PIPE_CAP_QUADS_DONT_FLATSHADE_FIRST to gallium,
which gets propagated to the return value of
glGet*(GL_QUADS_FOLLOW_PROVOKING_VERTEX_CONVENTION, ...).
Please review/push.
Marek
On Fri, Dec 18, 2009 at 10:58 AM, José Fonseca <jfons...@vmware.com> wrote:
> On Thu, 2009-12-17 at 20:41 -0800, Marek Olšák wrote:
> > Hi,
> >
> > GL_ARB_provoking_vertex states that quads are not required to abide
> > the provoking vertex convention, and the actual hardware and/or driver
> > behavior can be queried with
> > GL_QUADS_FOLLOW_PROVOKING_VERTEX_CONVENTION.
> >
> > I'd like to add a new PIPE_CAP_* to query for this capability in
> > Gallium, as it appears R3xx-R5xx hardware doesn't support the first
> > vertex convention for quads and I'd like the driver to behave
> > correctly. Fortunately, other primitive types are supported.
> >
> > I decided to use the name "quads follow flatshade_first convention"
> > instead of "provoking vertex convention" because the actual state
> > variable in pipe_rasterizer_state is called flatshade_first.
> >
> > The attached patch:
> > - adds PIPE_CAP_QUADS_FOLLOW_FLATSHADE_FIRST_CONVENTION
> > - adds the query in the Mesa state tracker
> > - and updates softpipe and llvmpipe to return 1 when this cap is
> > queried, and r300g to explicitly return 0
> >
> > Please review/push.
> >
> > Cheers.
> >
> > Marek
>
> Hi Marek,
>
> One problem I have with this patch and many of past get_param changes
> for that matter, is that it changes default behavior and forces all pipe
> drivers to be updated. It is not the first time that a get_param changes
> broke drivers because it. I happened with
> PIPE_CAP_BLEND_EQUATION_SEPARATE.
>
> As the number of drivers is increases this is a no-no. It also
> complicates writing drivers since they have to answer a large number of
> queries, most of which are specific to one or two particular hardware
> devices.
>
> IMO, there are two ways to overcome this:
>
> a) when introducing new PIPE_CAP_xxx have its semantics such that 0 will
> yield the previous behavior
>
> b) change get_param/paramf prototype so that it is passed the default
> value as an argument.
>
> That said, reading
> http://www.opengl.org/registry/specs/EXT/provoking_vertex.txt , issue #2
> (How should quads be handled?) it seems that 0 is actually a better
> default -- provoking vertex of quads does not apply to D3D and is only
> relevant for that extension. So I don't oppose for this to go in as is.
>
> But, independent of this change, lets fix the get_param/paramf calls.
> Keith, does b) above sound good to you?
>
> Jose
>
>
From b386423ae0d465ec8d38192a6b47d7b2aed6c95a Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Marek=20Ol=C5=A1=C3=A1k?= <mar...@gmail.com>
Date: Fri, 18 Dec 2009 05:10:27 +0100
Subject: [PATCH] gallium,r300g: add PIPE_CAP_QUADS_DONT_FLATSHADE_FIRST
---
src/gallium/drivers/r300/r300_screen.c | 2 ++
src/gallium/include/pipe/p_defines.h | 1 +
src/mesa/state_tracker/st_extensions.c | 3 +++
3 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/src/gallium/drivers/r300/r300_screen.c b/src/gallium/drivers/r300/r300_screen.c
index 18eecf7..5a2ec59 100644
--- a/src/gallium/drivers/r300/r300_screen.c
+++ b/src/gallium/drivers/r300/r300_screen.c
@@ -165,6 +165,8 @@ static int r300_get_param(struct pipe_screen* pscreen, int param)
case PIPE_CAP_TGSI_FS_COORD_ORIGIN_LOWER_LEFT:
case PIPE_CAP_TGSI_FS_COORD_PIXEL_CENTER_INTEGER:
return 0;
+ case PIPE_CAP_QUADS_DONT_FLATSHADE_FIRST:
+ return 1;
default:
debug_printf("r300: Implementation error: Bad param %d\n",
param);
diff --git a/src/gallium/include/pipe/p_defines.h b/src/gallium/include/pipe/p_defines.h
index 63ba311..edcd7f7 100644
--- a/src/gallium/include/pipe/p_defines.h
+++ b/src/gallium/include/pipe/p_defines.h
@@ -413,6 +413,7 @@ enum pipe_transfer_usage {
#define PIPE_CAP_TGSI_FS_COORD_ORIGIN_LOWER_LEFT 37
#define PIPE_CAP_TGSI_FS_COORD_PIXEL_CENTER_HALF_INTEGER 38
#define PIPE_CAP_TGSI_FS_COORD_PIXEL_CENTER_INTEGER 39
+#define PIPE_CAP_QUADS_DONT_FLATSHADE_FIRST 40
/**
diff --git a/src/mesa/state_tracker/st_extensions.c b/src/mesa/state_tracker/st_extensions.c
index f2a62f9..f0eb98c 100644
--- a/src/mesa/state_tracker/st_extensions.c
+++ b/src/mesa/state_tracker/st_extensions.c
@@ -124,6 +124,9 @@ void st_init_limits(struct st_context *st)
= CLAMP(screen->get_param(screen, PIPE_CAP_MAX_RENDER_TARGETS),
1, MAX_DRAW_BUFFERS);
+ c->QuadsFollowProvokingVertexConvention
+ = screen->get_param(screen, PIPE_CAP_QUADS_DONT_FLATSHADE_FIRST) ? 0 : 1;
+
/* Is TGSI_OPCODE_CONT supported? */
/* XXX separate query for early function return? */
st->ctx->Shader.EmitContReturn =
--
1.6.3.3
------------------------------------------------------------------------------
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com
_______________________________________________
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
