Skip to content
Permalink
Browse files
Change hasAlpha to isKnownToBeOpaque and correct the return value for…
… SVG images.

https://bugs.webkit.org/show_bug.cgi?id=106966

Reviewed by Stephen White.

Source/WebCore:

Previously, Image::currentFrameHasAlpha's default implementation returned false so SVG
images always returned false for currentFrameHasAlpha. Additionally, currentFrameHasAlpha
was treated as returning whether the frame had alpha when it would actually conservatively
return true.

This patch renames hasAlpha and currentFrameHasAlpha to isKnownToBeOpaque and
currentFrameIsKnownToBeOpaque, respectively. This rename better describes the actual
functionality. This patch also makes Image::isKnownToBeOpaque a pure virtual function and
correctly implements it for SVG images.

All users of isKnownToBeOpaque access SVG images using CachedImage::imageForRenderer which
currently returns a cached bitmap image. Therefore, this patch will not affect existing
functionality. A regression test has been added that will catch if this changes in the
future (e.g., WK106159 which proposes not returning cached bitmaps). The now unnecessary
isBitmapImage() calls have been removed in this patch.

image-box-shadow.html has been modified to test SVG images.

* css/CSSCrossfadeValue.cpp:
(WebCore::subimageKnownToBeOpaque):
(WebCore::CSSCrossfadeValue::knownToBeOpaque):

    Mostly straightforward rename but note the logic has been slightly altered: AND -> OR.

* css/CSSCrossfadeValue.h:
(CSSCrossfadeValue):
* css/CSSGradientValue.cpp:
(WebCore::CSSGradientValue::knownToBeOpaque):
* css/CSSGradientValue.h:
(CSSGradientValue):
* css/CSSImageGeneratorValue.cpp:
(WebCore::CSSImageGeneratorValue::knownToBeOpaque):
* css/CSSImageGeneratorValue.h:
(CSSImageGeneratorValue):
* css/CSSImageValue.cpp:
(WebCore::CSSImageValue::knownToBeOpaque):
* css/CSSImageValue.h:
(CSSImageValue):
* loader/cache/CachedImage.cpp:
(WebCore::CachedImage::currentFrameKnownToBeOpaque):
* loader/cache/CachedImage.h:
(CachedImage):
* platform/graphics/BitmapImage.cpp:
(WebCore::BitmapImage::currentFrameKnownToBeOpaque):
* platform/graphics/BitmapImage.h:
(BitmapImage):
* platform/graphics/GeneratedImage.h:
(GeneratedImage):
* platform/graphics/Image.h:

    Note: now a pure virtual function!

(Image):
* platform/graphics/blackberry/LayerTiler.cpp:
(WebCore::LayerTiler::updateTextureContentsIfNeeded):

    Removed unnecessary isBitmapImage() checks.

* platform/graphics/cg/GraphicsContext3DCG.cpp:
(WebCore::GraphicsContext3D::ImageExtractor::extractImage):

    Removed unnecessary isBitmapImage() checks.

* platform/graphics/cg/PDFDocumentImage.h:
(PDFDocumentImage):
* platform/graphics/chromium/GraphicsLayerChromium.cpp:

    Removed unnecessary isBitmapImage() checks.

(WebCore::GraphicsLayerChromium::setContentsToImage):
* platform/graphics/qt/StillImageQt.cpp:
(WebCore::StillImage::currentFrameKnownToBeOpaque):
* platform/graphics/qt/StillImageQt.h:
(StillImage):
* platform/graphics/skia/BitmapImageSingleFrameSkia.cpp:
(WebCore::BitmapImageSingleFrameSkia::currentFrameKnownToBeOpaque):
* platform/graphics/skia/BitmapImageSingleFrameSkia.h:
(BitmapImageSingleFrameSkia):
* platform/graphics/texmap/TextureMapperBackingStore.cpp:
(WebCore::TextureMapperTile::updateContents):
(WebCore::TextureMapperTiledBackingStore::updateContents):
* platform/graphics/texmap/coordinated/CoordinatedImageBacking.cpp:
(WebCore::CoordinatedImageBacking::update):
* rendering/RenderImage.cpp:
(WebCore::RenderImage::backgroundIsObscured):

    Removed unnecessary isBitmapImage() checks and slightly reworked the logic.

* rendering/style/FillLayer.cpp:
(WebCore::FillLayer::hasOpaqueImage):
* rendering/style/StyleCachedImage.cpp:
(WebCore::StyleCachedImage::knownToBeOpaque):
* rendering/style/StyleCachedImage.h:
(StyleCachedImage):
* rendering/style/StyleCachedImageSet.cpp:
(WebCore::StyleCachedImageSet::knownToBeOpaque):
* rendering/style/StyleCachedImageSet.h:
(StyleCachedImageSet):
* rendering/style/StyleGeneratedImage.cpp:
(WebCore::StyleGeneratedImage::knownToBeOpaque):
* rendering/style/StyleGeneratedImage.h:
(StyleGeneratedImage):
* rendering/style/StyleImage.h:
(StyleImage):
* rendering/style/StylePendingImage.h:
(WebCore::StylePendingImage::knownToBeOpaque):
* svg/graphics/SVGImage.h:
(SVGImage):

Source/WebKit/chromium:

* tests/DragImageTest.cpp:
(WebCore::TestImage::currentFrameKnownToBeOpaque):
(TestImage):
* tests/ImageLayerChromiumTest.cpp:
(WebCore::TestImage::currentFrameKnownToBeOpaque):
* tests/PlatformContextSkiaTest.cpp:
(WebCore::TEST):

Source/WebKit/win:

* WebKit.vcproj/WebKitExports.def.in:

LayoutTests:

An SVG image has been added to this test to check for regressions.

* fast/box-shadow/image-box-shadow-expected.html:
* fast/box-shadow/image-box-shadow.html:

    Add an SVG image and correct a small mistake in the test that used values of 256
    instead of 255.

* fast/box-shadow/resources/green.svg: Added.


Canonical link: https://commits.webkit.org/126899@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@141637 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
progers committed Feb 1, 2013
1 parent 25ffcb8 commit 6cc3787a94b12da834fa255a0e1d29ec77716b23
Showing 46 changed files with 246 additions and 73 deletions.
@@ -1,3 +1,20 @@
2013-02-01 Philip Rogers <pdr@google.com>

Change hasAlpha to isKnownToBeOpaque and correct the return value for SVG images.
https://bugs.webkit.org/show_bug.cgi?id=106966

Reviewed by Stephen White.

An SVG image has been added to this test to check for regressions.

* fast/box-shadow/image-box-shadow-expected.html:
* fast/box-shadow/image-box-shadow.html:

Add an SVG image and correct a small mistake in the test that used values of 256
instead of 255.

* fast/box-shadow/resources/green.svg: Added.

2013-02-01 Philippe Normand <pnormand@igalia.com>

Unreviewed, GTK TestExpectations update.
@@ -7,10 +7,11 @@
}
div {
display: inline-block;
background: rgb(0,256,0);
background: rgb(0,255,0);
height: 50px;
}
</style>
<div><img src="resources/green-alpha.png"></div>
<div><img src="resources/green.png"></div>
<div><img src="resources/green.jpg"></div>
<div><img src="resources/green.svg"></div>
@@ -1,11 +1,11 @@
<!DOCTYPE html>
<style>
img {
background: rgb(0,256,0);
background: rgb(0,255,0);
width: 50px;
height: 25px;
margin-bottom: 25px;
box-shadow: 0px 25px rgb(0,256,0);
box-shadow: 0px 25px rgb(0,255,0);
}
div {
display: inline-block;
@@ -16,3 +16,4 @@
<div><img src="resources/green-alpha.png"></div>
<div><img src="resources/green.png"></div>
<div><img src="resources/green.jpg"></div>
<div><img src="resources/green.svg"></div>
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
@@ -1,3 +1,119 @@
2013-02-01 Philip Rogers <pdr@google.com>

Change hasAlpha to isKnownToBeOpaque and correct the return value for SVG images.
https://bugs.webkit.org/show_bug.cgi?id=106966

Reviewed by Stephen White.

Previously, Image::currentFrameHasAlpha's default implementation returned false so SVG
images always returned false for currentFrameHasAlpha. Additionally, currentFrameHasAlpha
was treated as returning whether the frame had alpha when it would actually conservatively
return true.

This patch renames hasAlpha and currentFrameHasAlpha to isKnownToBeOpaque and
currentFrameIsKnownToBeOpaque, respectively. This rename better describes the actual
functionality. This patch also makes Image::isKnownToBeOpaque a pure virtual function and
correctly implements it for SVG images.

All users of isKnownToBeOpaque access SVG images using CachedImage::imageForRenderer which
currently returns a cached bitmap image. Therefore, this patch will not affect existing
functionality. A regression test has been added that will catch if this changes in the
future (e.g., WK106159 which proposes not returning cached bitmaps). The now unnecessary
isBitmapImage() calls have been removed in this patch.

image-box-shadow.html has been modified to test SVG images.

* css/CSSCrossfadeValue.cpp:
(WebCore::subimageKnownToBeOpaque):
(WebCore::CSSCrossfadeValue::knownToBeOpaque):

Mostly straightforward rename but note the logic has been slightly altered: AND -> OR.

* css/CSSCrossfadeValue.h:
(CSSCrossfadeValue):
* css/CSSGradientValue.cpp:
(WebCore::CSSGradientValue::knownToBeOpaque):
* css/CSSGradientValue.h:
(CSSGradientValue):
* css/CSSImageGeneratorValue.cpp:
(WebCore::CSSImageGeneratorValue::knownToBeOpaque):
* css/CSSImageGeneratorValue.h:
(CSSImageGeneratorValue):
* css/CSSImageValue.cpp:
(WebCore::CSSImageValue::knownToBeOpaque):
* css/CSSImageValue.h:
(CSSImageValue):
* loader/cache/CachedImage.cpp:
(WebCore::CachedImage::currentFrameKnownToBeOpaque):
* loader/cache/CachedImage.h:
(CachedImage):
* platform/graphics/BitmapImage.cpp:
(WebCore::BitmapImage::currentFrameKnownToBeOpaque):
* platform/graphics/BitmapImage.h:
(BitmapImage):
* platform/graphics/GeneratedImage.h:
(GeneratedImage):
* platform/graphics/Image.h:

Note: now a pure virtual function!

(Image):
* platform/graphics/blackberry/LayerTiler.cpp:
(WebCore::LayerTiler::updateTextureContentsIfNeeded):

Removed unnecessary isBitmapImage() checks.

* platform/graphics/cg/GraphicsContext3DCG.cpp:
(WebCore::GraphicsContext3D::ImageExtractor::extractImage):

Removed unnecessary isBitmapImage() checks.

* platform/graphics/cg/PDFDocumentImage.h:
(PDFDocumentImage):
* platform/graphics/chromium/GraphicsLayerChromium.cpp:

Removed unnecessary isBitmapImage() checks.

(WebCore::GraphicsLayerChromium::setContentsToImage):
* platform/graphics/qt/StillImageQt.cpp:
(WebCore::StillImage::currentFrameKnownToBeOpaque):
* platform/graphics/qt/StillImageQt.h:
(StillImage):
* platform/graphics/skia/BitmapImageSingleFrameSkia.cpp:
(WebCore::BitmapImageSingleFrameSkia::currentFrameKnownToBeOpaque):
* platform/graphics/skia/BitmapImageSingleFrameSkia.h:
(BitmapImageSingleFrameSkia):
* platform/graphics/texmap/TextureMapperBackingStore.cpp:
(WebCore::TextureMapperTile::updateContents):
(WebCore::TextureMapperTiledBackingStore::updateContents):
* platform/graphics/texmap/coordinated/CoordinatedImageBacking.cpp:
(WebCore::CoordinatedImageBacking::update):
* rendering/RenderImage.cpp:
(WebCore::RenderImage::backgroundIsObscured):

Removed unnecessary isBitmapImage() checks and slightly reworked the logic.

* rendering/style/FillLayer.cpp:
(WebCore::FillLayer::hasOpaqueImage):
* rendering/style/StyleCachedImage.cpp:
(WebCore::StyleCachedImage::knownToBeOpaque):
* rendering/style/StyleCachedImage.h:
(StyleCachedImage):
* rendering/style/StyleCachedImageSet.cpp:
(WebCore::StyleCachedImageSet::knownToBeOpaque):
* rendering/style/StyleCachedImageSet.h:
(StyleCachedImageSet):
* rendering/style/StyleGeneratedImage.cpp:
(WebCore::StyleGeneratedImage::knownToBeOpaque):
* rendering/style/StyleGeneratedImage.h:
(StyleGeneratedImage):
* rendering/style/StyleImage.h:
(StyleImage):
* rendering/style/StylePendingImage.h:
(WebCore::StylePendingImage::knownToBeOpaque):
* svg/graphics/SVGImage.h:
(SVGImage):

2013-02-01 Brady Eidson <beidson@apple.com>

Clean up WebArchive loading with the NetworkProcess
@@ -52,17 +52,17 @@ static bool subimageIsPending(CSSValue* value)
return false;
}

static bool subimageHasAlpha(CSSValue* value, const RenderObject* renderer)
static bool subimageKnownToBeOpaque(CSSValue* value, const RenderObject* renderer)
{
if (value->isImageValue())
return static_cast<CSSImageValue*>(value)->hasAlpha(renderer);
return static_cast<CSSImageValue*>(value)->knownToBeOpaque(renderer);

if (value->isImageGeneratorValue())
return static_cast<CSSImageGeneratorValue*>(value)->hasAlpha(renderer);
return static_cast<CSSImageGeneratorValue*>(value)->knownToBeOpaque(renderer);

ASSERT_NOT_REACHED();

return true;
return false;
}

static CachedImage* cachedImageForCSSValue(CSSValue* value, CachedResourceLoader* cachedResourceLoader)
@@ -139,9 +139,9 @@ bool CSSCrossfadeValue::isPending() const
return subimageIsPending(m_fromValue.get()) || subimageIsPending(m_toValue.get());
}

bool CSSCrossfadeValue::hasAlpha(const RenderObject* renderer) const
bool CSSCrossfadeValue::knownToBeOpaque(const RenderObject* renderer) const
{
return subimageHasAlpha(m_fromValue.get(), renderer) || subimageHasAlpha(m_toValue.get(), renderer);
return subimageKnownToBeOpaque(m_fromValue.get(), renderer) && subimageKnownToBeOpaque(m_toValue.get(), renderer);
}

void CSSCrossfadeValue::loadSubimages(CachedResourceLoader* cachedResourceLoader)
@@ -58,7 +58,7 @@ class CSSCrossfadeValue : public CSSImageGeneratorValue {
IntSize fixedSize(const RenderObject*);

bool isPending() const;
bool hasAlpha(const RenderObject*) const;
bool knownToBeOpaque(const RenderObject*) const;

void loadSubimages(CachedResourceLoader*);

@@ -462,13 +462,13 @@ bool CSSGradientValue::isCacheable() const
return true;
}

bool CSSGradientValue::hasAlpha(const RenderObject*) const
bool CSSGradientValue::knownToBeOpaque(const RenderObject*) const
{
for (size_t i = 0; i < m_stops.size(); ++i) {
if (m_stops[i].m_resolvedColor.hasAlpha())
return true;
return false;
}
return false;
return true;
}

void CSSGradientValue::reportBaseClassMemoryUsage(MemoryObjectInfo* memoryObjectInfo) const
@@ -81,7 +81,7 @@ class CSSGradientValue : public CSSImageGeneratorValue {
IntSize fixedSize(const RenderObject*) const { return IntSize(); }

bool isPending() const { return false; }
bool hasAlpha(const RenderObject*) const;
bool knownToBeOpaque(const RenderObject*) const;

void loadSubimages(CachedResourceLoader*) { }
PassRefPtr<CSSGradientValue> gradientWithStylesResolved(StyleResolver*);
@@ -196,21 +196,21 @@ bool CSSImageGeneratorValue::isPending() const
return false;
}

bool CSSImageGeneratorValue::hasAlpha(const RenderObject* renderer) const
bool CSSImageGeneratorValue::knownToBeOpaque(const RenderObject* renderer) const
{
switch (classType()) {
case CrossfadeClass:
return static_cast<const CSSCrossfadeValue*>(this)->hasAlpha(renderer);
return static_cast<const CSSCrossfadeValue*>(this)->knownToBeOpaque(renderer);
case CanvasClass:
return true;
return false;
case LinearGradientClass:
return static_cast<const CSSLinearGradientValue*>(this)->hasAlpha(renderer);
return static_cast<const CSSLinearGradientValue*>(this)->knownToBeOpaque(renderer);
case RadialGradientClass:
return static_cast<const CSSRadialGradientValue*>(this)->hasAlpha(renderer);
return static_cast<const CSSRadialGradientValue*>(this)->knownToBeOpaque(renderer);
default:
ASSERT_NOT_REACHED();
}
return true;
return false;
}

void CSSImageGeneratorValue::loadSubimages(CachedResourceLoader* cachedResourceLoader)
@@ -63,7 +63,7 @@ class CSSImageGeneratorValue : public CSSValue {
IntSize fixedSize(const RenderObject*);

bool isPending() const;
bool hasAlpha(const RenderObject*) const;
bool knownToBeOpaque(const RenderObject*) const;

void loadSubimages(CachedResourceLoader*);

@@ -113,9 +113,9 @@ void CSSImageValue::reportDescendantMemoryUsage(MemoryObjectInfo* memoryObjectIn
// No need to report m_image as it is counted as part of RenderArena.
}

bool CSSImageValue::hasAlpha(const RenderObject* renderer) const
bool CSSImageValue::knownToBeOpaque(const RenderObject* renderer) const
{
return m_image ? m_image->hasAlpha(renderer) : true;
return m_image ? m_image->knownToBeOpaque(renderer) : false;
}

} // namespace WebCore
@@ -52,7 +52,7 @@ class CSSImageValue : public CSSValue {

void reportDescendantMemoryUsage(MemoryObjectInfo*) const;

bool hasAlpha(const RenderObject*) const;
bool knownToBeOpaque(const RenderObject*) const;

void setInitiator(const AtomicString& name) { m_initiatorName = name; }

@@ -506,12 +506,12 @@ void CachedImage::reportMemoryUsage(MemoryObjectInfo* memoryObjectInfo) const
#endif
}

bool CachedImage::currentFrameHasAlpha(const RenderObject* renderer)
bool CachedImage::currentFrameKnownToBeOpaque(const RenderObject* renderer)
{
Image* image = imageForRenderer(renderer);
if (image->isBitmapImage())
image->nativeImageForCurrentFrame(); // force decode
return image->currentFrameHasAlpha();
return image->currentFrameKnownToBeOpaque();
}

} // namespace WebCore
@@ -54,7 +54,7 @@ class CachedImage : public CachedResource, public ImageObserver {
Image* image(); // Returns the nullImage() if the image is not available yet.
Image* imageForRenderer(const RenderObject*); // Returns the nullImage() if the image is not available yet.
bool hasImage() const { return m_image.get(); }
bool currentFrameHasAlpha(const RenderObject*); // Side effect: ensures decoded image is in cache, therefore should only be called when about to draw the image.
bool currentFrameKnownToBeOpaque(const RenderObject*); // Side effect: ensures decoded image is in cache, therefore should only be called when about to draw the image.

std::pair<Image*, float> brokenImage(float deviceScaleFactor) const; // Returns an image and the image's resolution scale factor.
bool willPaintBrokenImage() const;
@@ -336,9 +336,9 @@ bool BitmapImage::frameHasAlphaAtIndex(size_t index)
return m_source.frameHasAlphaAtIndex(index);
}

bool BitmapImage::currentFrameHasAlpha()
bool BitmapImage::currentFrameKnownToBeOpaque()
{
return frameHasAlphaAtIndex(currentFrame());
return !frameHasAlphaAtIndex(currentFrame());
}

ImageOrientation BitmapImage::currentFrameOrientation()
@@ -178,7 +178,7 @@ class BitmapImage : public Image {
#endif

virtual NativeImagePtr nativeImageForCurrentFrame();
virtual bool currentFrameHasAlpha();
virtual bool currentFrameKnownToBeOpaque() OVERRIDE;

ImageOrientation currentFrameOrientation();

@@ -57,6 +57,9 @@ class GeneratedImage : public Image {
virtual void drawPattern(GraphicsContext*, const FloatRect& srcRect, const AffineTransform& patternTransform,
const FloatPoint& phase, ColorSpace styleColorSpace, CompositeOperator, const FloatRect& destRect) = 0;

// FIXME: Implement this to be less conservative.
virtual bool currentFrameKnownToBeOpaque() OVERRIDE { return false; }

GeneratedImage() { }

IntSize m_size;
@@ -99,7 +99,7 @@ class Image : public RefCounted<Image> {

virtual bool isSVGImage() const { return false; }
virtual bool isBitmapImage() const { return false; }
virtual bool currentFrameHasAlpha() { return false; }
virtual bool currentFrameKnownToBeOpaque() = 0;

// Derived classes should override this if they can assure that
// the image contains only resources from its own security origin.
@@ -184,9 +184,7 @@ void LayerTiler::updateTextureContentsIfNeeded(double scale)
HashSet<TileIndex> finishedJobs;
if (!renderJobs.isEmpty()) {
if (Image* image = m_layer->contents()) {
bool isOpaque = false;
if (image->isBitmapImage())
isOpaque = !static_cast<BitmapImage*>(image)->currentFrameHasAlpha();
bool isOpaque = image->currentFrameKnownToBeOpaque();
if (NativeImagePtr nativeImage = image->nativeImageForCurrentFrame()) {
SkBitmap bitmap = SkBitmap(nativeImage->bitmap());
addTextureJob(TextureJob::setContents(bitmap, isOpaque));
@@ -237,9 +235,7 @@ void LayerTiler::updateTextureContentsIfNeeded(double scale)
return;

if (Image* image = m_layer->contents()) {
bool isOpaque = false;
if (image->isBitmapImage())
isOpaque = !static_cast<BitmapImage*>(image)->currentFrameHasAlpha();
bool isOpaque = image->currentFrameKnownToBeOpaque();
// No point in tiling an image layer, the image is already stored as an SkBitmap
NativeImagePtr nativeImage = m_layer->contents()->nativeImageForCurrentFrame();
if (nativeImage) {

0 comments on commit 6cc3787

Please sign in to comment.