Bugzilla – Bug 57842
r200: Culling is broken when rendering to an FBO
Last modified: 2013-01-03 18:31:24 UTC
Created attachment 70972 [details]
When rendering to an FBO with culling enabled, the r200 driver discards all geometry. As soon as GL_CULL_FACE is enabled, nothing is drawn. The glCullFace, glFrontFace and primitive orientation don't seem to make a difference. When rendering to a glx front or back buffer, culling behaves correctly.
The attached test case shows the issue. It creates an FBO, clears it with a blue color, and then draws a front- and back facing triangle in red and green respectively, and culls front-facing primitives. Afterwards the FBO texture is drawn to the window. The expected result is that the green triangle shows up. The actual result is a solid blue window.
I've had a quick look at the driver source code. r200FrontFace swaps front- and back facing geometry because the driver renders everything upside down on FBOs. This code doesn't seem to have any effect on this bug. What seems to trigger it is not inverting the viewport in r200UpdateWindow(), but I don't understand why.
The test case can be compiled with gcc fbo.c -o fbo -lGL -lglut , and started by simply running the binary without any parameters.
Hmm this is odd I can't see anything wrong with the culling setup wrt fbos.
The viewport, window handling though seems somewhat bogus.
First, r200UpdateViewportOffset() has no callers (it is used in radeon->vtbl.update_viewport_offset but that one seems to have no callers neither). This function would not take the y flip into account but since it isn't used... I'd just get rid of it but it's the only thing handling stippling that probably needs to move somewhere else.
Also, both r200UpdateViewportOffset() and r200UpdateWindow() use the driDrawable height for the yoffset in the viewport. I don't think that will work for fbos, should just use the ctx->DrawBuffer height instead?
I can't see though why that would just cause failures with culling.
Created attachment 71014 [details] [review]
potential fixes for viewport trouble
I had something like this in mind. Untested though and all that viewport updating stuff seems really weird. I hope I didn't create circular calling chains...
Might not fix this bug at all...
Created attachment 71022 [details] [review]
Fix for attachment 71014 [details] [review]
The patch needed this small fix to compile. It seems that my gcc doesn't like logical operations on floats, which kinda makes sense.
Otherwise I see no difference, but I haven't really tested polygon stipple or the scissor rect on fbos yet. Clipping is still broken.
(In reply to comment #3)
> Clipping is still broken.
Sorry, this should read "culling is still broken".
Created attachment 72047 [details]
Seems like i managed to fix it, but with dirty hack... see function in attachment. tcl somehow is not aware of wind inverting, and i just inverted commands, but it works that way :).
This fbo test case works, fbotexture and tri-cull from mesa demos - with both swtcl and tcl. What i tested, yeah Aquaria game is no more black with fbo usage and also Supertuxkart now show icons in minimap, etc.
Anyhow, maybe someone know how to make that better to be acceptible for mesa and pushed in git - i don't know better :).
I'll give the patch a try after the holidays. My I'm currently 100 miles away from my r200-equipped laptop and won't have access to it until Wednesday or Thursday.
Perhaps the message in this commit (http://cgit.freedesktop.org/mesa/mesa/commit/src/mesa/drivers/dri/r200/r200_state.c?id=60e60bb3026a269fefe1cfd3312fdf3a7e4c595f) is of note:
Tested with r300, radeon and r200 compile tested only.
Though looking at the changes of attachment 72047 [details], perhaps the other cards should be rechecked too - these changes look more like the changes made in that commit to r300, so it might not be as much of a hack as the author thought.
...minor correction: that r300 was for the non-galium driver (that was removed awhile ago), but it seems that the logic in that commit was incorrect, so the "hack" from that attachement seems to be a logic fix and most likely *is* valid for radeon/radeon_state.c.
That Dänzer's commit is good, it inverts winding properly and it works good but for swtcl only. My tries to make it work but for both swtcl and tcl, and for Stefan's test case also.
And it works for me both swtcl and tcl for those tests, but i am not very good at programming ;).
"Fixes fgl_glxgears and progs/demos/fbotexture after pressing 'c'."
Yeah that both works too "pressing 'c'" now works. Pbuffered fgl_glxgears works, but with -fbo switch - that never showing a texture on r200 :).
So i've tested: fgl_glxgears, fbotexture, supertuxkart, fbo test from this bug...
and also tri-cull to be sure no-fbo case work. And that all works for swtcl and tcl on rv280.
Created attachment 72050 [details]
STK minimap icon
He, he, i'am not programmer at all ;) but i managed to fix fogcoords, lighting and culling on rv280 with tcl this year.
Happy holidays :).
Created attachment 72051 [details] [review]
attachment 72047 [details] as patch for r200/radeon
@comment 9: your own attachment says it was not.
Anyway, attachment 72047 [details] translated into a patch.
'cull_face = (mode == GL_CW) ? R200_FFACE_CULL_CW : R200_FFACE_CULL_CCW'
is bit of a mouthful, but atm. I've got not idea how to turn that into proper obfuscation style of mesa sources.
Yeah i tried your patch and it works, thanks Raphal. Someone could pushed it in git if it's in good looking now ;).
(In reply to comment #11)
> Created attachment 72051 [details] [review] [review]
> attachment 72047 [details] as patch for r200/radeon
> @comment 9: your own attachment says it was not.
> Anyway, attachment 72047 [details] translated into a patch.
> 'cull_face = (mode == GL_CW) ? R200_FFACE_CULL_CW : R200_FFACE_CULL_CCW'
> is bit of a mouthful, but atm. I've got not idea how to turn that into
> proper obfuscation style of mesa sources.
patch looks correct to me. TCL culling is part of the tcl engine and hence ignored if there's no TCL. But viewport transform (which is where the fbo inversion happens) is done later, hence TCL culling is not subject to that inversion.
FWIW it would also be possible to simply disable TCL culling, setup culling is enough. The docs are actually saying it TCL culling is a performance optimization with questionable quality (as the coords aren't snapped to rasterizer subpixels). I guess it would also be possible to disable setup culling instead (of course only with tcl) but I'm not sure if that would make any performance compared to having enabled culling at both places.
The patch fixes the culling issues in the dx8 d3d samples as well. Smoki, can you mail the patch to the mesa-dev list? I could do it as well, but usually it's better if the author does this.
Feel free to send it Stefan. Shame on me :( i don't know how to make proper mesa patch nor how to send it :(.
Commit it to Git and use git send-email to send it to email@example.com . Google finds many tutorials on how to do this.