With Cairo, gfxImageFrame can be simplified. Parts: * Remove all alpha stuff as they don't have meaning in Cairo * Remove all Lock/Unlock pixels as they don't have meaning in Cairo * Either remove SetImageData or just make it memcpy in the pointer of GetImageData * Add a 'ImageUpdated' helper to access the ImageUpdated of mImage, so that image decoders can just do mFrame->ImageUpdated(rect); after having set pixels via the pointer from GetImageData * Set/GetBackgroundColor can be removed, as it is nowhere used * Replace 'gfxformat' with just alphadepth (0,1,8) This all to happen when codebase is fully switched to Cairo

Updated and more thorough analysis of what can be cleaned in gfxImageFrame X,Y: (readonly) Nowhere used, can be removed (but looks still usefull) SetImageData: 'depreciated' Bug 334144: /modules/libpr0n/decoders/bmp/nsICODecoder.cpp, line 83 -- nsresult nsICODecoder::SetImageData() Bug 334144: /modules/libpr0n/decoders/bmp/nsBMPDecoder.cpp, line 141 -- nsresult rv = mFrame->SetImageData(mDecoded, mBpr, line * mBpr); bug 366465: /modules/libpr0n/decoders/gif/nsGIFDecoder2.cpp, line 466 -- mImageFrame->SetImageData(mRGBLine, bpr, (aRowNumber+i)*bpr); bug 361299: /modules/libpr0n/decoders/icon/nsIconDecoder.cpp, line 148 -- mFrame->SetImageData(data, bpr, rownum * bpr); bug 376471: /modules/libpr0n/decoders/xbm/nsXBMDecoder.cpp, line 280 -- mFrame->SetImageData(mAlphaRow, abpr, mCurRow * abpr); bug 385891: /layout/generic/nsFrame.cpp, line 416 -- image->SetImageData(row_data, bpr, i*bpr); SetAlphaData: 'no use in Cairo' bug 385891: /layout/generic/nsFrame.cpp, line 415 -- image->SetAlphaData(alpha, abpr, i*abpr); GetAlphaData/GetAlphaDataLength: 'no use in Cairo' cvs remove: /modules/libpr0n/decoders/gif/imgContainerGIF.cpp, line 703 -- res = aCompositingFrame->GetAlphaData(&compositingAlphaData, bug 385883: /modules/libpr0n/src/imgRequest.cpp, line 544 -- frame->GetAlphaDataLength(&alphaSize); bug 385888: /widget/src/windows/nsWindow.cpp, line 2948 -- rv = frame->GetAlphaData(&adata, &dataLen); bug 385888: /widget/src/os2/nsWindow.cpp, line 1931 -- rv = frame->GetAlphaData(&adata, &dataLen); LockAlpha/UnlockAlphaData: 'no use in Cairo' bug 385886: /extensions/canvas3d/src/nsCanvasRenderingContextGL.cpp, line 857 -- rv |= frame->LockAlphaData(); cvs remove: /modules/libpr0n/decoders/gif/imgContainerGIF.cpp, line 702 -- aCompositingFrame->LockAlphaData(); bug 385888: /widget/src/windows/nsWindow.cpp, line 2947 -- frame->LockAlphaData(); bug 385888: /widget/src/os2/nsWindow.cpp, line 1930 -- frame->LockAlphaData(); GetBackgroundColor/SetBackgroundColor: Not used! GetAlphaBytesPerRow: Bug 334144: /modules/libpr0n/decoders/bmp/nsBMPDecoder.cpp, line 156 -- nsresult rv = mFrame->GetAlphaBytesPerRow(&abpr); cvs remove: /modules/libpr0n/decoders/gif/imgContainerGIF.cpp, line 516 -- nextFrameHasAlpha = NS_SUCCEEDED(aNextFrame->GetAlphaBytesPerRow(&aBPR)); bug 361299: /modules/libpr0n/decoders/icon/nsIconDecoder.cpp, line 139 -- mFrame->GetAlphaBytesPerRow(&abpr); bug 361299: /modules/libpr0n/decoders/icon/beos/nsIconChannel.cpp, line 239 -- PRUint32 alphaBytesPerRow = (iconSize / 8); bug 376471: /modules/libpr0n/decoders/xbm/nsXBMDecoder.cpp, line 214 -- mFrame->GetAlphaBytesPerRow(&abpr); Only used to test for transparency, should be replaced by GetFormat /modules/libpr0n/src/imgContainer.cpp, line 578 -- nextFrameHasAlpha = NS_SUCCEEDED(aNextFrame->GetAlphaBytesPerRow(&aBPR)); bug 385891: /layout/generic/nsFrame.cpp, line 380 -- image->GetAlphaBytesPerRow(&abpr); bug 385888: /widget/src/windows/nsWindow.cpp, line 2594 -- PRUint32 aAlphaBytesPerRow, bug 385888: /widget/src/windows/nsWindow.h, line 510 -- PRUint8* aAlphaData, PRUint32 aAlphaBytesPerRow,

Also DrawTo can be moved to from gfxImageFrame.idl to imgContainer.h (just like ClearFrame and CopyFrameImage are in imgContainer.cpp). This removes one function from the .IDL, as DrawTo no where used, except in imgContainer itself.

See bug 216682 for DrawTo/DrawToImage removal.

Put the correct bug for non-cairo removal for XBM decoding in dep.list.

Summary: SetImageData: 'depreciated' SetAlphaData: 'no use in Cairo' GetAlphaData/GetAlphaDataLength: 'no use in Cairo' LockAlpha/UnlockAlphaData: 'no use in Cairo' GetBackgroundColor/SetBackgroundColor: Not used! GetAlphaBytesPerRow: No use in Cairo Essential all 'alpha' things can be removed, and the Set*Data is depreciated, and Background is not used.

This cleanup saves 3K, and removes old code that no longer works (SetAlpha and friends don't work in Cairo/Thebes)

Comment on attachment 277545 [details] [diff] [review] Remove unused members from gfxImageFrame >--- gfx/src/shared/gfxImageFrame.h 23 Feb 2006 09:36:26 -0000 1.11 >+++ gfx/src/shared/gfxImageFrame.h 21 Aug 2007 15:52:34 -0000 >@@ -66,26 +66,18 @@ public: > > gfxImageFrame(); > virtual ~gfxImageFrame(); > > protected: > nsIntSize mSize; > > private: >- nsresult SetData(const PRUint8 *aData, PRUint32 aLength, >- PRInt32 aOffset, PRBool aSetAlpha); >- > nsCOMPtr<nsIImage> mImage; > > PRPackedBool mInitialized; > PRPackedBool mMutable; >- PRPackedBool mHasBackgroundColor; >- PRPackedBool mTopToBottom; > gfx_format mFormat; > >- PRInt32 mTimeout; // -1 means display forever >- nsIntPoint mOffset; >- >- gfx_color mBackgroundColor; >- >- PRInt32 mDisposalMethod; >+ PRInt32 mTimeout; // -1 means display forever >+ nsIntPoint mOffset; >+ PRInt32 mDisposalMethod; > }; Since gfx_format is an IDL "long" (why? seems like a shorter one would be fine for the eight possible values), the ordering of this struct should probably be this for minimal size: nsIntPoint mOffset; nsCOMPtr<nsIImage> mImage; gfx_format mFormat; PRInt32 mTimeout; // -1 means display forever PRInt32 mDisposalMethod; PRPackedBool mInitialized; PRPackedBool mMutable;

Comment on attachment 277545 [details] [diff] [review] Remove unused members from gfxImageFrame Previous comment should be handled in a separate bug.

Actually re-ordering won't reduce size, as all fields except the PackedBool's are (multiple of) DWORD's. Only by also changing mFormat to PRUint8 and mDisposalMethod to PRUint8 we could save about 8 bytes.

Please specify dependencies when using checkin-needed. This patch can't be checked in on its own - it at least depends on the patch in bug 391643. There's also an alphaBytesPerRow hit in nsXBMDecoder.cpp, which I didn't find a bug for, and I don't know what else.

The open bugs in the dep-list all need to be closed to checkin this patch. They are all waiting on Approval1.9 or (super)reviews. (Note bug 385886 is about Canvas3d which is not part of build, so not neccesary a blocking dependency).

Oopsie, wrong patch attach. This one is the right one!

This patch can be committed as soon as bug 376471 is committed.

Now that all 'Depends on' bugs are fixed, this one can now be checked in. Thanks in advance, Alfred

Checking in gfx/idl/gfxIImageFrame.idl; /cvsroot/mozilla/gfx/idl/gfxIImageFrame.idl,v <-- gfxIImageFrame.idl new revision: 1.17; previous revision: 1.16 done Checking in gfx/src/shared/gfxImageFrame.cpp; /cvsroot/mozilla/gfx/src/shared/gfxImageFrame.cpp,v <-- gfxImageFrame.cpp new revision: 1.41; previous revision: 1.40 done Checking in gfx/src/shared/gfxImageFrame.h; /cvsroot/mozilla/gfx/src/shared/gfxImageFrame.h,v <-- gfxImageFrame.h new revision: 1.12; previous revision: 1.11 done

Thanks! libgkgfxthebes.so Total: -1480 (+0/-1480) Code: -1357 (+0/+0) Data: -123 (+0/-1480) 1.5K codesize savings!