From ba8b248bff475629f4ab1f6a59d1737d2ff174a3 Mon Sep 17 00:00:00 2001 From: Seth Fowler Date: Wed, 4 Feb 2015 13:50:56 -0800 Subject: [PATCH] Bug 1128225 (Part 1) - Return a result enumeration from imgIContainer::Draw. r=tn --- dom/canvas/CanvasRenderingContext2D.cpp | 6 ++- image/public/imgIContainer.idl | 66 ++++++++++++++++++++++--- image/src/ClippedImage.cpp | 10 ++-- image/src/ClippedImage.h | 28 +++++------ image/src/DynamicImage.cpp | 11 ++--- image/src/FrozenImage.cpp | 2 +- image/src/FrozenImage.h | 14 +++--- image/src/ImageWrapper.cpp | 2 +- image/src/OrientedImage.cpp | 2 +- image/src/OrientedImage.h | 14 +++--- image/src/RasterImage.cpp | 38 ++++++++++---- image/src/RasterImage.h | 12 ++--- image/src/VectorImage.cpp | 34 ++++++++----- 13 files changed, 159 insertions(+), 80 deletions(-) diff --git a/dom/canvas/CanvasRenderingContext2D.cpp b/dom/canvas/CanvasRenderingContext2D.cpp index 3fd0a0935c0..a995695e514 100644 --- a/dom/canvas/CanvasRenderingContext2D.cpp +++ b/dom/canvas/CanvasRenderingContext2D.cpp @@ -4452,13 +4452,15 @@ CanvasRenderingContext2D::DrawDirectlyToCanvas( SVGImageContext svgContext(scaledImageSize, Nothing(), CurrentState().globalAlpha); - nsresult rv = image.mImgContainer-> + auto result = image.mImgContainer-> Draw(context, scaledImageSize, ImageRegion::Create(gfxRect(src.x, src.y, src.width, src.height)), image.mWhichFrame, GraphicsFilter::FILTER_GOOD, Some(svgContext), modifiedFlags); - NS_ENSURE_SUCCESS_VOID(rv); + if (result != DrawResult::SUCCESS) { + NS_WARNING("imgIContainer::Draw failed"); + } } void diff --git a/image/public/imgIContainer.idl b/image/public/imgIContainer.idl index 8731e0fcd53..437d61c88ee 100644 --- a/image/public/imgIContainer.idl +++ b/image/public/imgIContainer.idl @@ -34,13 +34,60 @@ class SVGImageContext; namespace mozilla { namespace image { + class ImageRegion; struct Orientation; + +/** + * An enumeration representing the result of a drawing operation. + * + * Most users of DrawResult will only be interested in whether the value is + * SUCCESS or not. The other values are primarily useful for debugging and error + * handling. + * + * SUCCESS: We successfully drew a completely decoded frame of the requested + * size. Drawing again with FLAG_SYNC_DECODE would not change the result. + * + * INCOMPLETE: We successfully drew a frame that was partially decoded. (Note + * that successfully drawing a partially decoded frame may not actually draw any + * pixels!) Drawing again with FLAG_SYNC_DECODE would improve the result. + * + * WRONG_SIZE: We successfully drew a wrongly-sized frame that had to be scaled. + * This is only returned if drawing again with FLAG_SYNC_DECODE would improve + * the result; if the size requested was larger than the intrinsic size of the + * image, for example, we would generally have to scale whether FLAG_SYNC_DECODE + * was specified or not, and therefore we would not return WRONG_SIZE. + * + * NOT_READY: We failed to draw because no decoded version of the image was + * available. Drawing again with FLAG_SYNC_DECODE would improve the result. + * (Though FLAG_SYNC_DECODE will not necessarily work until after the image's + * load event!) + * + * TEMPORARY_ERROR: We failed to draw due to a temporary error. Drawing may + * succeed at a later time. + * + * BAD_IMAGE: We failed to draw because the image has an error. This is a + * permanent condition. + * + * BAD_ARGS: We failed to draw because bad arguments were passed to draw(). + */ +enum class DrawResult : uint8_t +{ + SUCCESS, + INCOMPLETE, + WRONG_SIZE, + NOT_READY, + TEMPORARY_ERROR, + BAD_IMAGE, + BAD_ARGS +}; + } } %} +native DrawResult(mozilla::image::DrawResult); [ptr] native gfxContext(gfxContext); [ref] native gfxMatrix(gfxMatrix); [ref] native gfxRect(gfxRect); @@ -69,7 +116,7 @@ native nsIntSizeByVal(nsIntSize); * * Internally, imgIContainer also manages animation of images. */ -[scriptable, builtinclass, uuid(3a630c1d-3365-4873-bc01-54468decf8c6)] +[scriptable, builtinclass, uuid(4e7d0b17-cb57-4d68-860f-59b303a86dbd)] interface imgIContainer : nsISupports { /** @@ -335,14 +382,17 @@ interface imgIContainer : nsISupports * node, and the size of the viewport that the full image * would occupy. Ignored for raster images. * @param aFlags Flags of the FLAG_* variety + * @return A DrawResult value indicating whether and to what degree the + * drawing operation was successful. */ - [noscript] void draw(in gfxContext aContext, - [const] in nsIntSize aSize, - [const] in ImageRegion aRegion, - in uint32_t aWhichFrame, - in gfxGraphicsFilter aFilter, - [const] in MaybeSVGImageContext aSVGContext, - in uint32_t aFlags); + [noscript, notxpcom] DrawResult + draw(in gfxContext aContext, + [const] in nsIntSize aSize, + [const] in ImageRegion aRegion, + in uint32_t aWhichFrame, + in gfxGraphicsFilter aFilter, + [const] in MaybeSVGImageContext aSVGContext, + in uint32_t aFlags); /* * Ensures that an image is decoding. Calling this function guarantees that diff --git a/image/src/ClippedImage.cpp b/image/src/ClippedImage.cpp index 27caa70400f..53b3e217ee7 100644 --- a/image/src/ClippedImage.cpp +++ b/image/src/ClippedImage.cpp @@ -297,7 +297,7 @@ MustCreateSurface(gfxContext* aContext, return willTile || willResample; } -NS_IMETHODIMP +NS_IMETHODIMP_(DrawResult) ClippedImage::Draw(gfxContext* aContext, const nsIntSize& aSize, const ImageRegion& aRegion, @@ -318,7 +318,9 @@ ClippedImage::Draw(gfxContext* aContext, // GetFrame will call DrawSingleTile internally. RefPtr surface = GetFrameInternal(aSize, aSVGContext, aWhichFrame, aFlags); - NS_ENSURE_TRUE(surface, NS_ERROR_FAILURE); + if (!surface) { + return DrawResult::TEMPORARY_ERROR; + } // Create a drawable from that surface. nsRefPtr drawable = @@ -328,7 +330,7 @@ ClippedImage::Draw(gfxContext* aContext, gfxUtils::DrawPixelSnapped(aContext, drawable, aSize, aRegion, SurfaceFormat::B8G8R8A8, aFilter); - return NS_OK; + return DrawResult::SUCCESS; } return DrawSingleTile(aContext, aSize, aRegion, aWhichFrame, @@ -352,7 +354,7 @@ UnclipViewport(const SVGImageContext& aOldContext, aOldContext.GetPreserveAspectRatio()); } -nsresult +DrawResult ClippedImage::DrawSingleTile(gfxContext* aContext, const nsIntSize& aSize, const ImageRegion& aRegion, diff --git a/image/src/ClippedImage.h b/image/src/ClippedImage.h index f02211cfd3e..c8764b71417 100644 --- a/image/src/ClippedImage.h +++ b/image/src/ClippedImage.h @@ -39,13 +39,13 @@ public: GetFrame(uint32_t aWhichFrame, uint32_t aFlags) MOZ_OVERRIDE; NS_IMETHOD GetImageContainer(layers::LayerManager* aManager, layers::ImageContainer** _retval) MOZ_OVERRIDE; - NS_IMETHOD Draw(gfxContext* aContext, - const nsIntSize& aSize, - const ImageRegion& aRegion, - uint32_t aWhichFrame, - GraphicsFilter aFilter, - const Maybe& aSVGContext, - uint32_t aFlags) MOZ_OVERRIDE; + NS_IMETHOD_(DrawResult) Draw(gfxContext* aContext, + const nsIntSize& aSize, + const ImageRegion& aRegion, + uint32_t aWhichFrame, + GraphicsFilter aFilter, + const Maybe& aSVGContext, + uint32_t aFlags) MOZ_OVERRIDE; NS_IMETHOD RequestDiscard() MOZ_OVERRIDE; NS_IMETHOD_(Orientation) GetOrientation() MOZ_OVERRIDE; NS_IMETHOD_(nsIntRect) GetImageSpaceInvalidationRect(const nsIntRect& aRect) MOZ_OVERRIDE; @@ -66,13 +66,13 @@ private: uint32_t aWhichFrame, uint32_t aFlags); bool ShouldClip(); - nsresult DrawSingleTile(gfxContext* aContext, - const nsIntSize& aSize, - const ImageRegion& aRegion, - uint32_t aWhichFrame, - GraphicsFilter aFilter, - const Maybe& aSVGContext, - uint32_t aFlags); + DrawResult DrawSingleTile(gfxContext* aContext, + const nsIntSize& aSize, + const ImageRegion& aRegion, + uint32_t aWhichFrame, + GraphicsFilter aFilter, + const Maybe& aSVGContext, + uint32_t aFlags); // If we are forced to draw a temporary surface, we cache it here. nsAutoPtr mCachedSurface; diff --git a/image/src/DynamicImage.cpp b/image/src/DynamicImage.cpp index 78b5d083caf..cd74b546e99 100644 --- a/image/src/DynamicImage.cpp +++ b/image/src/DynamicImage.cpp @@ -186,12 +186,11 @@ DynamicImage::GetFrame(uint32_t aWhichFrame, SurfaceFormat::B8G8R8A8); nsRefPtr context = new gfxContext(dt); - nsresult rv = Draw(context, size, ImageRegion::Create(size), + auto result = Draw(context, size, ImageRegion::Create(size), aWhichFrame, GraphicsFilter::FILTER_NEAREST, Nothing(), aFlags); - NS_ENSURE_SUCCESS(rv, nullptr); - return dt->Snapshot(); + return result == DrawResult::SUCCESS ? dt->Snapshot() : nullptr; } NS_IMETHODIMP_(bool) @@ -210,7 +209,7 @@ DynamicImage::GetImageContainer(LayerManager* aManager, return NS_OK; } -NS_IMETHODIMP +NS_IMETHODIMP_(DrawResult) DynamicImage::Draw(gfxContext* aContext, const nsIntSize& aSize, const ImageRegion& aRegion, @@ -226,7 +225,7 @@ DynamicImage::Draw(gfxContext* aContext, if (aSize == drawableSize) { gfxUtils::DrawPixelSnapped(aContext, mDrawable, drawableSize, aRegion, SurfaceFormat::B8G8R8A8, aFilter); - return NS_OK; + return DrawResult::SUCCESS; } gfxSize scale(double(aSize.width) / drawableSize.width, @@ -240,7 +239,7 @@ DynamicImage::Draw(gfxContext* aContext, gfxUtils::DrawPixelSnapped(aContext, mDrawable, drawableSize, region, SurfaceFormat::B8G8R8A8, aFilter); - return NS_OK; + return DrawResult::SUCCESS; } NS_IMETHODIMP diff --git a/image/src/FrozenImage.cpp b/image/src/FrozenImage.cpp index ec3169fb97a..5da2a109adb 100644 --- a/image/src/FrozenImage.cpp +++ b/image/src/FrozenImage.cpp @@ -58,7 +58,7 @@ FrozenImage::GetImageContainer(layers::LayerManager* aManager, return NS_OK; } -NS_IMETHODIMP +NS_IMETHODIMP_(DrawResult) FrozenImage::Draw(gfxContext* aContext, const nsIntSize& aSize, const ImageRegion& aRegion, diff --git a/image/src/FrozenImage.h b/image/src/FrozenImage.h index c9987ddcf23..e504e523114 100644 --- a/image/src/FrozenImage.h +++ b/image/src/FrozenImage.h @@ -39,13 +39,13 @@ public: GetFrame(uint32_t aWhichFrame, uint32_t aFlags) MOZ_OVERRIDE; NS_IMETHOD GetImageContainer(layers::LayerManager* aManager, layers::ImageContainer** _retval) MOZ_OVERRIDE; - NS_IMETHOD Draw(gfxContext* aContext, - const nsIntSize& aSize, - const ImageRegion& aRegion, - uint32_t aWhichFrame, - GraphicsFilter aFilter, - const Maybe& aSVGContext, - uint32_t aFlags) MOZ_OVERRIDE; + NS_IMETHOD_(DrawResult) Draw(gfxContext* aContext, + const nsIntSize& aSize, + const ImageRegion& aRegion, + uint32_t aWhichFrame, + GraphicsFilter aFilter, + const Maybe& aSVGContext, + uint32_t aFlags) MOZ_OVERRIDE; NS_IMETHOD_(void) RequestRefresh(const TimeStamp& aTime) MOZ_OVERRIDE; NS_IMETHOD GetAnimationMode(uint16_t* aAnimationMode) MOZ_OVERRIDE; NS_IMETHOD SetAnimationMode(uint16_t aAnimationMode) MOZ_OVERRIDE; diff --git a/image/src/ImageWrapper.cpp b/image/src/ImageWrapper.cpp index f375f25aa01..2ddc0f68878 100644 --- a/image/src/ImageWrapper.cpp +++ b/image/src/ImageWrapper.cpp @@ -199,7 +199,7 @@ ImageWrapper::GetImageContainer(LayerManager* aManager, return mInnerImage->GetImageContainer(aManager, _retval); } -NS_IMETHODIMP +NS_IMETHODIMP_(DrawResult) ImageWrapper::Draw(gfxContext* aContext, const nsIntSize& aSize, const ImageRegion& aRegion, diff --git a/image/src/OrientedImage.cpp b/image/src/OrientedImage.cpp index 20db2f51764..7fea526ca67 100644 --- a/image/src/OrientedImage.cpp +++ b/image/src/OrientedImage.cpp @@ -251,7 +251,7 @@ OrientViewport(const SVGImageContext& aOldContext, aOldContext.GetPreserveAspectRatio()); } -NS_IMETHODIMP +NS_IMETHODIMP_(DrawResult) OrientedImage::Draw(gfxContext* aContext, const nsIntSize& aSize, const ImageRegion& aRegion, diff --git a/image/src/OrientedImage.h b/image/src/OrientedImage.h index 244c3c7ad74..62f53d2c8ed 100644 --- a/image/src/OrientedImage.h +++ b/image/src/OrientedImage.h @@ -36,13 +36,13 @@ public: GetFrame(uint32_t aWhichFrame, uint32_t aFlags) MOZ_OVERRIDE; NS_IMETHOD GetImageContainer(layers::LayerManager* aManager, layers::ImageContainer** _retval) MOZ_OVERRIDE; - NS_IMETHOD Draw(gfxContext* aContext, - const nsIntSize& aSize, - const ImageRegion& aRegion, - uint32_t aWhichFrame, - GraphicsFilter aFilter, - const Maybe& aSVGContext, - uint32_t aFlags) MOZ_OVERRIDE; + NS_IMETHOD_(DrawResult) Draw(gfxContext* aContext, + const nsIntSize& aSize, + const ImageRegion& aRegion, + uint32_t aWhichFrame, + GraphicsFilter aFilter, + const Maybe& aSVGContext, + uint32_t aFlags) MOZ_OVERRIDE; NS_IMETHOD_(nsIntRect) GetImageSpaceInvalidationRect( const nsIntRect& aRect) MOZ_OVERRIDE; nsIntSize OptimalImageSizeForDest(const gfxSize& aDest, diff --git a/image/src/RasterImage.cpp b/image/src/RasterImage.cpp index 3f96f6687ce..9a9c8a7281b 100644 --- a/image/src/RasterImage.cpp +++ b/image/src/RasterImage.cpp @@ -1699,7 +1699,7 @@ RasterImage::RequestScale(imgFrame* aFrame, } } -void +DrawResult RasterImage::DrawWithPreDownscaleIfNeeded(DrawableFrameRef&& aFrameRef, gfxContext* aContext, const nsIntSize& aSize, @@ -1728,23 +1728,39 @@ RasterImage::DrawWithPreDownscaleIfNeeded(DrawableFrameRef&& aFrameRef, gfxContextMatrixAutoSaveRestore saveMatrix(aContext); ImageRegion region(aRegion); + bool frameIsComplete = true; // We already checked HQ scaled frames. if (!frameRef) { + // There's no HQ scaled frame available, so we'll have to use the frame + // provided by the caller. frameRef = Move(aFrameRef); + frameIsComplete = frameRef->IsImageComplete(); } // By now we may have a frame with the requested size. If not, we need to // adjust the drawing parameters accordingly. IntSize finalSize = frameRef->GetImageSize(); + bool couldRedecodeForBetterFrame = false; if (ThebesIntSize(finalSize) != aSize) { gfx::Size scale(double(aSize.width) / finalSize.width, double(aSize.height) / finalSize.height); aContext->Multiply(gfxMatrix::Scaling(scale.width, scale.height)); region.Scale(1.0 / scale.width, 1.0 / scale.height); + + couldRedecodeForBetterFrame = mDownscaleDuringDecode && + CanDownscaleDuringDecode(aSize, aFlags); } if (!frameRef->Draw(aContext, region, aFilter, aFlags)) { RecoverFromLossOfFrames(aSize, aFlags); + return DrawResult::TEMPORARY_ERROR; } + if (!frameIsComplete) { + return DrawResult::INCOMPLETE; + } + if (couldRedecodeForBetterFrame) { + return DrawResult::WRONG_SIZE; + } + return DrawResult::SUCCESS; } //****************************************************************************** @@ -1757,7 +1773,7 @@ RasterImage::DrawWithPreDownscaleIfNeeded(DrawableFrameRef&& aFrameRef, * [const] in SVGImageContext aSVGContext, * in uint32_t aWhichFrame, * in uint32_t aFlags); */ -NS_IMETHODIMP +NS_IMETHODIMP_(DrawResult) RasterImage::Draw(gfxContext* aContext, const nsIntSize& aSize, const ImageRegion& aRegion, @@ -1767,18 +1783,20 @@ RasterImage::Draw(gfxContext* aContext, uint32_t aFlags) { if (aWhichFrame > FRAME_MAX_VALUE) - return NS_ERROR_INVALID_ARG; + return DrawResult::BAD_ARGS; if (mError) - return NS_ERROR_FAILURE; + return DrawResult::BAD_IMAGE; // Illegal -- you can't draw with non-default decode flags. // (Disabling colorspace conversion might make sense to allow, but // we don't currently.) if (DecodeFlags(aFlags) != DECODE_FLAGS_DEFAULT) - return NS_ERROR_FAILURE; + return DrawResult::BAD_ARGS; - NS_ENSURE_ARG_POINTER(aContext); + if (!aContext) { + return DrawResult::BAD_ARGS; + } if (IsUnlocked() && mProgressTracker) { mProgressTracker->OnUnlockedDraw(); @@ -1797,14 +1815,14 @@ RasterImage::Draw(gfxContext* aContext, if (mDrawStartTime.IsNull()) { mDrawStartTime = TimeStamp::Now(); } - return NS_OK; + return DrawResult::NOT_READY; } bool shouldRecordTelemetry = !mDrawStartTime.IsNull() && ref->IsImageComplete(); - DrawWithPreDownscaleIfNeeded(Move(ref), aContext, aSize, - aRegion, aFilter, flags); + auto result = DrawWithPreDownscaleIfNeeded(Move(ref), aContext, aSize, + aRegion, aFilter, flags); if (shouldRecordTelemetry) { TimeDuration drawLatency = TimeStamp::Now() - mDrawStartTime; @@ -1813,7 +1831,7 @@ RasterImage::Draw(gfxContext* aContext, mDrawStartTime = TimeStamp(); } - return NS_OK; + return result; } //****************************************************************************** diff --git a/image/src/RasterImage.h b/image/src/RasterImage.h index d152ccface9..6c0072f8cbb 100644 --- a/image/src/RasterImage.h +++ b/image/src/RasterImage.h @@ -288,12 +288,12 @@ public: } private: - void DrawWithPreDownscaleIfNeeded(DrawableFrameRef&& aFrameRef, - gfxContext* aContext, - const nsIntSize& aSize, - const ImageRegion& aRegion, - GraphicsFilter aFilter, - uint32_t aFlags); + DrawResult DrawWithPreDownscaleIfNeeded(DrawableFrameRef&& aFrameRef, + gfxContext* aContext, + const nsIntSize& aSize, + const ImageRegion& aRegion, + GraphicsFilter aFilter, + uint32_t aFlags); TemporaryRef CopyFrame(uint32_t aWhichFrame, uint32_t aFlags); diff --git a/image/src/VectorImage.cpp b/image/src/VectorImage.cpp index 86179f73d04..5841734a45c 100644 --- a/image/src/VectorImage.cpp +++ b/image/src/VectorImage.cpp @@ -687,13 +687,12 @@ VectorImage::GetFrame(uint32_t aWhichFrame, nsRefPtr context = new gfxContext(dt); - nsresult rv = Draw(context, imageIntSize, + auto result = Draw(context, imageIntSize, ImageRegion::Create(imageIntSize), aWhichFrame, GraphicsFilter::FILTER_NEAREST, Nothing(), aFlags); - NS_ENSURE_SUCCESS(rv, nullptr); - return dt->Snapshot(); + return result == DrawResult::SUCCESS ? dt->Snapshot() : nullptr; } //****************************************************************************** @@ -747,7 +746,7 @@ struct SVGDrawingParameters * in gfxGraphicsFilter aFilter, * [const] in MaybeSVGImageContext aSVGContext, * in uint32_t aFlags); */ -NS_IMETHODIMP +NS_IMETHODIMP_(DrawResult) VectorImage::Draw(gfxContext* aContext, const nsIntSize& aSize, const ImageRegion& aRegion, @@ -756,16 +755,25 @@ VectorImage::Draw(gfxContext* aContext, const Maybe& aSVGContext, uint32_t aFlags) { - if (aWhichFrame > FRAME_MAX_VALUE) - return NS_ERROR_INVALID_ARG; + if (aWhichFrame > FRAME_MAX_VALUE) { + return DrawResult::BAD_ARGS; + } - NS_ENSURE_ARG_POINTER(aContext); - if (mError || !mIsFullyLoaded) - return NS_ERROR_FAILURE; + if (!aContext) { + return DrawResult::BAD_ARGS; + } + + if (mError) { + return DrawResult::BAD_IMAGE; + } + + if (!mIsFullyLoaded) { + return DrawResult::NOT_READY; + } if (mIsDrawing) { NS_WARNING("Refusing to make re-entrant call to VectorImage::Draw"); - return NS_ERROR_FAILURE; + return DrawResult::TEMPORARY_ERROR; } if (mAnimationConsumers == 0 && mProgressTracker) { @@ -786,7 +794,7 @@ VectorImage::Draw(gfxContext* aContext, if (aFlags & FLAG_BYPASS_SURFACE_CACHE) { CreateSurfaceAndShow(params); - return NS_OK; + return DrawResult::SUCCESS; } DrawableFrameRef frameRef = @@ -802,7 +810,7 @@ VectorImage::Draw(gfxContext* aContext, nsRefPtr svgDrawable = new gfxSurfaceDrawable(surface, ThebesIntSize(frameRef->GetSize())); Show(svgDrawable, params); - return NS_OK; + return DrawResult::SUCCESS; } // We lost our surface due to some catastrophic event. @@ -811,7 +819,7 @@ VectorImage::Draw(gfxContext* aContext, CreateSurfaceAndShow(params); - return NS_OK; + return DrawResult::SUCCESS; } void