"Ensure frame rect is on screen or empty" - take 2

Revert commit e13b82fac077383d9f93631a177573509be44f38
(I215b312b421a30bf985f681424a4c4198f069862), and fix a different way.
The prior fix resulted in incorrectly decoding a frame whose bounds
extended beyond the canvas. Instead of changing the internal frame rect,
just change the one passed to the client.

Change-Id: I82568cb672f1027a9dc17ec0a15d355d561c356d
Reviewed-on: https://skia-review.googlesource.com/c/libgifcodec/+/353445
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>

SkGifImageReader.cpp

diff --git a/SkGifImageReader.cpp b/SkGifImageReader.cpp
index 6d37d59..f0184cb 100644
--- a/SkGifImageReader.cpp
+++ b/SkGifImageReader.cpp
@@ -767,12 +767,6 @@
             if (currentFrameIsFirstFrame()) {
                 fScreenHeight = std::max(fScreenHeight, yOffset + height);
                 fScreenWidth = std::max(fScreenWidth, xOffset + width);
-            } else {
-                // If a non-first frame is offscreen, it will have no effect on
-                // the output image. Modify its offsets to be consistent with
-                // the Wuffs implementation.
-                if (xOffset > fScreenWidth) xOffset = fScreenWidth;
-                if (yOffset > fScreenHeight) yOffset = fScreenHeight;
             }
 
             // NOTE: Chromium placed this block after setHeaderDefined, down
@@ -820,8 +814,7 @@
                 return SkCodec::kSuccess;
             }
 
-            width  = std::min(width,  fScreenWidth  - xOffset);
-            height = std::min(height, fScreenHeight - yOffset);
+
             currentFrame->setXYWH(xOffset, yOffset, width, height);
             currentFrame->setInterlaced(SkToBool(currentComponent[8] & 0x40));
 

SkLibGifCodec.cpp

diff --git a/SkLibGifCodec.cpp b/SkLibGifCodec.cpp
index b76c18f..95f5bf1 100644
--- a/SkLibGifCodec.cpp
+++ b/SkLibGifCodec.cpp
@@ -130,6 +130,18 @@
 
     if (frameInfo) {
         frameContext->fillIn(frameInfo, frameContext->isComplete());
+
+        auto* rect = &frameInfo->fFrameRect;
+        auto bounds = this->bounds();
+        if (!rect->intersect(bounds)) {
+            // If a frame is offscreen, it will have no effect on the output
+            // image. Modify its bounds to be consistent with the Wuffs
+            // implementation.
+            rect->setLTRB(std::min(rect->left(), bounds.right()),
+                          std::min(rect->top(), bounds.bottom()),
+                          std::min(rect->right(), bounds.right()),
+                          std::min(rect->bottom(), bounds.bottom()));
+        }
     }
     return true;
 }
@@ -225,9 +237,12 @@
     // This is only called by prepareToDecode, which ensures frameIndex is in range.
     SkASSERT(frame);
 
-    // Swizzler::Make only reads left and right of this rect.
-    SkIRect swizzleRect = frame->frameRect();
-    SkASSERT(swizzleRect.right() <= fReader->screenWidth());
+    const int xBegin = frame->xOffset();
+    const int xEnd = std::min(frame->frameRect().right(), fReader->screenWidth());
+
+    // CreateSwizzler only reads left and right of the frame. We cannot use the frame's raw
+    // frameRect, since it might extend beyond the edge of the frame.
+    SkIRect swizzleRect = SkIRect::MakeLTRB(xBegin, 0, xEnd, 0);
 
     SkImageInfo swizzlerInfo = dstInfo;
     if (this->colorXform()) {
@@ -404,15 +419,15 @@
     const SkGIFFrameContext* frameContext = fReader->frameContext(frameIndex);
     // The pixel data and coordinates supplied to us are relative to the frame's
     // origin within the entire image size, i.e.
-    // (frameContext->xOffset, frameContext->yOffset).
+    // (frameContext->xOffset, frameContext->yOffset). There is no guarantee
+    // that width == (size().width() - frameContext->xOffset), so
+    // we must ensure we don't run off the end of either the source data or the
+    // row's X-coordinates.
     const int width = frameContext->width();
     const int xBegin = frameContext->xOffset();
     const int yBegin = frameContext->yOffset() + rowNumber;
-    const int xEnd = xBegin + width;
+    const int xEnd = std::min(xBegin + width, this->dimensions().width());
     const int yEnd = std::min(yBegin + repeatCount, this->dimensions().height());
-
-    SkASSERT(xEnd <= this->dimensions().width());
-
     // FIXME: No need to make the checks on width/xBegin/xEnd for every row. We could instead do
     // this once in prepareToDecode.
     if (!width || (xBegin < 0) || (yBegin < 0) || (xEnd <= xBegin) || (yEnd <= yBegin))