Skip to content

Commit

Permalink
Bug 1260324. Don't draw garbage to the screen if an image doesn't hap…
Browse files Browse the repository at this point in the history
…pen to be decoded. r=seth

Layout has been using imgIContainer::IsOpaque to determine if the image will draw opaquely to all pixels it covers, and doing culling based on this.

However imgIContainer::IsOpaque doesn't guarantee anything. It only describes if the image, when in a decoded state, has all opaque pixels. So if the image doesn't have fully decoded frames around (because they got discarded) it may not draw opaquely to all of its pixels.

So we create a new function that first checks if there is a fully decoded frame.
  • Loading branch information
tnikkel committed Aug 23, 2016
1 parent 9946339 commit 5e78afe
Show file tree
Hide file tree
Showing 10 changed files with 49 additions and 13 deletions.
4 changes: 1 addition & 3 deletions image/DynamicImage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -195,10 +195,8 @@ DynamicImage::GetFrameAtSize(const IntSize& aSize,
}

NS_IMETHODIMP_(bool)
DynamicImage::IsOpaque()
DynamicImage::WillDrawOpaqueNow()
{
// XXX(seth): For performance reasons it'd be better to return true here, but
// I'm not sure how we can guarantee it for an arbitrary gfxDrawable.
return false;
}

Expand Down
4 changes: 2 additions & 2 deletions image/ImageWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -185,9 +185,9 @@ ImageWrapper::GetFrameAtSize(const IntSize& aSize,
}

NS_IMETHODIMP_(bool)
ImageWrapper::IsOpaque()
ImageWrapper::WillDrawOpaqueNow()
{
return mInnerImage->IsOpaque();
return mInnerImage->WillDrawOpaqueNow();
}

NS_IMETHODIMP_(bool)
Expand Down
2 changes: 1 addition & 1 deletion image/OrientedImage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ OrientedImage::GetFrame(uint32_t aWhichFrame,

// Determine an appropriate format for the surface.
gfx::SurfaceFormat surfaceFormat;
if (InnerImage()->IsOpaque()) {
if (InnerImage()->WillDrawOpaqueNow()) {
surfaceFormat = gfx::SurfaceFormat::B8G8R8X8;
} else {
surfaceFormat = gfx::SurfaceFormat::B8G8R8A8;
Expand Down
35 changes: 34 additions & 1 deletion image/RasterImage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ RasterImage::LookupFrame(const IntSize& aSize,
return Move(result.Surface());
}

NS_IMETHODIMP_(bool)
bool
RasterImage::IsOpaque()
{
if (mError) {
Expand All @@ -395,6 +395,39 @@ RasterImage::IsOpaque()
return !(progress & FLAG_HAS_TRANSPARENCY);
}

NS_IMETHODIMP_(bool)
RasterImage::WillDrawOpaqueNow()
{
if (!IsOpaque()) {
return false;
}

if (mAnimationState) {
// We never discard frames of animated images.
return true;
}

// If we are not locked our decoded data could get discard at any time (ie
// between the call to this function and when we are asked to draw), so we
// have to return false if we are unlocked.
if (IsUnlocked()) {
return false;
}

LookupResult result =
SurfaceCache::LookupBestMatch(ImageKey(this),
RasterSurfaceKey(mSize,
DefaultSurfaceFlags(),
PlaybackType::eStatic));
MatchType matchType = result.Type();
if (matchType == MatchType::NOT_FOUND || matchType == MatchType::PENDING ||
!result.Surface()->IsFinished()) {
return false;
}

return true;
}

void
RasterImage::OnSurfaceDiscarded()
{
Expand Down
2 changes: 2 additions & 0 deletions image/RasterImage.h
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,8 @@ class RasterImage final : public ImageResource
// Helpers
bool CanDiscard();

bool IsOpaque();

protected:
explicit RasterImage(ImageURL* aURI = nullptr);

Expand Down
2 changes: 1 addition & 1 deletion image/VectorImage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -678,7 +678,7 @@ VectorImage::GetFirstFrameDelay()
}

NS_IMETHODIMP_(bool)
VectorImage::IsOpaque()
VectorImage::WillDrawOpaqueNow()
{
return false; // In general, SVG content is not opaque.
}
Expand Down
2 changes: 1 addition & 1 deletion image/VectorImage.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class VectorImage final : public ImageResource,
nsresult aResult,
bool aLastPart) override;

void OnSurfaceDiscarded() override;
virtual void OnSurfaceDiscarded() override;

/**
* Callback for SVGRootRenderingObserver.
Expand Down
7 changes: 5 additions & 2 deletions image/imgIContainer.idl
Original file line number Diff line number Diff line change
Expand Up @@ -255,9 +255,12 @@ interface imgIContainer : nsISupports
in uint32_t aFlags);

/**
* Whether this image is opaque (i.e., needs a background painted behind it).
* Returns true if this image will draw opaquely right now if asked to draw
* with FLAG_HIGH_QUALITY_SCALING and otherwise default flags. If this image
* (when decoded) is opaque but no decoded frames are available then
* willDrawOpaqueNow will return false.
*/
[notxpcom] boolean isOpaque();
[noscript, notxpcom] boolean willDrawOpaqueNow();

/**
* @return true if getImageContainer() is expected to return a valid
Expand Down
2 changes: 1 addition & 1 deletion layout/generic/nsImageFrame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1634,7 +1634,7 @@ nsDisplayImage::GetOpaqueRegion(nsDisplayListBuilder* aBuilder,
bool* aSnap)
{
*aSnap = false;
if (mImage && mImage->IsOpaque()) {
if (mImage && mImage->WillDrawOpaqueNow()) {
const nsRect frameContentBox = GetBounds(aSnap);
return GetDestRect().Intersect(frameContentBox);
}
Expand Down
2 changes: 1 addition & 1 deletion layout/style/nsStyleStruct.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2218,7 +2218,7 @@ nsStyleImage::IsOpaque() const
MOZ_ASSERT(imageContainer, "IsComplete() said image container is ready");

// Check if the crop region of the image is opaque.
if (imageContainer->IsOpaque()) {
if (imageContainer->WillDrawOpaqueNow()) {
if (!mCropRect) {
return true;
}
Expand Down

0 comments on commit 5e78afe

Please sign in to comment.