694374 - Determine if RasterImage::AdvanceFrame can be simplified

Status: NEW

(Core :: Graphics: ImageLib, defect)

Description

[Scott Johnson (:jwir3)]

There is a special case in RasterImage::AdvanceFrame (introduced in bug 666446) that looks like the following:

  if (nextFrameIndex == 0 && currentFrameIndex == 0) {
    frameToUse = nextFrame;
    aDirtyRect = &(mAnim->firstFrameRefreshArea);
  } else {
    imgFrame *curFrame = mFrames[currentFrameIndex];
    if (!curFrame) {
      return false;
    }

    // Change frame
    if (NS_FAILED(DoComposite(aDirtyRect, curFrame,
                              nextFrame, nextFrameIndex))) {
      // something went wrong, move on to next
      NS_WARNING("RasterImage::AdvanceFrame(): Compositing of frame failed");
      nextFrame->SetCompositingFailed(PR_TRUE);
      mAnim->currentAnimationFrameIndex = nextFrameIndex;
      mAnim->currentAnimationFrameTime = aTime;
      return false;
    }

    nextFrame->SetCompositingFailed(PR_FALSE);
  }

This requires special handling for the very first frame of an animation. It could probably be simplified to something like:

    imgFrame *curFrame = mFrames[currentFrameIndex];

    if (!curFrame) {
      return false;
    }

    if (!nextFrame) {
      nextFrame = curFrame;
    }

    // Change frame
    if (NS_FAILED(DoComposite(aDirtyRect, curFrame,
                              nextFrame, nextFrameIndex))) {
      // something went wrong, move on to next
      NS_WARNING("RasterImage::AdvanceFrame(): Compositing of frame failed");
      nextFrame->SetCompositingFailed(PR_TRUE);
      mAnim->currentAnimationFrameIndex = nextFrameIndex;
      mAnim->currentAnimationFrameTime = aTime;
      return false;
    }

    nextFrame->SetCompositingFailed(PR_FALSE);

This calls DoComposite() with curFrame == nextFrame, which could be problematic. We need to investigate if this can be simplified, and if so, perform this simplification.