Bug 32945

Doesn't the performance boost come from the fact that a large part of the screen is not rendered?

Doh! Yeah, I suppose that's a lot more plausible.(In reply to comment #1)
> Doesn't the performance boost come from the fact that a large part of the
> screen is not rendered?

Doh! Yeah, I suppose that's a lot more plausible.

Does this patch fix anything?

https://bugs.freedesktop.org/attachment.cgi?id=42432

Created attachment 42458 [details]
glxgears screenshot with patch

The patch is a big improvment, but it doesn't completely solve it. 

I'm attaching a screenshot of glxgears. In Doom3 the bug seems to mostly involve the gun, any time it's moved there's a sort of black after image still visible on the screen.

Created attachment 42459 [details]
screenshot from doom3

I've made a new patch. Could you give it a try?

https://bugs.freedesktop.org/attachment.cgi?id=42464

Created attachment 42469 [details]
doom3 with second version of patch

(In reply to comment #6)
> I've made a new patch. Could you give it a try?
> 
> https://bugs.freedesktop.org/attachment.cgi?id=42464

This seems slightly worse. Not much difference in glxgears, but in Doom3 the corruption is a lot more persistent.

Not sure if it's relevant, but in glxgears, the amount of corruption depends on what angle you're viewing the gears from.

It appears that one third of the zbuffer is not cleared on your machine. In src/gallium/drivers/r300/r300_emit.c, can you add this line:

numdw = ALIGN_DIVUP(numdw * 3, 2);

at line 1171 and do some testing?

Could you also please attach your dmesg?

Created attachment 42495 [details]
dmesg output

(In reply to comment #8)
> It appears that one third of the zbuffer is not cleared on your machine. In
> src/gallium/drivers/r300/r300_emit.c, can you add this line:
> 
> numdw = ALIGN_DIVUP(numdw * 3, 2);
> 
> at line 1171 and do some testing?
> 
> Could you also please attach your dmesg?

Adding the line didn't make any difference. 

In fact, I did some poking around (without any real understanding) and the value of numdw doesn't seem to make any difference. Replacing OUT_CS(numdw); with OUT_CS(0); gives the same result here. But maybe that's expected?

dmesg is attached.

It doesn't add up. Changing numdw must make a visible difference, it determines how many pixels are cleared (multiplied by something).

I don't think 0 is even allowed. Try 1 and see how large part of the screen is rendered correctly. It should be 64x64 or something like that, I don't know for sure. Knowing that size would come in handy. The rest of the screen should be corrupted.

(In reply to comment #10)
> It doesn't add up. Changing numdw must make a visible difference, it determines
> how many pixels are cleared (multiplied by something).
> 
> I don't think 0 is even allowed. Try 1 and see how large part of the screen is
> rendered correctly. It should be 64x64 or something like that, I don't know for
> sure. Knowing that size would come in handy. The rest of the screen should be
> corrupted.

With 1 the whole screen is corrupt and mostly black - but you can still see part of the gears flickering though it's hard to catch in a screenshot.

Created attachment 42497 [details] [review]
hyperz fix v3

Thanks for your time. I've attached a new patch. It fixes more bugs and although it doesn't fix yours, it's needed for additional testing.

Please do the following.

1) Use numdw = 1.

2) Download Mesa demos if you don't already have them.
(git://anongit.freedesktop.org/mesa/demos, compile with "cmake .; make")

3) Go to src/demos.

4) Execute ./shadowtex with hyperz enabled. Press 'i'.

5) Hold 'p'. You should see flickering all over the screen, but there should be a little tile in the bottom left-hand corner. How large is the tile?

Created attachment 42527 [details]
shadowtex screenshot

Okay, so if I have done this correctly, the small white area in the left corner is 48x16, if you mean the larger black and white rendering it seems to be 256x257.

I'm attaching a screenshot of shadowtex.

I have fixed the 8x8 compression mode in master. It should fix the bottom of the screen being uninitialized. Please test.

If the latest commits don't help, please add the following line at the end of r300_chipset.c and test again:

caps->hiz_ram = 0;

No change with the latest commits, but caps->hiz_ram = 0; makes the problem go away.

Please check out the latest Mesa master branch. There are some new fixes, which should fix even this bug.

Created attachment 43961 [details]
Screenshot of ETQW

(In reply to comment #17)
> Please check out the latest Mesa master branch. There are some new fixes, which
> should fix even this bug.

Things are considerably better now. The corruption in gears and doom3 seems to be completely gone. (And the speedup in doom is around 40%!)

There's still something funky in ETQW, on the left and right sides. A screenshot is attached.

Created attachment 43962 [details]
shadowtex

Attaching new shadowtex screenshot in case that's useful.

I can't fix this by myself, because I don't have your GPU. If you want to play with it, here's how:

In mesa/src/gallium/drivers/r300, there is file r300_texture_desc.c. On lines 356 and 357, there are two arrays containing info how HiZ buffers should be aligned:

static unsigned hiz_align_x[4] = {8, 32, 48, 32};
static unsigned hiz_align_y[4] = {8, 8, 8, 32};

The 3rd element in those arrays contains a number for your GPU (because your GPU has 3 pipes). Currently the alignment is 48x8. Feel free to try a different size, like 32x32, 64x64, 48x32, whatever comes to your mind.

If nothing helps, it means the alignment is right and the problem is somewhere else.

I can see the same problems in shadowtex with a "ATI Technologies Inc R580 [Radeon X1900]" card. (MESA master)

Moreover while the comment at the start of r300_setup_hyperz_properties() says a R580 should have 4 pipes, when I print the "pipes" variable, its value is 3.

Anyway playing with hiz_align_(x|y) values doesn't change anything for me.

(In reply to comment #21)
> Anyway playing with hiz_align_(x|y) values doesn't change anything for me.

I've been doing the same, with no change in behaviour. So I guess the problem is somewhere else? :/

Created attachment 44333 [details] [review]
Possible fix

The attached patch fixes the issue with shadowtex for me. Sven, can you try it and also see if doom3 is still OK with it ?

With 3 pipes values are NPOT, so align() is not suitable in some places I think.

(In reply to comment #23)
> Created an attachment (id=44333) [details]
> Possible fix
> 
> The attached patch fixes the issue with shadowtex for me. Sven, can you try it
> and also see if doom3 is still OK with it ?
> 
> With 3 pipes values are NPOT, so align() is not suitable in some places I
> think.

Pushed, thanks a lot!

(In reply to comment #23)
> Created an attachment (id=44333) [details]
> Possible fix
> 
> The attached patch fixes the issue with shadowtex for me. Sven, can you try it
> and also see if doom3 is still OK with it ?
> 
> With 3 pipes values are NPOT, so align() is not suitable in some places I
> think.

Cool! This does indeed fix things!

The only remaining app with HiZ problems here is Lightsmark, but it seems to be a general problem tracked in bug 33648.

(In reply to comment #25)
> Cool! This does indeed fix things!

Alright, closing.