Ensure frame rect is on screen or empty
Bug: b/160984428
There is no reason for a client to need to know that the frame rect
extends beyond the edge of the "canvas"/"screen"/image bounds. A typical
use case is more interested in knowing what part of the original bounds
the frame update occupies.
If the offsets are beyond the image bounds, move them to the
right/bottom edge of the image (and make the width/height 0). This
matches Wuffs.
Change-Id: I215b312b421a30bf985f681424a4c4198f069862
Reviewed-on: https://skia-review.googlesource.com/c/libgifcodec/+/339831
Commit-Queue: Leon Scroggins <scroggo@google.com>
Commit-Queue: Derek Sollenberger <djsollen@google.com>
Auto-Submit: Leon Scroggins <scroggo@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
diff --git a/SkGifImageReader.cpp b/SkGifImageReader.cpp
index f0184cb..6d37d59 100644
--- a/SkGifImageReader.cpp
+++ b/SkGifImageReader.cpp
@@ -767,6 +767,12 @@
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
@@ -814,7 +820,8 @@
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));
diff --git a/SkLibGifCodec.cpp b/SkLibGifCodec.cpp
index 71d0e81..b76c18f 100644
--- a/SkLibGifCodec.cpp
+++ b/SkLibGifCodec.cpp
@@ -225,12 +225,9 @@
// This is only called by prepareToDecode, which ensures frameIndex is in range.
SkASSERT(frame);
- 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);
+ // Swizzler::Make only reads left and right of this rect.
+ SkIRect swizzleRect = frame->frameRect();
+ SkASSERT(swizzleRect.right() <= fReader->screenWidth());
SkImageInfo swizzlerInfo = dstInfo;
if (this->colorXform()) {
@@ -407,15 +404,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). 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.
+ // (frameContext->xOffset, frameContext->yOffset).
const int width = frameContext->width();
const int xBegin = frameContext->xOffset();
const int yBegin = frameContext->yOffset() + rowNumber;
- const int xEnd = std::min(xBegin + width, this->dimensions().width());
+ const int xEnd = xBegin + 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))
