Bug 19712

Summary: exa: composite: driver ignores repeatType attribute
Product: xorg Reporter: Tom Jaeger <ThJaeger>
Component: Driver/RadeonAssignee: xf86-video-ati maintainers <xorg-driver-ati>
Status: RESOLVED FIXED QA Contact: Xorg Project Team <xorg-team>
Severity: normal    
Priority: medium    
Version: git   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments:
Description Flags
patch
none
Start of acceleration for RepeatPad and RepeatReflect
none
Modified test using an 8x8 source, which should hit hardware acceleration
none
Test using a 16x16 source, really hits hardware acceleration for me none

Description Tom Jaeger 2009-01-23 17:08:48 UTC
XRender supports four possible repeat types: RepeatNone, RepeatNormal, RepeatPad and RepeatReflect.  The driver currently only handles the first two, so it should fall back to software when it encounters RepeatPad or RepeatReflect.  This is currently forcing cairo to use slow client-side rendering, thus preventing firefox from enabling bilinear filtering on linux.
Comment 1 Tom Jaeger 2009-01-23 17:09:25 UTC
Created attachment 22191 [details] [review]
patch
Comment 2 Michel Dänzer 2009-01-24 02:14:02 UTC
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.
Comment 3 Michel Dänzer 2009-01-24 02:18:31 UTC
(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/ .
Comment 4 Clemens Eisserer 2009-01-24 13:16:01 UTC
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.
Comment 5 Michel Dänzer 2009-01-26 10:12:27 UTC
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?
Comment 6 Tom Jaeger 2009-01-26 11:21:19 UTC
Here's a very simple test:

http://lists.freedesktop.org/archives/xorg/2008-February/032973.html
Comment 7 Michel Dänzer 2009-01-27 00:10:14 UTC
(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. :)
Comment 8 Alex Deucher 2009-01-29 07:22:18 UTC
both r100 and r200 fail on the repeat reflect test.  OTOH, the reflect test fails even without the patch.
Comment 9 Michel Dänzer 2009-01-29 07:42:17 UTC
(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?
Comment 10 Michel Dänzer 2009-01-29 07:49:19 UTC
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. :}
Comment 11 Michel Dänzer 2009-01-29 08:07:25 UTC
Created attachment 22354 [details]
Modified test using an 8x8 source, which should hit hardware acceleration

This still passes on my RV350. :)
Comment 12 Tom Jaeger 2009-01-29 09:01:45 UTC
Sorry, I probably should have mentioned this earlier.  This is a known bug in pixman:

http://bugs.freedesktop.org/show_bug.cgi?id=19704
Comment 13 Alex Deucher 2009-01-29 11:56:18 UTC
8 isn't big enough since the EXA pitch alignment is 64 bytes.
Comment 14 Clemens Eisserer 2009-01-29 12:14:10 UTC
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.
Comment 15 Michel Dänzer 2009-01-30 00:52:34 UTC
Created attachment 22375 [details] [review]
Test using a 16x16 source, really hits hardware acceleration for me
Comment 16 Michel Dänzer 2009-01-30 01:07:42 UTC
(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. :)
Comment 17 Alex Deucher 2009-01-30 06:56:04 UTC
Both r1xx and r2xx pass the test.
Comment 18 Michel Dänzer 2009-01-30 07:23:32 UTC
(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.

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.