Bug 9910 - fragment.position is broken on R300 but works on software mesa
Summary: fragment.position is broken on R300 but works on software mesa
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/r300 (show other bugs)
Version: git
Hardware: All All
: highest major
Assignee: Default DRI bug account
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-02-08 00:09 UTC by Oliver McFadden
Modified: 2009-08-24 12:25 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
Diff for tri-position.c (1.45 KB, patch)
2007-02-08 01:45 UTC, Oliver McFadden
Details | Splinter Review
fix up wpos handling in r300 vertex programs (2.71 KB, patch)
2007-02-08 12:03 UTC, Roland Scheidegger
Details | Splinter Review
wpos recursive fix - test1 (1.30 KB, patch)
2007-02-15 13:10 UTC, Rune Petersen
Details | Splinter Review
print out vertex program (1.22 KB, patch)
2007-02-17 10:46 UTC, Rune Petersen
Details | Splinter Review
Log of tri-position with all patches to date (1.11 KB, text/plain)
2007-02-17 12:28 UTC, Oliver McFadden
Details
Screenshot (191.82 KB, image/jpeg)
2007-02-17 12:37 UTC, Oliver McFadden
Details

Description Oliver McFadden 2007-02-08 00:09:46 UTC
You can demonstrate this with the tri-position demo (progs/fp/tri-position)
which should produce a colored triangle. On software mesa it correctly produces
a colored triangle, but on R300 it only produces a white triangle.

I'm testing this a little further back than Git master because I haven't updated
my X server from Git yet, but the bug should still occur on master.

I have no idea about why this happens, but at least this time I've got a simple
test case. :)
Comment 1 Oliver McFadden 2007-02-08 01:27:41 UTC
I just checked Mesa master and it seems this bug was fixed somewhere along the
way.

I am however seeing another problem which just prints "problem in t_dst_class"
and calls exit (0), so I'll leave this bug open for now and try to find out what
the cause of this is. I think it may be related.
Comment 2 Oliver McFadden 2007-02-08 01:44:08 UTC
I've got a simple test case for the "problem in t_dst_class" error. It seems
that when you use your own vertex program (even if it's just a simple DP4
transform) this error will occur. Without creating your own vertex program, this
error does not occur.

The "problem in t_dst_class" printf-and-exit should probably be changed to
_mesa_error, too.

Attached is the diff for tri-position.c.
Comment 3 Oliver McFadden 2007-02-08 01:45:48 UTC
Created attachment 8632 [details] [review]
Diff for tri-position.c
Comment 4 Oliver McFadden 2007-02-08 11:37:10 UTC
I tracked this down to insert_wpos (r300_vertexprog.c) with help from some people on #dri-devel.  It seems this code is broken when you specify your own vertex program. Specifically when you do the DP4 transform yourself. The following program will work fine, but performing the DP4 transform yourself results in the "problem in t_dst_class" error.  !!ARBvp1.0 OPTION ARB_position_invariant; END  Also, the following program will work fine, but will clobber the generated code. I guess some checking is required for used texcoords.  !!ARBvp1.0 OPTION ARB_position_invariant; MOV result.texcoord[0], 0; END 
Comment 5 Oliver McFadden 2007-02-08 11:38:47 UTC
Sorry, Bugzilla clobbered the formatting of my last comment for some reason.
Hopefully it's still readable.
Comment 6 Roland Scheidegger 2007-02-08 12:03:07 UTC
Created attachment 8637 [details] [review]
fix up wpos handling in r300 vertex programs

(In reply to comment #4)
> I tracked this down to insert_wpos (r300_vertexprog.c) with help from some
> people on #dri-devel.  It seems this code is broken when you specify your own
> vertex program. Specifically when you do the DP4 transform yourself.
The function adding wpos makes some assumption that all HPOS output values are in SrcReg[2] (which must additionally be a TEMP!), which might be the case on a very lucky day.
Try out the attached patch, though it's just a quick guess without any testing, so you probably need to fix some stupid bugs...
Comment 7 Oliver McFadden 2007-02-09 15:23:49 UTC
Roland already knows this as I was talking to him on #dri-devel, but for the benefit of those on the mailing list, I'll describe the current state here.  The patch has been committed, which helps things a little, but the insertion of the instructions is still buggy, and the instructions themselves are not correct. The instructions produce correct output for the tri-position program, but not nessiceraly in every circumstance, as they should. 
Comment 8 Oliver McFadden 2007-02-09 22:23:50 UTC
I think that for this to work, we either need to figure out how the blob does it, or try doing a full OpenGL viewport transform (I'm not really sure how to do this?) and see if that works correctly...  I also still think there are bugs in the insert_wpos code, as something like http://z3ro.name/tmp/oops.txt can even be generated in some cases. It seems the program gets generated multiple times, and it eventually gets corrupted; *_SAT is not valid in a vertex program.  I think this still needs a bit of work. 
Comment 9 Oliver McFadden 2007-02-14 07:21:21 UTC
Added CC for Rune Petersen as he's the author of the original function. 
Comment 10 Rune Petersen 2007-02-14 11:31:01 UTC
would you mind making a sample showing the problem, it would make it easier for me to figure what is wrong.  
Comment 11 Oliver McFadden 2007-02-14 11:37:17 UTC
I'm not quite sure what you mean?  I believe that the corruption that happens is from something going wrong and inserting the code more than once. When this happens, something might be partially overwritten and this corrupts the program.  I'm not really sure what the cause would be, though. You can see this at http://z3ro.name/tmp/oops.txt.  Note that I have added more _mesa_print_program's into the code, so there is some duplication, but you can see that gradually the program becomes corrupted.  I don't have a small demo for showing when the instructions produce incorrect results yet. I think maybe if the triangle is not screen aligned, so maybe try it at some odd angles.  Let me know if I can provide any further information. I'd really like to see this fixed and working. :) 
Comment 12 Rune Petersen 2007-02-14 14:02:46 UTC
I think I know what is wrong, I'll work on a patch...
Comment 13 Oliver McFadden 2007-02-14 14:39:38 UTC
Okay. It's probably a good idea to attach it here and I'll test it rather than directly committing. Of course, either works... 
Comment 14 Rune Petersen 2007-02-15 13:10:18 UTC
Created attachment 8740 [details] [review]
wpos recursive fix - test1
Comment 15 Rune Petersen 2007-02-15 13:11:16 UTC
take it for a spin, and se if it helps...
Comment 16 Oliver McFadden 2007-02-15 22:23:01 UTC
I just tested that patch, and I think that it avoids the corruption, but Mesa is still generating the program multiple times before finally ending up with a program that has multiple groups of the standard transform instructions (DP4) and a lot of redundant instructions.  Let me know if you need any more information. 
Comment 17 Oliver McFadden 2007-02-17 01:05:28 UTC
You should probably check that patch in, even though it doesn't seem to fix my problem completely. 
Comment 18 Rune Petersen 2007-02-17 10:46:59 UTC
Created attachment 8763 [details] [review]
print out vertex program

fits on top of wpos recursive fix - test1

please submit a new log with this patch.
Comment 19 Oliver McFadden 2007-02-17 12:28:36 UTC
Created attachment 8764 [details]
Log of tri-position with all patches to date
Comment 20 Oliver McFadden 2007-02-17 12:31:59 UTC
Hmm. It seems that this works okay now. I also checked with my code, and there
isn't any recursion issue there anymore. At least the new program printing patch
doesn't show it.

So, the only thing left now (assuming this is all correct) is to make the
instructions correct. They do not seem to work in every case. I can attach a
screenshot of this from my program to show the bug.
Comment 21 Oliver McFadden 2007-02-17 12:37:48 UTC
Created attachment 8765 [details]
Screenshot

This is a screenshot of my program based on heavily modified XreaL and Quake 3
code. This is rendering a mirror, so it should look as a mirror normally would
in the real world, however you can see it's not rendering correctly here.

Sorry for the weird angle, but the bug isn't really visible at parallel angles.

This is not a bug in my code, as this will work with the blob, but not with R300
yet.
Comment 22 Oliver McFadden 2007-02-17 12:40:54 UTC
Just to be clear; this doesn't work correctly at any angles including parallel
angles. The only difference when looking parallel to the mirror surface is that
you won't see any reflection then.

I just want to make perfectly clear that it doesn't work at any angle.
Comment 23 Rune Petersen 2007-02-17 13:10:22 UTC
I am glad the wpos problem is solved; I'll see about getting the fix committed.

About your other problem:
 - submit it as a new bug (they frown upon having more than one bug per Bug# :)
 - if you can cook the problem down to a small app that shows the bug,
   more will be able to solve it. (chances of getting fixed greater)

Oh and please CC me on the new bug.
Comment 24 Oliver McFadden 2007-02-19 15:04:27 UTC
I created the new bug report. Not sure if you've seen that yet, but I added you to the CC list. 
Comment 25 Oliver McFadden 2007-03-06 10:11:48 UTC
Changing this to fixed as the new discussion is happening in the new bug. Bug
#10024.
Comment 26 Adam Jackson 2009-08-24 12:25:45 UTC
Mass version move, cvs -> git


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.