Closed
Bug 450916
Opened 16 years ago
Closed 16 years ago
Moving a canvas inside a stack via mouse events is very slow
Categories
(Core :: XUL, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: pavlov, Assigned: vlad)
References
Details
(Keywords: fixed1.9.1, mobile, perf)
Attachments
(3 files, 2 obsolete files)
1.59 KB,
application/x-javascript
|
Details | |
514 bytes,
application/vnd.mozilla.xul+xml
|
Details | |
11.57 KB,
patch
|
jrmuizel
:
review+
pavlov
:
superreview+
|
Details | Diff | Splinter Review |
This testcase runs fine on all my desktop machines but is horribly slow on the n810.
Reporter | ||
Comment 1•16 years ago
|
||
Reporter | ||
Comment 2•16 years ago
|
||
Assignee | ||
Comment 3•16 years ago
|
||
A few things are needed for best performance:
1) An X server built against a libpixman with both patches in bug 451621 applied.
2) Both of those patches applied to the mozilla libpixman (soon to be not needed, will get them checked in soon)
3) This patch applied to gecko
4) Running fennec with MOZ_NO_NATIVE_THEME=1 in the environment
Note that #1 isn't entirely required -- it's a good optimization, but even without it, the testcase is pretty snappy. However, I have only tested against a recent X server and relatively recent unpatched pixman; Xomap is much older than both of those.
With all this stuff (with or without #1), the testcase here is completely responsive no matter how quickly I move the mouse. Fennec feels a lot better, but it's still sluggish; I think there are some problems inside fennec itself that are causing that though, because profiles don't show us spending an unreasonable amount of time in painting (10% during my 60 second profiling run, which was constantly dragging around the content area).
This patch probably doesn't play nice with native theme itself; that should get fixed as much as possible in a followup patch, but I don't think it's possible to really fix it. Our current native theme code depends on an X visual existing that matches the drawable depth that we want; that's just not true on these systems, where we only have a single 16bpp visual (and perhaps a 32bpp visual if COMPOSITE is available; I'm not sure if we can take advantage of that). However, despite that, if we can get some kind of global MOZ_MOBILE_OPTIMIZE or something #define, we can conditionally do this stuff.
Assignee: jag → vladimir
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•16 years ago
|
||
(We could turn native theme into a pref easily enough, too, to avoid the env var business)
Assignee | ||
Comment 5•16 years ago
|
||
Adds a missing null check that otherwise causes glib warnings (harmless, in this case).
Attachment #337252 -
Attachment is obsolete: true
Reporter | ||
Comment 6•16 years ago
|
||
this bug is a lot better if you tell gtk to redraw, but we should still profile and see if this patch helps.
Flags: blocking1.9.1+
Assignee | ||
Comment 7•16 years ago
|
||
Ok, here's an updated patch -- everything sits behind one-time-checked global prefs, one pref each for:
- forcing 24bpp mode for doublebuffer surfaces
- using a single pixmap instead of recreating it each expose
- disabling native theme
Attachment #337396 -
Attachment is obsolete: true
Assignee | ||
Updated•16 years ago
|
Attachment #338904 -
Flags: superreview?(pavlov)
Attachment #338904 -
Flags: review?(jmuizelaar)
Updated•16 years ago
|
Attachment #338904 -
Flags: review?(jmuizelaar) → review+
Assignee | ||
Comment 8•16 years ago
|
||
Updated the patch slightly locally -- it now passes the GdkDrawable in as the first arg to gdk_pixmap_new, like the old code did, to make sure that if all the prefs are the default the code is identical to the previous.
Reporter | ||
Updated•16 years ago
|
Attachment #338904 -
Flags: superreview?(pavlov) → superreview+
Reporter | ||
Comment 9•16 years ago
|
||
Comment on attachment 338904 [details] [diff] [review]
Updated; use prefs for everything
The code here seems fine, but it is difficult to understand the impact of each of these prefs, and how they might interact with one another. Which ones are optimal for the situation we're in?
Comment 10•16 years ago
|
||
In theory enabling all of them should help. However more testing of each of them is probably needed.
Comment 11•16 years ago
|
||
nsWindow::GetSurfaceForGdkDrawable(GdkDrawable* aDrawable,
- const nsSize& aSize)
+ const nsSize& aSize,
+ PRInt32 aDepth)
+ switch (aDepth) {
GdkDrawables have a depth even without a GdkColormap, so you can just
switch (gdk_drawable_get_depth(aDrawable)) {
Rest looks good.
BTW, a visual for PictFormat lookup would mean that NativeThemes on displays with a RGB24 default visual wouldn't suffer with mozilla.widget.force-24bpp.
http://marc.info/?l=cairo&m=121624409609428&w=2
Comment 12•16 years ago
|
||
(In reply to comment #11)
> BTW, a visual for PictFormat lookup would mean that NativeThemes on displays
> with a RGB24 default visual wouldn't suffer with mozilla.widget.force-24bpp.
Vlad pointed out that always passing the GdkDrawable in as the
first arg to gdk_pixmap_new avoids that problem.
Assignee | ||
Comment 13•16 years ago
|
||
Checked patch in -- going to tentatively mark this as fixed...
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Updated•16 years ago
|
Keywords: fixed1.9.1
You need to log in
before you can comment on or make changes to this bug.
Description
•