Created attachment 294782 [details] [diff] [review] V1: Clean up the clipping in drawFrameTo bug 408288 fixed a nasty issue in clipping of animation frames against the 'composition' frame. However, there are more cases to caught to prevent misuse. Also, the third parameter of DrawFrameTo is actually the rectangle of the 'source' frame (the frame being drawn) providing the size of the source, and the offset where it needs to be. The callers: DrawFrameTo(aPrevFrame, mAnim->compositingFrame, prevFrameRect); and DrawFrameTo(aNextFrame, mAnim->compositingFrame, nextFrameRect); This means that: 1. srcRect is same as the aDstRect passed as parameter 2. that 'GetRect' is thus not needed. 3. the parameters of dst.Scale are actually always 1,1. So this call is then not needed. Annotation to the patch: part 1: explain the real use of the third parameter Part 2: use NS_ENSURE_ARG_PARAM part 3: skip one GetRect part 4: move the checks for out of bounds before the if (format) as they apply for all cases part 5: use PR_MIN for code simplicity (and the compiler does smart tricks with PR_MIN). part 6. move the .Translate/.Rectangle before the blend operation (so we can remove .NewPath) part 7. only do .CurrentOperator and .SetOperator(defaultOperator) when really needed part 8. remove the .Scale as it never did anything except for some overhead
this looks right -- can we get a little extra testing on this before we land it?
Outside of regular animated GIFs and PNGs, are there any specific test images I should try?
This patch seems to be working OK. I haven't noticed any issues with animated PNGs or GIFs with it. I also looked at some of the other problematic GIFs in some of the other clipping bugs of late and they all appear to be working correctly as well.
(In reply to comment #3) > This patch seems to be working OK. I haven't noticed any issues with animated > PNGs or GIFs with it. I also looked at some of the other problematic GIFs in > some of the other clipping bugs of late and they all appear to be working > correctly as well. > Can we get those added to a test suite?
Checking in modules/libpr0n/src/imgContainer.cpp; /cvsroot/mozilla/modules/libpr0n/src/imgContainer.cpp,v <-- imgContainer.cpp new revision: 1.65; previous revision: 1.64 done