[Mesa-dev] [PATCH 1/2] r200: fix r200 large points

[Alex Deucher]

On Mon, Nov 8, 2010 at 4:12 PM, Roland Scheidegger <sroland at vmware.com> wrote:
> DD_POINT_SIZE got never set for some time now (as it was set only in ifdefed
> out code), which caused the r200 driver to use the point primitive mistakenly
> in some cases which can only do size 1 instead of point sprite. Since the
> logic to use point instead of point sprite prim is flaky at best anyway (can't
> work correctly for per-vertex point size), just drop this and always emit point
> sprites (except for AA points) - reasons why the driver tried to use points for
> size 1.0 are unknown though it is possible they are faster or more conformant.
> Note that we can't emit point sprites without point sprite cntl as that might
> result in undefined point sizes, hence need drm version check (which was
> unnecessary before as it should always have selected points). An
> alternative would be to rely on the RE point size clamp controls which could
> clamp the size to 1.0 min/max even if the SE point size is undefined, but currently
> always use 0 for min clamp. (As a side note, this also means the driver does
> not honor the gl spec which mandates points, but not point sprites, with zero size
> to be rendered as size 1.)
> This should fix recent reports of https://bugs.freedesktop.org/show_bug.cgi?id=702.
> This is a candidate for the mesa 7.9 branch.

Looks good to me.  I suppose a drm cs checker fix would be nice for
completeness, but as long as the 3D driver emits the point sprite cntl
properly, I think it should be fine.  Are there any cases where it
wouldn't?

Reviewed-by: Alex Deucher <alexdeucher at gmail.com>

> ---
>  src/mesa/drivers/dri/r200/r200_swtcl.c |    7 +++----
>  src/mesa/drivers/dri/r200/r200_tcl.c   |    5 ++---
>  2 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/r200/r200_swtcl.c b/src/mesa/drivers/dri/r200/r200_swtcl.c
> index 3886416..c56a49d 100644
> --- a/src/mesa/drivers/dri/r200/r200_swtcl.c
> +++ b/src/mesa/drivers/dri/r200/r200_swtcl.c
> @@ -319,10 +319,9 @@ static INLINE GLuint reduced_hw_prim( struct gl_context *ctx, GLuint prim)
>  {
>    switch (prim) {
>    case GL_POINTS:
> -      return (ctx->Point.PointSprite ||
> -        ((ctx->_TriangleCaps & (DD_POINT_SIZE | DD_POINT_ATTEN)) &&
> -        !(ctx->_TriangleCaps & (DD_POINT_SMOOTH)))) ?
> -        R200_VF_PRIM_POINT_SPRITES : R200_VF_PRIM_POINTS;
> +      return (((R200_CONTEXT(ctx))->radeon.radeonScreen->drmSupportsPointSprites &&
> +              !(ctx->_TriangleCaps & DD_POINT_SMOOTH)) ?
> +        R200_VF_PRIM_POINT_SPRITES : R200_VF_PRIM_POINTS);
>    case GL_LINES:
>    /* fallthrough */
>    case GL_LINE_LOOP:
> diff --git a/src/mesa/drivers/dri/r200/r200_tcl.c b/src/mesa/drivers/dri/r200/r200_tcl.c
> index 84db7c9..7aed116 100644
> --- a/src/mesa/drivers/dri/r200/r200_tcl.c
> +++ b/src/mesa/drivers/dri/r200/r200_tcl.c
> @@ -68,9 +68,8 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
>  #define HAVE_ELTS        1
>
>
> -#define HW_POINTS           ((ctx->Point.PointSprite || \
> -                               ((ctx->_TriangleCaps & (DD_POINT_SIZE | DD_POINT_ATTEN)) && \
> -                               !(ctx->_TriangleCaps & (DD_POINT_SMOOTH)))) ? \
> +#define HW_POINTS           (((R200_CONTEXT(ctx))->radeon.radeonScreen->drmSupportsPointSprites && \
> +                             !(ctx->_TriangleCaps & DD_POINT_SMOOTH)) ? \
>                                R200_VF_PRIM_POINT_SPRITES : R200_VF_PRIM_POINTS)
>  #define HW_LINES            R200_VF_PRIM_LINES
>  #define HW_LINE_LOOP        0
> --
> 1.7.0.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>