Skip to content
Permalink
Browse files
toDataURL() uses stale data after putImageData()
https://bugs.webkit.org/show_bug.cgi?id=65767

Source/WebCore:

This patch fixes the issue we've encountered of getting back
stale copies of the CGContext of accelerated ImageBuffers who have seen
putImageData calls but have not been drawn into via the CG API.
This issue is fixed by modifying the way we implement putImageData
in ImageBufferCG to draw the bits wrapped in a CGImage while the CGContext
is in a state where the data will effectively be copied (as is needed for
implementing putImageData) instead of directly modifying the bits of the IOSurface.

Reviewed by Chris Marrin.

Test: fast/canvas/check-stale-putImageData, pixel test to check that the canvas is in fact painted.

* platform/graphics/cg/ImageBufferCG.cpp: Implement new way of putting image data.
* platform/graphics/ImageBuffer.h: Merged two previously separate put data calls
    into a single and more sensibly named 'putByteArray', since that's what it does!

* WebCore.exp.in: Added new WKSI call for use in ImageBufferCG.cpp
* platform/mac/WebCoreSystemInterface.h:
* platform/mac/WebCoreSystemInterface.mm:

Using new method name.
* html/canvas/CanvasRenderingContext2D.cpp:
* platform/graphics/ImageBuffer.cpp:
* platform/graphics/ShadowBlur.cpp:
* platform/graphics/filters/FEColorMatrix.cpp:
* platform/graphics/filters/FEDropShadow.cpp:
* platform/graphics/filters/FilterEffect.cpp:

Updated other ports' ImageBuffers to use new method.
* platform/graphics/cairo/ImageBufferCairo.cpp:
* platform/graphics/qt/ImageBufferQt.cpp:
* platform/graphics/skia/ImageBufferSkia.cpp:
* platform/graphics/wince/ImageBufferWinCE.cpp:
* platform/graphics/wx/ImageBufferWx.cpp:

Source/WebKit/chromium:

Reviewed by Chris Marrin.

* src/WebViewImpl.cpp: Updated method name.
(WebKit::WebViewImpl::doPixelReadbackToCanvas):

Source/WebKit/mac:

Reviewed by Chris Marrin.

* WebCoreSupport/WebSystemInterface.mm:

Source/WebKit2:

Reviewed by Chris Marrin.

* WebProcess/WebCoreSupport/mac/WebSystemInterface.mm:
(InitWebCoreSystemInterface):

WebKitLibraries:

Added WKCGContextResetClip for use in reseting clip for new putByteArray method.

Reviewed by Chris Marrin.

* WebKitSystemInterface.h: Added WKCGContextResetClip.
* libWebKitSystemInterfaceLeopard.a:
* libWebKitSystemInterfaceSnowLeopard.a:
* libWebKitSystemInterfaceLion.a:


Canonical link: https://commits.webkit.org/94762@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@106836 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
Matt Delaney committed Feb 6, 2012
1 parent e36423c commit 1f1f1e53b01d0f7105ff6b04825bd726b807b5e6
Showing 31 changed files with 211 additions and 110 deletions.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
@@ -0,0 +1,6 @@
CONSOLE MESSAGE: line 28: a == b? false
CONSOLE MESSAGE: line 35: a == c? false
1, 3, and 4 should all show the same red square. 2 should show nothing.



@@ -0,0 +1,42 @@
<html><head></head><body>
1, 3, and 4 should all show the same red square. 2 should show nothing.
<ol>
<li><canvas id="canvas" width="100" height="100"></canvas></li>
<li><img src="" id="a"></li>
<li><img src="" id="b"></li>
<li><img src="" id="c"></li>
</ol>

<script type="text/javascript">

if (window.layoutTestController)
layoutTestController.dumpAsText(true);

var canvas = document.getElementById( 'canvas' );
var context = canvas.getContext( '2d' );
var img = context.getImageData( 0, 0, canvas.width, canvas.height );

var img_a = canvas.toDataURL( 'image/png' );

/* fill with red */
for ( var i = 0; i < img.data.length; i += 4 )
img.data[i] = img.data[i+3] = 255;

context.putImageData( img, 0, 0 );
var img_b = canvas.toDataURL( 'image/png' );

console.log( 'a == b? ' + (img_a == img_b) );

context.moveTo( 0, 0 );
context.lineTo( 0, 0 );
context.stroke();

var img_c = canvas.toDataURL( 'image/png' );
console.log( 'a == c? ' + (img_a == img_c) );

document.getElementById( 'a' ).src = img_a;
document.getElementById( 'b' ).src = img_b;
document.getElementById( 'c' ).src = img_c;
</script>

</body></html>
@@ -1,3 +1,43 @@
2012-02-06 Matthew Delaney <mdelaney@apple.com>

toDataURL() uses stale data after putImageData()
https://bugs.webkit.org/show_bug.cgi?id=65767

This patch fixes the issue we've encountered of getting back
stale copies of the CGContext of accelerated ImageBuffers who have seen
putImageData calls but have not been drawn into via the CG API.
This issue is fixed by modifying the way we implement putImageData
in ImageBufferCG to draw the bits wrapped in a CGImage while the CGContext
is in a state where the data will effectively be copied (as is needed for
implementing putImageData) instead of directly modifying the bits of the IOSurface.

Reviewed by Chris Marrin.

Test: fast/canvas/check-stale-putImageData, pixel test to check that the canvas is in fact painted.

* platform/graphics/cg/ImageBufferCG.cpp: Implement new way of putting image data.
* platform/graphics/ImageBuffer.h: Merged two previously separate put data calls
into a single and more sensibly named 'putByteArray', since that's what it does!

* WebCore.exp.in: Added new WKSI call for use in ImageBufferCG.cpp
* platform/mac/WebCoreSystemInterface.h:
* platform/mac/WebCoreSystemInterface.mm:

Using new method name.
* html/canvas/CanvasRenderingContext2D.cpp:
* platform/graphics/ImageBuffer.cpp:
* platform/graphics/ShadowBlur.cpp:
* platform/graphics/filters/FEColorMatrix.cpp:
* platform/graphics/filters/FEDropShadow.cpp:
* platform/graphics/filters/FilterEffect.cpp:

Updated other ports' ImageBuffers to use new method.
* platform/graphics/cairo/ImageBufferCairo.cpp:
* platform/graphics/qt/ImageBufferQt.cpp:
* platform/graphics/skia/ImageBufferSkia.cpp:
* platform/graphics/wince/ImageBufferWinCE.cpp:
* platform/graphics/wx/ImageBufferWx.cpp:

2012-02-06 Caio Marcelo de Oliveira Filho <caio.oliveira@openbossa.org>

Provide more attribute methods in Element
@@ -1487,6 +1487,7 @@ _stringIsCaseInsensitiveEqualToString
_suggestedFilenameWithMIMEType
_wkAdvanceDefaultButtonPulseAnimation
_wkCGContextGetShouldSmoothFonts
_wkCGContextResetClip
_wkCGPatternCreateWithImageAndTransform
_wkCopyCFLocalizationPreferredName
_wkCopyCFURLResponseSuggestedFilename
@@ -1971,7 +1971,7 @@ void CanvasRenderingContext2D::putImageData(ImageData* data, float dx, float dy,
IntRect sourceRect(destRect);
sourceRect.move(-destOffset);

buffer->putUnmultipliedImageData(data->data()->data(), IntSize(data->width(), data->height()), sourceRect, IntPoint(destOffset));
buffer->putByteArray(Unmultiplied, data->data()->data(), IntSize(data->width(), data->height()), sourceRect, IntPoint(destOffset));
didDraw(destRect, CanvasDidDrawApplyNone); // ignore transform, shadow and clip
}

@@ -86,7 +86,7 @@ inline void ImageBuffer::genericConvertToLuminanceMask()
double luma = (r * 0.2125 + g * 0.7154 + b * 0.0721) * ((double)a / 255.0);
srcPixelArray->set(pixelOffset + 3, luma);
}
putUnmultipliedImageData(srcPixelArray.get(), luminanceRect.size(), luminanceRect, IntPoint());
putByteArray(Unmultiplied, srcPixelArray.get(), luminanceRect.size(), luminanceRect, IntPoint());
}

void ImageBuffer::convertToLuminanceMask()
@@ -101,8 +101,7 @@ namespace WebCore {
PassRefPtr<ByteArray> getUnmultipliedImageData(const IntRect&) const;
PassRefPtr<ByteArray> getPremultipliedImageData(const IntRect&) const;

void putUnmultipliedImageData(ByteArray*, const IntSize& sourceSize, const IntRect& sourceRect, const IntPoint& destPoint);
void putPremultipliedImageData(ByteArray*, const IntSize& sourceSize, const IntRect& sourceRect, const IntPoint& destPoint);
void putByteArray(Multiply multiplied, ByteArray*, const IntSize& sourceSize, const IntRect& sourceRect, const IntPoint& destPoint);

void convertToLuminanceMask();

@@ -828,7 +828,7 @@ void ShadowBlur::blurShadowBuffer(const IntSize& templateSize)
IntRect blurRect(IntPoint(), templateSize);
RefPtr<ByteArray> layerData = m_layerImage->getUnmultipliedImageData(blurRect);
blurLayerImage(layerData->data(), blurRect.size(), blurRect.width() * 4);
m_layerImage->putUnmultipliedImageData(layerData.get(), blurRect.size(), blurRect, IntPoint());
m_layerImage->putByteArray(Unmultiplied, layerData.get(), blurRect.size(), blurRect, IntPoint());
}

void ShadowBlur::blurAndColorShadowBuffer(const IntSize& templateSize)
@@ -204,41 +204,40 @@ PassRefPtr<ByteArray> ImageBuffer::getPremultipliedImageData(const IntRect& rect
return getImageData<Premultiplied>(rect, m_data, m_size);
}

template <Multiply multiplied>
void putImageData(ByteArray*& source, const IntSize& sourceSize, const IntRect& sourceRect, const IntPoint& destPoint, ImageBufferData& data, const IntSize& size)
void ImageBuffer::putByteArray(Multiply multiplied, ByteArray* source, const IntSize& sourceSize, const IntRect& sourceRect, const IntPoint& destPoint)
{
ASSERT(cairo_surface_get_type(data.m_surface) == CAIRO_SURFACE_TYPE_IMAGE);
ASSERT(cairo_surface_get_type(m_data.m_surface) == CAIRO_SURFACE_TYPE_IMAGE);

unsigned char* dataDst = cairo_image_surface_get_data(data.m_surface);
unsigned char* dataDst = cairo_image_surface_get_data(m_data.m_surface);

ASSERT(sourceRect.width() > 0);
ASSERT(sourceRect.height() > 0);

int originx = sourceRect.x();
int destx = destPoint.x() + sourceRect.x();
ASSERT(destx >= 0);
ASSERT(destx < size.width());
ASSERT(destx < m_size.width());
ASSERT(originx >= 0);
ASSERT(originx <= sourceRect.maxX());

int endx = destPoint.x() + sourceRect.maxX();
ASSERT(endx <= size.width());
ASSERT(endx <= m_size.width());

int numColumns = endx - destx;

int originy = sourceRect.y();
int desty = destPoint.y() + sourceRect.y();
ASSERT(desty >= 0);
ASSERT(desty < size.height());
ASSERT(desty < m_size.height());
ASSERT(originy >= 0);
ASSERT(originy <= sourceRect.maxY());

int endy = destPoint.y() + sourceRect.maxY();
ASSERT(endy <= size.height());
ASSERT(endy <= m_size.height());
int numRows = endy - desty;

unsigned srcBytesPerRow = 4 * sourceSize.width();
int stride = cairo_image_surface_get_stride(data.m_surface);
int stride = cairo_image_surface_get_stride(m_data.m_surface);

unsigned char* srcRows = source->data() + originy * srcBytesPerRow + originx * 4;
for (int y = 0; y < numRows; ++y) {
@@ -257,21 +256,11 @@ void putImageData(ByteArray*& source, const IntSize& sourceSize, const IntRect&
}
srcRows += srcBytesPerRow;
}
cairo_surface_mark_dirty_rectangle (data.m_surface,
cairo_surface_mark_dirty_rectangle(m_data.m_surface,
destx, desty,
numColumns, numRows);
}

void ImageBuffer::putUnmultipliedImageData(ByteArray* source, const IntSize& sourceSize, const IntRect& sourceRect, const IntPoint& destPoint)
{
putImageData<Unmultiplied>(source, sourceSize, sourceRect, destPoint, m_data, m_size);
}

void ImageBuffer::putPremultipliedImageData(ByteArray* source, const IntSize& sourceSize, const IntRect& sourceRect, const IntPoint& destPoint)
{
putImageData<Premultiplied>(source, sourceSize, sourceRect, destPoint, m_data, m_size);
}

#if !PLATFORM(GTK)
static cairo_status_t writeFunction(void* closure, const unsigned char* data, unsigned int length)
{
@@ -307,26 +307,36 @@ PassRefPtr<ByteArray> ImageBuffer::getPremultipliedImageData(const IntRect& rect
return m_data.getData(rect, m_size, m_context->isAcceleratedContext(), false);
}

void ImageBuffer::putUnmultipliedImageData(ByteArray* source, const IntSize& sourceSize, const IntRect& sourceRect, const IntPoint& destPoint)
void ImageBuffer::putByteArray(Multiply multiplied, ByteArray* source, const IntSize& sourceSize, const IntRect& sourceRect, const IntPoint& destPoint)
{
if (m_context->isAcceleratedContext()) {
CGContextFlush(context()->platformContext());
#if defined(BUILDING_ON_LION)
m_data.m_lastFlushTime = currentTimeMS();
#endif
if (!m_context->isAcceleratedContext()) {
m_data.putData(source, sourceSize, sourceRect, destPoint, m_size, m_context->isAcceleratedContext(), multiplied == Unmultiplied);
return;
}
m_data.putData(source, sourceSize, sourceRect, destPoint, m_size, m_context->isAcceleratedContext(), true);
}

void ImageBuffer::putPremultipliedImageData(ByteArray* source, const IntSize& sourceSize, const IntRect& sourceRect, const IntPoint& destPoint)
{
if (m_context->isAcceleratedContext()) {
CGContextFlush(context()->platformContext());
#if defined(BUILDING_ON_LION)
m_data.m_lastFlushTime = currentTimeMS();
#endif
}
m_data.putData(source, sourceSize, sourceRect, destPoint, m_size, m_context->isAcceleratedContext(), false);
// Make a copy of the source to ensure the bits don't change before being drawn
IntSize sourceCopySize(sourceRect.width(), sourceRect.height());
OwnPtr<ImageBuffer> sourceCopy = ImageBuffer::create(sourceCopySize, ColorSpaceDeviceRGB, Unaccelerated);
if (!sourceCopy)
return;
sourceCopy->m_data.putData(source, sourceSize, sourceRect, IntPoint(-sourceRect.x(), -sourceRect.y()), sourceCopy->size(), sourceCopy->context()->isAcceleratedContext(), multiplied == Unmultiplied);

// Set up context for using drawImage as a direct bit copy
CGContextRef destContext = context()->platformContext();
CGContextSaveGState(destContext);
CGContextConcatCTM(destContext, AffineTransform(CGContextGetCTM(destContext)).inverse());
wkCGContextResetClip(destContext);
CGContextSetInterpolationQuality(destContext, kCGInterpolationNone);
CGContextSetAlpha(destContext, 1.0);
CGContextSetBlendMode(destContext, kCGBlendModeCopy);
CGContextSetShadowWithColor(destContext, CGSizeZero, 0, 0);

// Draw the image in CG coordinate space
IntPoint destPointInCGCoords(destPoint.x() + sourceRect.x(), m_size.height() - (destPoint.y()+sourceRect.y()) - sourceRect.height());
IntRect destRectInCGCoords(destPointInCGCoords, sourceCopySize);
RetainPtr<CGImageRef> sourceCopyImage(AdoptCF, sourceCopy->copyNativeImage());
CGContextDrawImage(destContext, destRectInCGCoords, sourceCopyImage.get());
CGContextRestoreGState(destContext);
}

static inline CFStringRef jpegUTI()
@@ -187,7 +187,7 @@ void FEColorMatrix::platformApplySoftware()
break;
}

resultImage->putUnmultipliedImageData(pixelArray.get(), imageRect.size(), imageRect, IntPoint());
resultImage->putByteArray(Unmultiplied, pixelArray.get(), imageRect.size(), imageRect, IntPoint());
}

void FEColorMatrix::dump()
@@ -108,7 +108,7 @@ void FEDropShadow::platformApplySoftware()

contextShadow.blurLayerImage(srcPixelArray->data(), shadowArea.size(), 4 * shadowArea.size().width());

resultImage->putPremultipliedImageData(srcPixelArray.get(), shadowArea.size(), shadowArea, IntPoint());
resultImage->putByteArray(Premultiplied, srcPixelArray.get(), shadowArea.size(), shadowArea, IntPoint());

resultContext->setCompositeOperation(CompositeSourceIn);
resultContext->fillRect(FloatRect(FloatPoint(), absolutePaintRect().size()), m_shadowColor, ColorSpaceDeviceRGB);
@@ -130,9 +130,9 @@ ImageBuffer* FilterEffect::asImageBuffer()
m_imageBufferResult = ImageBuffer::create(m_absolutePaintRect.size(), ColorSpaceLinearRGB, m_filter->renderingMode());
IntRect destinationRect(IntPoint(), m_absolutePaintRect.size());
if (m_premultipliedImageResult)
m_imageBufferResult->putPremultipliedImageData(m_premultipliedImageResult.get(), destinationRect.size(), destinationRect, IntPoint());
m_imageBufferResult->putByteArray(Premultiplied, m_premultipliedImageResult.get(), destinationRect.size(), destinationRect, IntPoint());
else
m_imageBufferResult->putUnmultipliedImageData(m_unmultipliedImageResult.get(), destinationRect.size(), destinationRect, IntPoint());
m_imageBufferResult->putByteArray(Unmultiplied, m_unmultipliedImageResult.get(), destinationRect.size(), destinationRect, IntPoint());
return m_imageBufferResult.get();
}

@@ -301,33 +301,32 @@ static inline unsigned int premultiplyABGRtoARGB(unsigned int x)
return x;
}

template <Multiply multiplied>
void putImageData(ByteArray*& source, const IntSize& sourceSize, const IntRect& sourceRect, const IntPoint& destPoint, ImageBufferData& data, const IntSize& size)
void ImageBuffer::putByteArray(Multiply multiplied, ByteArray* source, const IntSize& sourceSize, const IntRect& sourceRect, const IntPoint& destPoint)
{
ASSERT(sourceRect.width() > 0);
ASSERT(sourceRect.height() > 0);

int originx = sourceRect.x();
int destx = destPoint.x() + sourceRect.x();
ASSERT(destx >= 0);
ASSERT(destx < size.width());
ASSERT(destx < m_size.width());
ASSERT(originx >= 0);
ASSERT(originx <= sourceRect.maxX());

int endx = destPoint.x() + sourceRect.maxX();
ASSERT(endx <= size.width());
ASSERT(endx <= m_size.width());

int numColumns = endx - destx;

int originy = sourceRect.y();
int desty = destPoint.y() + sourceRect.y();
ASSERT(desty >= 0);
ASSERT(desty < size.height());
ASSERT(desty < m_size.height());
ASSERT(originy >= 0);
ASSERT(originy <= sourceRect.maxY());

int endy = destPoint.y() + sourceRect.maxY();
ASSERT(endy <= size.height());
ASSERT(endy <= m_size.height());
int numRows = endy - desty;

unsigned srcBytesPerRow = 4 * sourceSize.width();
@@ -361,35 +360,25 @@ void putImageData(ByteArray*& source, const IntSize& sourceSize, const IntRect&
}
}

bool isPainting = data.m_painter->isActive();
bool isPainting = m_data.m_painter->isActive();
if (!isPainting)
data.m_painter->begin(&data.m_pixmap);
m_data.m_painter->begin(&m_data.m_pixmap);
else {
data.m_painter->save();
m_data.m_painter->save();

// putImageData() should be unaffected by painter state
data.m_painter->resetTransform();
data.m_painter->setOpacity(1.0);
data.m_painter->setClipping(false);
m_data.m_painter->resetTransform();
m_data.m_painter->setOpacity(1.0);
m_data.m_painter->setClipping(false);
}

data.m_painter->setCompositionMode(QPainter::CompositionMode_Source);
data.m_painter->drawImage(destx, desty, image);
m_data.m_painter->setCompositionMode(QPainter::CompositionMode_Source);
m_data.m_painter->drawImage(destx, desty, image);

if (!isPainting)
data.m_painter->end();
m_data.m_painter->end();
else
data.m_painter->restore();
}

void ImageBuffer::putUnmultipliedImageData(ByteArray* source, const IntSize& sourceSize, const IntRect& sourceRect, const IntPoint& destPoint)
{
putImageData<Unmultiplied>(source, sourceSize, sourceRect, destPoint, m_data, m_size);
}

void ImageBuffer::putPremultipliedImageData(ByteArray* source, const IntSize& sourceSize, const IntRect& sourceRect, const IntPoint& destPoint)
{
putImageData<Premultiplied>(source, sourceSize, sourceRect, destPoint, m_data, m_size);
m_data.m_painter->restore();
}

// We get a mimeType here but QImageWriter does not support mimetypes but

0 comments on commit 1f1f1e5

Please sign in to comment.