Bug 27305 - [bisected]piglit case texgen regressed in xserver
[bisected]piglit case texgen regressed in xserver
Status: VERIFIED FIXED
Product: xorg
Classification: Unclassified
Component: Server/General
unspecified
Other Linux (All)
: medium normal
Assigned To: Keith Packard
Xorg Project Team
: patch
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-03-25 01:18 UTC by Yi Sun
Modified: 2010-05-14 08:32 UTC (History)
3 users (show)

See Also:


Attachments
xorg.0.log (22.62 KB, text/plain)
2010-03-25 01:18 UTC, Yi Sun
no flags Details
update_fake_front_on_swap.patch (2.29 KB, patch)
2010-03-26 03:47 UTC, Francisco Jerez
no flags Details | Splinter Review
Patch without refactoring other stuff (972 bytes, patch)
2010-04-20 06:58 UTC, Kristian Høgsberg
no flags Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Yi Sun 2010-03-25 01:18:38 UTC
Created attachment 34428 [details]
xorg.0.log

Environment:
------------
Platform: G45
Mesa:           (master)99386921e778271c9b3edf90123ab6319e23fc95
Xserver:                (master)3083c5d0c4386cdd7083b7a83ac72fdad2f1e61e
Xf86_video_intel:               (master)9c037f61a490c96f9095f7ff3fecbf41f5efe9f7
Libdrm:         (master)c1c8bbf80b1f734e23996bf805dc78f32ebaf56f


Bug detailed description:
--------------------------
yesterday case texgen failed,with bisect ,we found it was caused by commit fa5103a02bd509e4a102afdad2ab26cb22210367 in xserver. On 945gme works well.  And the culprit commit is:

commit fa5103a02bd509e4a102afdad2ab26cb22210367
Author: Francisco Jerez <currojerez@riseup.net>
Date:   Wed Feb 24 23:18:01 2010 +0100

    dri2: No need to blit from front on DRI2GetBuffers if they're just being reused.

    It can be quite an expensive operation, so we're better off not doing
    it unless it's totally required.

    Signed-off-by: Francisco Jerez <currojerez@riseup.net>
    Signed-off-by: Kristian Høgsberg <krh@bitplanet.net>
    Reviewed-by: Kristian Høgsberg <krh@bitplanet.net>

Reproduce steps:
----------------
1.start X
2../texgen -auto
Comment 1 Gordon Jin 2010-03-25 19:09:45 UTC
Here's the piglit result:

test_texgen_eye: 0,0 failed

Output:
Probe at (16,16)
  Expected: 0.250000 0.250000 0.000000
  Observed: 0.000000 0.000000 0.000000
PIGLIT: {'result': 'fail' }
Comment 2 Francisco Jerez 2010-03-26 03:47:45 UTC
Created attachment 34475 [details] [review]
update_fake_front_on_swap.patch

Apparently the problem is that SwapBuffers isn't updating the fake front when a swap is completed. This wasn't a problem before because the SwapBuffers request was unconditionally followed by a GetBuffers request, but we cannot rely on it anymore.

The attached patch fixes it for me (using the nouveau DRI driver).
Comment 3 zhao jian 2010-03-29 02:32:48 UTC
(In reply to comment #2)
> Created an attachment (id=34475) [details]
> update_fake_front_on_swap.patch
> Apparently the problem is that SwapBuffers isn't updating the fake front when a
> swap is completed. This wasn't a problem before because the SwapBuffers request
> was unconditionally followed by a GetBuffers request, but we cannot rely on it
> anymore.
> The attached patch fixes it for me (using the nouveau DRI driver).

Two regressed glean cases(coloredTexPerf2, coloredLitPerf2) are also caused by this commit. 
We will try with your patch tomorrow. 
Comment 4 zhao jian 2010-03-30 05:26:18 UTC
(In reply to comment #2)
> Created an attachment (id=34475) [details]
> update_fake_front_on_swap.patch
> Apparently the problem is that SwapBuffers isn't updating the fake front when a
> swap is completed. This wasn't a problem before because the SwapBuffers request
> was unconditionally followed by a GetBuffers request, but we cannot rely on it
> anymore.
> The attached patch fixes it for me (using the nouveau DRI driver).

Yes. with this patch it also works well here. 
Comment 5 Gordon Jin 2010-04-14 01:49:28 UTC
Krh/Keith, can you review and commit?
Comment 6 Francisco Jerez 2010-04-14 02:13:11 UTC
(In reply to comment #5)
> Krh/Keith, can you review and commit?

I sent a properly formatted and cleaned up version of the patch to the ML a couple of weeks back (Kristian, you're CC'ed), please, use it instead of the attached version.
Comment 7 Kristian Høgsberg 2010-04-14 08:42:29 UTC
(In reply to comment #6)
> (In reply to comment #5)
> > Krh/Keith, can you review and commit?
> 
> I sent a properly formatted and cleaned up version of the patch to the ML a
> couple of weeks back (Kristian, you're CC'ed), please, use it instead of the
> attached version.

I reviewed it on the list and Cc'ed Keith.
Comment 8 Kristian Høgsberg 2010-04-20 06:58:21 UTC
Created attachment 35180 [details] [review]
Patch without refactoring other stuff

Here's a patch that only fixes the bug.
Comment 9 Yi Sun 2010-04-20 20:29:47 UTC
the patch works well. That case pass with it.
Comment 10 Kristian Høgsberg 2010-05-10 05:38:17 UTC
Resent the patch to the list with the changes suggested in the review:

  http://lists.x.org/archives/xorg-devel/2010-May/008306.html
Comment 11 Kristian Høgsberg 2010-05-12 09:21:21 UTC
Assigning to Keith since the list is sitting on xorg-devel waiting for him to apply it.
Comment 12 zhao jian 2010-05-14 08:32:04 UTC
It is now in xserver master now, and these three cases work well now. So verified.