Firefox 5 is VERY slow rendering opacity values from javascript

RESOLVED WORKSFORME

Status

()

Core
Graphics
RESOLVED WORKSFORME
7 years ago
3 years ago

People

(Reporter: Enrique, Unassigned)

Tracking

({perf, regression})

Trunk
All
Mac OS X
perf, regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
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

7 years ago
Component: General → Layout
Product: Firefox → Core
QA Contact: general → layout

Comment 1

7 years ago
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
Last Resolved: 7 years ago
Resolution: --- → INVALID

Comment 2

7 years ago
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
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
(Reporter)

Comment 4

7 years ago
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?
(Reporter)

Comment 5

7 years ago
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.

Comment 6

7 years ago
this needs a simplified testcase
Keywords: regression, testcase-wanted
(Reporter)

Comment 7

7 years ago
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?
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
(Reporter)

Comment 10

7 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

7 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?
Created attachment 550490 [details] [diff] [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.

I haven't really thought through the reference counting implications here, but I think it should work.
Attachment #550490 - Flags: review?(jmuizelaar)
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
Attachment #550490 - Flags: review?(ranma42)

Comment 15

7 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 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

7 years ago
Keywords: perf

Updated

6 years ago
QA Contact: thebes → jmuizelaar

Comment 18

6 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.
Blocks: 811017
This should be fixed by CoreGraphics Azure
Depends on: 896489
Flags: needinfo?
Attachment #550490 - Flags: review?(roc)
Attachment #550490 - Flags: review?(ranma42)
Attachment #550490 - Flags: review?(jmuizelaar)
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

3 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
OS X is fine as well. I agree with comment 19: this was fixed when we stopped using cairo.
Status: NEW → RESOLVED
Last Resolved: 7 years ago3 years ago
Flags: needinfo?(mstange)
Resolution: --- → WORKSFORME
Keywords: testcase-wanted
You need to log in before you can comment on or make changes to this bug.