66461 - [libimg]stretched transparent GIFs should not eat CPU time

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Randell Jesup [:jesup] (needinfo me)]

From the footprint meeting:

Transparent GIFs (especially 1x1's) stretched into different sizes should be
optimized away - we shouldn't store a (say) 400x200 bitmap of 0's, then spend
lots of time rendering, compositing, the transparent GIF.  At least we should
save the memory, but avoiding rendering would be a significant performance win
as well.

Comment 1

[saari (gone)]

I'll take this, cc'ing pavlov too

Comment 2

[saari (gone)]

Fixing in libpr0n

Comment 3

[Chris Petersen]

*** Bug 64631 has been marked as a duplicate of this bug. ***

Comment 4

[saari (gone)]

Okay, in libpr0n we strech images at render time, we don't cache each 
scaled copy, so I'm going to close this. We could still optimize away 1x1 
transparent spacer GIFs entirely, but there isn't a big savings there. Feel 
free to open this bug again for that issue (but it won't be mozilla 0.9 
targeted)

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

Ok, thanks for getting this.  Reopening for the performance item involved -
avoiding stretching and renderering 1x1 transparent gifs would be a noticable
win (scaling is not cheap).

Removing target milestone, changing subject to CPU only, removing footprint.

Comment 6

[saari (gone)]

Okay, targeting 9.1 for totally optimizing away transparent spacers.

Comment 7

[Terri Preston]

changing status to [imglib]

Comment 8

[saari (gone)]

targeting 1.0

Comment 9

[tor]

Attachment: patch to avoid rendering spacer gifs on gtk platform

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

Very cool!  I looked over the patch; it seems quite reasonable.  This appears to
avoid drawing a transparent spacer of any size; does it avoid scaling it as
well, or has that already been handled?

Comment 11

[tor]

Scaling happens on the fly in the nsImageGTK::Draw* methods, and this patch
causes all of those to bail before doing any work on a spacer.

Comment 12

[Randell Jesup [:jesup] (needinfo me)]

I tried out the patch - it seems to work well.  The number of "alpha bitmap not
scaled" warnings is WAY down.  Everything renders fine (not surprising since we
only eliminate bitmaps with all alpha of 0).  This should be a nice little boost
in performance on complex pages.

Comment 13

[Dan Rosen]

Lots of these:

+  if ((mAlphaDepth==1) && mIsSpacer) {
+    return NS_OK;
+  }

Kill the braces around one-liners, and r=dr.

Comment 14

[tor]

Taking bug.

Comment 15

[tor]

Attachment: same + brace-be-gone

Comment 16

[tor]

Attachment: same + fix off by one error in rightmask/rightindex

Comment 17

[Dan Rosen]

r=dr

Comment 18

[Christopher Blizzard (:blizzard)]

sr=blizzard if you comment some of the bit masking

Comment 19

[tor]

Attachment: same + comments

Comment 20

[Asa Dotzler [:asa]]

a= [e-mail] for checkin to the trunk.
(on behalf of drivers)

Comment 21

[tor]

Checked in on trunk.