450400 - Moving a background image using backgroundPosition in a XUL stack is slow

Status: REOPENED

(Core :: XUL, defect)

Description

[Stuart Parmenter]

Attachment: testcase

Given that this eats 100% of my CPU I would expect it to be a lot faster.

Comment 1

[Stuart Parmenter]

roc: any idea what is going on here?

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Nothing springs to mind. If you profile it, you should check that we're not spending any time in reflow. We should be spending all our time painting. You might also want to increase the JS inter-frame delay and verify that we call nsCSSRendering::PaintBackgroundSC exactly once per call to doPhysics().

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(It runs pretty smoothly for me on Mac. A few occasional pauses, probably cycle collection :-(.)

Comment 4

[Stuart Parmenter]

jeff: can you take a look at this?

Comment 5

[Jeff Muizelaar [:jrmuizel]]

I was actually already looking at test case similar to that.

One thing that helped my test case was making the window opaque. e.g. <window style="background-color:red"... Does that make a difference for you?

I'll look at this more today.

Comment 6

[Jeff Muizelaar [:jrmuizel]]

I tried the background-color:red with your testcase, it didn't help.

Comment 7

[Jeff Muizelaar [:jrmuizel]]

I was able to get some poor profiling data and it definitely looks like the problem is x/cairo related...

Comment 8

[Jeff Muizelaar [:jrmuizel]]

Attachment: A rough patch to draw using an image surface

Here's a rough a patch does all the drawing to an image surface. There isn't support for shm yet, but it still brings the test case (with background-color:blue) from 2fps to 12fps. It also seems to break native theme drawing.

Comment 9

[Doug Turner (:dougt)]

jeff, love to try it out, but it doesn't apply cleanly to the tip.

also, why remove the MOZ_DFB code?

Comment 10

[Jeff Muizelaar [:jrmuizel]]

Attachment: Update to tip

Updated against tip. Native themes don't crash anymore either.

I removed DFB and glitz only so that I could see more clearly what was going on in the code. It's purely temporary.

Comment 11

[Jeff Muizelaar [:jrmuizel]]

Attachment: Use XShm

Updates the previous patch to actually use XShm. Improves the frame rate on my n810 to about 15fps.

Comment 12

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: force all X pixmaps to be 24bpp

Here's another approach; this forces us to use 24bpp pixmaps in many places where before we were using either the default display depth or the depth of the window, which were both 16.  Using 16bpp drawables is horrible for performance; we pay a memory cost for 24bpp, but hopefully the drawables won't be that big (given the typical display sizes), so it shouldn't be an issue.

There are still problems with this -- in particular, native theme performance is significantly improved, but it's still a massive perf hit.  I'm pretty sure we're hitting the slow path (temp xlib surface creation and alpha recovery) on most everything.

Comment 13

[Jeff Muizelaar [:jrmuizel]]

Attachment: Disable native windows

Patch disables native windows which avoids a lot of time being spent in cairo_draw_with_gdk() when drawing to an image surface like when we are using the shm.patch

Comment 14

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

I would suggest we stick the disabling block at the start of that function, inside a #ifdef MOZ_SIMPLE_NATIVE_THEME or something (or, perhaps, #ifdef GTK_SUCKS)

Comment 15

[Karl Tomlinson (:karlt)]

FWIW, seems to work fine (almost no cpu for me) with 24-bit default depth and radeon driver from xf86-video-ati-6.9.0 and EXA acceleration.

I wonder why the slow path is being hit?  Are image surfaces involved?

When seeing a problem, is the driver doing acceleration through EXA, XAA, or shadowfb?  Check /var/log/Xorg.0.log if you have that.

Comment 16

[Jeff Muizelaar [:jrmuizel]]

Attachment: bunch of stuff related to shm

includes:
- Use XShm
- arm pixman opts
- disable native windows
- offscreen image surfaces

Comment 17

[Jeff Muizelaar [:jrmuizel]]

Attachment: make sure that stride is a multiple of 4

New version of the shm stuff that actually runs on the n810. This patch fixes a BadAccess XShmPutImage error by making the shared memory region larger. I'm not positive that we need round the stride. We may only need to round the total size, but this does work.

Comment 18

[Jeff Muizelaar [:jrmuizel]]

Attachment: shm patch v1

The start of making this patch sane. No longer removes dfb and glitz stuff. Likely still breaks it.

Comment 19

[Jeff Muizelaar [:jrmuizel]]

Attachment: shm patch v2

Some more cleanup. This patch, in theory, keeps the existing code working and has the shm code selectable with a MOZ_ENABLE_SHM define.

Comment 20

[Jeff Muizelaar [:jrmuizel]]

Attachment: shm patch v3

Use bytes_per_line from ShmCreateImage instead of guessing the proper thing to do with stride.

Comment 21

[Jeff Muizelaar [:jrmuizel]]

This should work better now. I'll test it though...

Comment 22

[Jeff Muizelaar [:jrmuizel]]

Yeah, it is much better than it was.
The bug can probably be closed.

Comment 23

[(dormant account)]

I'd like to test an updated version of this patch to see if paint speed can be improved.

Comment 24

[Olli Pettay [:smaug][[e-mail]]]

Taras, you reopened a blocking1.9.1+ bug. Do you mean you want
to update the patch also for 1.9.1?

Comment 25

[(dormant account)]

I just want to experiment with the trunk