Closed
Bug 665293
Opened 13 years ago
Closed 9 years ago
Firefox 5 is VERY slow rendering opacity values from javascript
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: ethernal82, Unassigned)
References
()
Details
(Keywords: perf, regression)
Attachments
(1 file)
(deleted),
patch
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:5.0) Gecko/20100101 Firefox/5.0
Build Identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:5.0) Gecko/20100101 Firefox/5.0
Firefox 5 is extremely SLOW in websites which use javascript to "play" with opacity. As you can see on http://www.emoure.com/index.php?seccion=galeria (when you open an image).
Reproducible: Always
Steps to Reproduce:
1.Enter url http://www.emoure.com/index.php?seccion=galeria
2.Open an image.
Actual Results:
Image "open" from opacity 0 to 1, but very slow.
Expected Results:
Opacity must change smoothly for good "open" effect...
Updated•13 years ago
|
Component: General → Layout
Product: Firefox → Core
QA Contact: general → layout
This is what the author wants, no?
http://www.ncreativity.com/host/moure/propios.js
<img onload=ajustar(this) ...>
function ajustar(img) //Si ya se ha cargado la imagen {
...
aparecertimer = setInterval("aparecer()", 25 );
...
function aparecer()
{
//Que aparezca suavemente
if (a >= 1 && a < 10)
{
imagen.style.opacity = '0.0'+a;
}
if (a >= 10 && a < 100)
{
imagen.style.opacity = '0.'+a;
}
...
Status: UNCONFIRMED → RESOLVED
Closed: 13 years ago
Resolution: --- → INVALID
Hmm, I have the same effect with Opera and IE8, but it's smooth and faster with Chrome (tested on winXP)
Status: RESOLVED → UNCONFIRMED
Resolution: INVALID → ---
Summary: Firefox 5 is VERY slow rendering opacity values from javascrit → Firefox 5 is VERY slow rendering opacity values from javascript
Comment 3•13 years ago
|
||
Against WinXP with D3D9, I believe to see a slight Improvement with disabling Layers Accel. Else this, it's reproducible too. Opera 11.50 b1040 renders like Chrome 14.
OS: Mac OS X → All
Version: unspecified → Trunk
j.j. exactly! that is the code and the problem with Firefox and opacity from javascript. If you calculate the number of iterations of aparecer() function are only twenty one:
1. First time a == 1
2º time a == 6
3. a== 11
4. 16
5. 21
6. 26
7. 31
8. 36
9. 41
10. 46
11. 51
12. 56
13. 61
14. 66
15. 71
16. 76
17. 81
18. 86
19. 91
20. 96
21. 101 that is the last iteration.
Ridiculous 21 iterations flood Firefox? I don't understand what's happened since core of Firefox4 was released... Chrome, Opera and Safari runs very smoothly this effect, any idea?
Obviously i talk about Chrome, Opera and Safari under Mac X os, not in windows. But the problem is there... with Firefox in any operative system since Firefox4.
I simplified the testcase and the problem go away, as you can see on: http://www.ncreativity.com/prueba/ffox/
So know the question is: What caused the problem before?
Comment 8•13 years ago
|
||
Sounds likely that too much was being repainted before...
Can we attach those testcases to the bug so they won't disappear?
Component: Layout → Layout: View Rendering
QA Contact: layout → layout.view-rendering
Comment 9•13 years ago
|
||
Enrique, does this build fix the problem for you?
https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mstange@themasta.com-7fd27d580413/try-macosx64/firefox-7.0a1.en-US.mac.dmg
Reporter | ||
Comment 10•13 years ago
|
||
(In reply to comment #9)
> Enrique, does this build fix the problem for you?
> https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mstange@themasta.
> com-7fd27d580413/try-macosx64/firefox-7.0a1.en-US.mac.dmg
Yes, that build fix the problem. So what's the problem with Firefox4/5?
What kind of Firefox release is Nightly?
Comment 11•13 years ago
|
||
Reproducible for me also on Mozilla/5.0 (Windows NT 6.1; rv:8.0a1) Gecko/20110725 Firefox/8.0a1
The pictures are loading more slowly than in Chrome, for example.
Markus, what did that build do?
Comment 13•13 years ago
|
||
What's slow here is the CopyBackground part of PushGroupAndCopyBackground. In order to paint the background (which is a CGBitmapContext) into the group surface, we need a CGImage of it, which we get using CGBitmapContextCreateImage. CGBitmapContextCreateImage promises that future changes on the bitmap context won't be reflected in the CGImage that it gives us, i.e. it has copy semantics. The docs say:
> In some cases the copy operation actually follows copy-on-write semantics, so
> that the actual physical copy of the bits occur only if the underlying data in
> the bitmap graphics context is modified.
Unfortunately it looks like we never get the copy-on-write optimization. I don't know why.
However, as far as I can tell, Cairo doesn't need the copy semantics of CGBitmapContextCreateImage because it always creates a new CGImage when the surface contents change. So we can just create our CGImage using CGBitmapContextCreateWithData with the raw CGBitmapContext data.
_cairo_quartz_create_cgimage already does exactly that, and with this patch that's where we end up.
I haven't really thought through the reference counting implications here, but I think it should work.
Attachment #550490 -
Flags: review?(jmuizelaar)
Comment 14•13 years ago
|
||
That patch should fix the Mac story. We could use bug 624090 for Windows.
Status: UNCONFIRMED → NEW
Component: Layout: View Rendering → Graphics
Ever confirmed: true
OS: All → Mac OS X
QA Contact: layout.view-rendering → thebes
Hardware: x86 → All
Updated•13 years ago
|
Attachment #550490 -
Flags: review?(ranma42)
Comment 15•13 years ago
|
||
(In reply to Markus Stange from comment #13)
> Created attachment 550490 [details] [diff] [review] [diff] [details] [review]
> don't use CGBitmapContextCreateImage
>
> What's slow here is the CopyBackground part of PushGroupAndCopyBackground.
> In order to paint the background (which is a CGBitmapContext) into the group
> surface, we need a CGImage of it, which we get using
> CGBitmapContextCreateImage. CGBitmapContextCreateImage promises that future
> changes on the bitmap context won't be reflected in the CGImage that it
> gives us, i.e. it has copy semantics. The docs say:
> > In some cases the copy operation actually follows copy-on-write semantics, so
> > that the actual physical copy of the bits occur only if the underlying data in
> > the bitmap graphics context is modified.
> Unfortunately it looks like we never get the copy-on-write optimization. I
> don't know why.
>
> However, as far as I can tell, Cairo doesn't need the copy semantics of
> CGBitmapContextCreateImage because it always creates a new CGImage when the
> surface contents change. So we can just create our CGImage using
> CGBitmapContextCreateWithData with the raw CGBitmapContext data.
> _cairo_quartz_create_cgimage already does exactly that, and with this patch
> that's where we end up.
cairo-quartz creates a new CGImage every time a surface is used as a source.
In Mozilla, this is patched to only recreate CGImages of images that change by
http://hg.mozilla.org/projects/graphics/file/27988781680a/gfx/cairo/quartz-cache-CGImageRef.patch
In order to get COW semantic and guarantee correctness even when the CGImage
isn't consumed immediately by Quartz (which seems to happen at least for
pdf quartz surfaces), a snapshot should be taken, because otherwise the CGImage
will not get copy semantics when the underlying content changes.
>
> I haven't really thought through the reference counting implications here,
> but I think it should work.
Comment 16•13 years ago
|
||
Comment on attachment 550490 [details] [diff] [review]
don't use CGBitmapContextCreateImage
Roc, what do you think about this?
Attachment #550490 -
Flags: review?(roc)
It seems to me that _cairo_surface_to_cgimage already doesn't snapshot, when the surface is not a quartz bitmap surface. It calls _cairo_surface_acquire_source_image, which doesn't snapshot in general, then calls _cairo_quartz_create_cgimage, which does CGDataProviderCreateWithData (no copy as far as I know), followed by CGImageCreate (also no copy as far as I know). Taking the fallback path in more cases won't cause a new bug if we don't already have one.
Looking at the callers to _cairo_surface_to_cgimage, none of them keep the CGImage around. So we only need the CGImage to be a snapshot if Quartz itself needs it (I can believe Andrea when he says PDF needs it), or if we're doing a self-copy operation (which is technically undefined by cairo, although I think it should work).
I don't want to lose the CGImage caching that we currently have. I think it's a significant optimization.
So I think there's a few things we need to do here:
1) Have _cairo_surface_to_cgimage know whether it needs to make a snapshot or not. Probably we should make a snapshot if it's a self-copy or the destination is not a quartz bitmap surface.
2) If we need a snapshot, make one, probably just by copying the memory buffer and freeing it in the data-provider's release callback. Or maybe we could use cairo's snapshot mechanism, I don't know much about that. Don't cache the CGImage or use the current cached image.
3) If we don't need a snapshot, use the cached CGImage if there is one, otherwise create the CGImage using the existing path that uses _cairo_quartz_create_cgimage, and cache that.
Updated•13 years ago
|
QA Contact: thebes → jmuizelaar
Comment 18•13 years ago
|
||
I could reproduce it on Mozilla/5.0 (Windows NT 5.1; rv:10.0.2) Gecko/20100101 Firefox/10.0.2 - latest public release.
Comment 19•11 years ago
|
||
This should be fixed by CoreGraphics Azure
Depends on: 896489
Flags: needinfo?
Updated•11 years ago
|
Attachment #550490 -
Flags: review?(roc)
Attachment #550490 -
Flags: review?(ranma42)
Attachment #550490 -
Flags: review?(jmuizelaar)
Comment 20•9 years ago
|
||
The reporter's URL works well for me on Windows and Linux (and seems comparable to Edge/Chrome). How's OSX these days?
Flags: needinfo? → needinfo?(mstange)
Comment 21•9 years ago
|
||
WFM on XP as well
(on my machine it's notable slower compared with Chrome but a huge improvement)
FWIW, the script's location changed but the code noted in comment 1 remained
Comment 22•9 years ago
|
||
OS X is fine as well. I agree with comment 19: this was fixed when we stopped using cairo.
Status: NEW → RESOLVED
Closed: 13 years ago → 9 years ago
Flags: needinfo?(mstange)
Resolution: --- → WORKSFORME
Updated•9 years ago
|
Keywords: testcase-wanted
You need to log in
before you can comment on or make changes to this bug.
Description
•