Open Bug 450400 Opened 12 years ago Updated 1 year ago

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


(Core :: XUL, defect)

Not set





(Reporter: pavlov, Assigned: jrmuizel)



(Keywords: fixed1.9.1, mobile, perf)


(5 files, 6 obsolete files)

1.02 KB, application/vnd.mozilla.xul+xml
23.21 KB, patch
Details | Diff | Splinter Review
9.64 KB, patch
Details | Diff | Splinter Review
750 bytes, patch
Details | Diff | Splinter Review
17.21 KB, patch
Details | Diff | Splinter Review
Attached file testcase
Given that this eats 100% of my CPU I would expect it to be a lot faster.
roc: any idea what is going on here?
Blocks: 450297
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().
(It runs pretty smoothly for me on Mac. A few occasional pauses, probably cycle collection :-(.)
jeff: can you take a look at this?
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.
I tried the background-color:red with your testcase, it didn't help.
I was able to get some poor profiling data and it definitely looks like the problem is x/cairo related...
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.
Assignee: jag → jmuizelaar
Blocks: 436061
jeff, love to try it out, but it doesn't apply cleanly to the tip.

also, why remove the MOZ_DFB code?
Attached patch Update to tip (obsolete) — Splinter Review
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.
Attachment #334361 - Attachment is obsolete: true
Attached patch Use XShmSplinter Review
Updates the previous patch to actually use XShm. Improves the frame rate on my n810 to about 15fps.
Attachment #334520 - Attachment is obsolete: true
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.
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
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)
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.
OS: Windows Vista → Linux
Attached patch bunch of stuff related to shm (obsolete) — Splinter Review
- Use XShm
- arm pixman opts
- disable native windows
- offscreen image surfaces
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.
Attachment #336545 - Attachment is obsolete: true
Attached patch shm patch v1 (obsolete) — Splinter Review
The start of making this patch sane. No longer removes dfb and glitz stuff. Likely still breaks it.
Attachment #336701 - Attachment is obsolete: true
Attached patch shm patch v2 (obsolete) — Splinter Review
Some more cleanup. This patch, in theory, keeps the existing code working and has the shm code selectable with a MOZ_ENABLE_SHM define.
Attachment #336725 - Attachment is obsolete: true
Attached patch shm patch v3Splinter Review
Use bytes_per_line from ShmCreateImage instead of guessing the proper thing to do with stride.
Attachment #336742 - Attachment is obsolete: true
Flags: blocking1.9.1+
This should work better now. I'll test it though...
Yeah, it is much better than it was.
The bug can probably be closed.
Closed: 12 years ago
Resolution: --- → FIXED
I'd like to test an updated version of this patch to see if paint speed can be improved.
Blocks: 459117
Resolution: FIXED → ---
Taras, you reopened a blocking1.9.1+ bug. Do you mean you want
to update the patch also for 1.9.1?
I just want to experiment with the trunk
Flags: blocking1.9.1+
No longer blocks: 459117
You need to log in before you can comment on or make changes to this bug.