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)

All
macOS
defect
Not set
normal

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: ethernal82, Unassigned)

References

()

Details

(Keywords: perf, regression)

Attachments

(1 file)

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...
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
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.
this needs a simplified testcase
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
(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?
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?
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)
(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.
Keywords: perf
QA Contact: thebes → jmuizelaar
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)
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
Closed: 13 years ago9 years ago
Flags: needinfo?(mstange)
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: