Bug 19712

Created attachment 22191 [details] [review]
patch

Pushed, thanks.

Note that I think it should be easy to accelerate RepeatPad and RepeatReflect, as in contrast to RepeatNormal they have similar semantics to 3D APIs.

(In reply to comment #2)
> Note that I think it should be easy to accelerate RepeatPad and RepeatReflect,
> as in contrast to RepeatNormal they have similar semantics to 3D APIs.

Err, s/RepeatNormal/RepeatNone/ .

At least accalerated RepeatPad would be great, basically I think its even more important RepeatNone (if that wouldn't be default).

Mozilla is evaluating using RepeatPad for their images, and my Java2D XRender backend uses it a lot too.

Created attachment 22250 [details] [review]
Start of acceleration for RepeatPad and RepeatReflect

Here's a possible start for acceleration of RepeatPad and RepeatReflect. R100/R200 parts only compile tested, R300 parts only lightly tested - are there any simple tests for RepeatPad/Reflect?

Here's a very simple test:

http://lists.freedesktop.org/archives/xorg/2008-February/032973.html

(In reply to comment #6)
> http://lists.freedesktop.org/archives/xorg/2008-February/032973.html

Thanks for reminding me of this. It passes on my RV350. :)

both r100 and r200 fail on the repeat reflect test.  OTOH, the reflect test fails even without the patch.

(In reply to comment #8)
> both r100 and r200 fail on the repeat reflect test.

Do you see the problem in the repeat-test-reflect-out.png file generated by the test? Have you tried other *_CLAMP_S/T_MIRROR_* flags?

> OTOH, the reflect test fails even without the patch.

Hmm, so maybe something else is (also) broken on your system?

Actually, the 2x2 source used by the test is probably a fallback because it doesn't match the hardware's implicit POT texture pitch. So the problem you're seeing is probably a pixman bug, but I haven't really verified my change either. :}

Created attachment 22354 [details]
Modified test using an 8x8 source, which should hit hardware acceleration

This still passes on my RV350. :)

Sorry, I probably should have mentioned this earlier.  This is a known bug in pixman:

http://bugs.freedesktop.org/show_bug.cgi?id=19704

8 isn't big enough since the EXA pitch alignment is 64 bytes.

Some documentation about hardware restrictions would be quite cool - I am currently working on a Java2D XRender backend - and optimal performance across different driver/gpu combinations causes a lot of guessing.

Created attachment 22375 [details] [review]
Test using a 16x16 source, really hits hardware acceleration for me

(In reply to comment #13)
> 8 isn't big enough since the EXA pitch alignment is 64 bytes.

Argh. Try the 16x16 test. I also forgot this doesn't matter for >= R300, so I was hitting acceleration even with 2x2.


(In reply to comment #14)
> Some documentation about hardware restrictions would be quite cool - I am
> currently working on a Java2D XRender backend - and optimal performance across
> different driver/gpu combinations causes a lot of guessing.

In general, older hardware tends to only support repeat with power-of-two dimensions. And pre-R300 Radeons have this peculiarity where you can't choose an explicit pitch for power-of-two textures, the hardware just rounds up the width to the next multiple of 32 bytes. For other reasons, we can currently only use multiples of 64 bytes for the pitch, so if those pitches don't match (and the height is > 1) we can't do repeat. See RADEONPitchMatches() and its callers.


(In reply to comment #12)
> Sorry, I probably should have mentioned this earlier.  This is a known bug in
> pixman:
> 
> http://bugs.freedesktop.org/show_bug.cgi?id=19704

So it looks like this patch already gets us ahead of software fallbacks, at least on >= R300. :)

Both r1xx and r2xx pass the test.

(In reply to comment #17)
> Both r1xx and r2xx pass the test.

Yay! I pushed the patch, someone may want to look into extending the source tile code to handle RepeatReflect as well.