Bug 41603

Summary:	[bisected SNB]oglc mustpass(basic.clip) fails on Sandybirdge
Product:	Mesa	Reporter:	fangxun <xunx.fang>
Component:	Drivers/DRI/i965	Assignee:	Paul Berry <stereotype441>
Status:	VERIFIED FIXED	QA Contact:
Severity:	normal
Priority:	high	CC:	stereotype441
Version:	git
Hardware:	All
OS:	Linux (All)
Whiteboard:
i915 platform:		i915 features:

Description fangxun 2011-10-09 00:58:34 UTC

System Environment:
--------------------------
Arch:           i386
Platform:       huronriver
Libdrm:  (master)2.4.26-11-gc82ef03e4c92017bf5644f294ea04e30500f8d4c
Mesa:    (master)b861479f83ea140bfe24357d09f18a6d026d97b5
Xserver:(master)xorg-server-1.11.0-176-gfae7ed62ad476114bd37b566202cf6b6546c0a1f
Xf86_video_intel: (master)2.16.0-115-gc5414ec992d935e10156a2b513d5ec2dded2f689
Kernel: (drm-intel-next)64a742fac3a22f57303d8f1b7e347350a1c48254

Bug detailed description:
------------------------- 
Oglc userclip(basic.allCases) also fails Sandybirdge. 
Bisect shows 018ea68d8780ab5baeef0b8122b8410e5e55ae6d is the first bad commit.
commit 018ea68d8780ab5baeef0b8122b8410e5e55ae6d
Author:     Paul Berry <stereotype441@gmail.com>
AuthorDate: Tue Sep 27 12:57:08 2011 -0700
Commit:     Paul Berry <stereotype441@gmail.com>
CommitDate: Thu Oct 6 19:29:14 2011 -0700

    i965 Gen6+: De-compact clip planes.

    Previously, if the user enabled a non-consecutive set of clip planes
    (e.g. 0, 1, and 3), the driver would compact them down to a
    consecutive set starting at 0.  This optimization was of dubious
    value, and complicated the implementation of gl_ClipDistance.

    This patch changes the driver so that with Gen6 and later chipsets, we
    no longer compact the clip planes.  However, we still discard any clip
    planes beyond the highest number that is in use, so performance should
    not be affected for applications that use clip planes consecutively
    from 0.

    With chipsets previous to Gen6, we still compact the clip planes,
    since the pre-Gen6 clipper thread relies on this behavior.

    Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
    Reviewed-by: Eric Anholt <eric@anholt.net>


Reproduce steps:
----------------
1. start X
2. ./oglconform -z -s -suite all -v 2 -D 115 -test mustpass basic.clip

Comment 1 Paul Berry 2011-10-09 08:09:07 UTC

This is clearly my responsibility.  I'll investigate it ASAP.

Comment 2 Paul Berry 2011-10-10 11:53:51 UTC

Ok, I believe I've found the bug.

When I de-compacted clip planes on Gen6+, I updated both the old and new VS back-ends to reflect the change in how clip planes are stored, but I failed to change the code in gen6_vs_state.c that uploads clip plane constants when using the old VS back-end.  As a result, if the set of enabled clip planes wasn't contiguous starting with 0, then clipping would not occur properly.  This only affects the old VS back-end (which is used for fixed-function and ARB vertex programs, not for GLSL vertex shaders).

This case wasn't caught by Piglit because most of the Piglit clipping tests use GLSL vertex shaders.

I'll make a fix and add a piglit test to verify it.

Comment 3 Paul Berry 2011-10-10 16:11:53 UTC

Patch sent to Mesa-dev list for review.

Comment 4 Paul Berry 2011-10-13 08:48:41 UTC

Fixed in commit 7a4fee71aa74f0e73cf6141c08ba1d536f20cd17

    i965 Gen6+: De-compact clip plane constants for old VS backend.
    
    In commit 018ea68d8780ab5baeef0b8122b8410e5e55ae6d, when I
    de-compacted clip planes on Gen6+, I updated both the old and new VS
    back-ends to reflect the change in how clip planes are stored, but I
    failed to change the code in gen6_vs_state.c that uploads clip plane
    constants when using the old VS back-end.
    
    As a result, if the set of enabled clip planes wasn't contiguous
    starting with 0, then clipping would not occur properly.  This patch
    corrects gen6_vs_state.c to upload clip plane constants in the new
    de-compacted form.
    
    This only affects the old VS back-end (which is used for
    fixed-function and ARB vertex programs, not for GLSL vertex shaders).
    
    Fixes Piglit test fixed-clip-enables.
    
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=41603
    
    Reviewed-by: Eric Anholt <eric@anholt.net>

Comment 5 fangxun 2011-10-17 00:55:27 UTC

Verified with mesa master commit 7a4fee71aa74f0e73cf6141c08ba1d536f20cd17.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.