Log Message
Assertion failure running Mozilla's WebGL performance regression tests https://bugs.webkit.org/show_bug.cgi?id=85942
Reviewed by Stephen White. Fixed incorrect assumptions about source formats and buffer sizes when uploading to floating-point textures. Added code paths supporting the necessary conversions. Tests have been added to the WebGL conformance suite which cover these new code paths; they verify uploads of HTMLCanvasElement, HTMLImageElement, HTMLVideoElement, and ImageData to floating-point textures. However, because floating-point texture support is optional, and generally only supported on bots which run with real GPUs and not in virtual machines, it isn't feasible to incorporate these tests as layout tests. Ran the new WebGL conformance tests in Chromium on Linux; all pass. * platform/graphics/GraphicsContext3D.cpp: (WebCore::GraphicsContext3D::extractImageData): Properly compute size of destination buffer. (WebCore): Add pack/unpack routines for converting RGBA8/BGRA8 to floating point. (WebCore::doFloatingPointPacking): Support RGBA8 and BGRA8 source formats. (WebCore::isFloatingPointSource): Factored out logic for assertions. (WebCore::GraphicsContext3D::packPixels): Generalized assertions and logic. * platform/graphics/cairo/GraphicsContext3DCairo.cpp: (WebCore::GraphicsContext3D::getImageData): Properly compute size of destination buffer. * platform/graphics/cg/GraphicsContext3DCG.cpp: (WebCore::GraphicsContext3D::getImageData): Properly compute size of destination buffer. * platform/graphics/qt/GraphicsContext3DQt.cpp: (WebCore::GraphicsContext3D::getImageData): Properly compute size of destination buffer. * platform/graphics/skia/GraphicsContext3DSkia.cpp: (WebCore::GraphicsContext3D::getImageData): Properly compute size of destination buffer.
Modified Paths
- trunk/Source/WebCore/ChangeLog
- trunk/Source/WebCore/platform/graphics/GraphicsContext3D.cpp
- trunk/Source/WebCore/platform/graphics/cairo/GraphicsContext3DCairo.cpp
- trunk/Source/WebCore/platform/graphics/cg/GraphicsContext3DCG.cpp
- trunk/Source/WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp
- trunk/Source/WebCore/platform/graphics/skia/GraphicsContext3DSkia.cpp
Diff
Modified: trunk/Source/WebCore/ChangeLog (117190 => 117191)
--- trunk/Source/WebCore/ChangeLog 2012-05-15 23:58:55 UTC (rev 117190)
+++ trunk/Source/WebCore/ChangeLog 2012-05-16 00:00:52 UTC (rev 117191)
@@ -1,3 +1,57 @@
+2012-05-15 Kenneth Russell <[email protected]>
+
+ Assertion failure running Mozilla's WebGL performance regression tests
+ https://bugs.webkit.org/show_bug.cgi?id=85942
+
+ Reviewed by Stephen White.
+
+ Fixed incorrect assumptions about source formats and buffer sizes
+ when uploading to floating-point textures. Added code paths
+ supporting the necessary conversions.
+
+ Tests have been added to the WebGL conformance suite which cover
+ these new code paths; they verify uploads of HTMLCanvasElement,
+ HTMLImageElement, HTMLVideoElement, and ImageData to
+ floating-point textures. However, because floating-point texture
+ support is optional, and generally only supported on bots which
+ run with real GPUs and not in virtual machines, it isn't feasible
+ to incorporate these tests as layout tests.
+
+ Ran the new WebGL conformance tests in Chromium on Linux; all
+ pass.
+
+ * platform/graphics/GraphicsContext3D.cpp:
+ (WebCore::GraphicsContext3D::extractImageData):
+ Properly compute size of destination buffer.
+
+ (WebCore):
+ Add pack/unpack routines for converting RGBA8/BGRA8 to floating point.
+
+ (WebCore::doFloatingPointPacking):
+ Support RGBA8 and BGRA8 source formats.
+
+ (WebCore::isFloatingPointSource):
+ Factored out logic for assertions.
+
+ (WebCore::GraphicsContext3D::packPixels):
+ Generalized assertions and logic.
+
+ * platform/graphics/cairo/GraphicsContext3DCairo.cpp:
+ (WebCore::GraphicsContext3D::getImageData):
+ Properly compute size of destination buffer.
+
+ * platform/graphics/cg/GraphicsContext3DCG.cpp:
+ (WebCore::GraphicsContext3D::getImageData):
+ Properly compute size of destination buffer.
+
+ * platform/graphics/qt/GraphicsContext3DQt.cpp:
+ (WebCore::GraphicsContext3D::getImageData):
+ Properly compute size of destination buffer.
+
+ * platform/graphics/skia/GraphicsContext3DSkia.cpp:
+ (WebCore::GraphicsContext3D::getImageData):
+ Properly compute size of destination buffer.
+
2012-05-15 James Robinson <[email protected]>
[chromium] Chromium port never sets USE(CG) so code behind it is dead
Modified: trunk/Source/WebCore/platform/graphics/GraphicsContext3D.cpp (117190 => 117191)
--- trunk/Source/WebCore/platform/graphics/GraphicsContext3D.cpp 2012-05-15 23:58:55 UTC (rev 117190)
+++ trunk/Source/WebCore/platform/graphics/GraphicsContext3D.cpp 2012-05-16 00:00:52 UTC (rev 117191)
@@ -200,8 +200,13 @@
return false;
int width = imageData->width();
int height = imageData->height();
- int dataBytes = width * height * 4;
- data.resize(dataBytes);
+
+ unsigned int packedSize;
+ // Output data is tightly packed (alignment == 1).
+ if (computeImageSizeInBytes(format, type, width, height, 1, &packedSize, 0) != GraphicsContext3D::NO_ERROR)
+ return false;
+ data.resize(packedSize);
+
if (!packPixels(imageData->data()->data(),
SourceFormatRGBA8,
width,
@@ -704,6 +709,32 @@
}
}
+void unpackOneRowOfRGBA8ToRGBA32F(const uint8_t* source, float* destination, unsigned int pixelsPerRow)
+{
+ const float scaleFactor = 1.0f / 255.0f;
+ for (unsigned int i = 0; i < pixelsPerRow; ++i) {
+ destination[0] = source[0] * scaleFactor;
+ destination[1] = source[1] * scaleFactor;
+ destination[2] = source[2] * scaleFactor;
+ destination[3] = source[3] * scaleFactor;
+ source += 4;
+ destination += 4;
+ }
+}
+
+void unpackOneRowOfBGRA8ToRGBA32F(const uint8_t* source, float* destination, unsigned int pixelsPerRow)
+{
+ const float scaleFactor = 1.0f / 255.0f;
+ for (unsigned int i = 0; i < pixelsPerRow; ++i) {
+ destination[0] = source[2] * scaleFactor;
+ destination[1] = source[1] * scaleFactor;
+ destination[2] = source[0] * scaleFactor;
+ destination[3] = source[3] * scaleFactor;
+ source += 4;
+ destination += 4;
+ }
+}
+
void unpackOneRowOfRGB32FToRGBA32F(const float* source, float* destination, unsigned int pixelsPerRow)
{
for (unsigned int i = 0; i < pixelsPerRow; ++i) {
@@ -1062,6 +1093,31 @@
}
}
+void packOneRowOfRGBA32FToRGB32FUnmultiply(const float* source, float* destination, unsigned int pixelsPerRow)
+{
+ for (unsigned int i = 0; i < pixelsPerRow; ++i) {
+ float scaleFactor = source[3] ? 1.0f / source[3] : 1.0f;
+ destination[0] = source[0] * scaleFactor;
+ destination[1] = source[1] * scaleFactor;
+ destination[2] = source[2] * scaleFactor;
+ source += 4;
+ destination += 3;
+ }
+}
+
+// Used only during RGBA8 or BGRA8 -> floating-point uploads.
+void packOneRowOfRGBA32FToRGBA32F(const float* source, float* destination, unsigned int pixelsPerRow)
+{
+ for (unsigned int i = 0; i < pixelsPerRow; ++i) {
+ destination[0] = source[0];
+ destination[1] = source[1];
+ destination[2] = source[2];
+ destination[3] = source[3];
+ source += 4;
+ destination += 4;
+ }
+}
+
void packOneRowOfRGBA32FToRGBA32FPremultiply(const float* source, float* destination, unsigned int pixelsPerRow)
{
for (unsigned int i = 0; i < pixelsPerRow; ++i) {
@@ -1075,6 +1131,19 @@
}
}
+void packOneRowOfRGBA32FToRGBA32FUnmultiply(const float* source, float* destination, unsigned int pixelsPerRow)
+{
+ for (unsigned int i = 0; i < pixelsPerRow; ++i) {
+ float scaleFactor = source[3] ? 1.0f / source[3] : 1.0f;
+ destination[0] = source[0] * scaleFactor;
+ destination[1] = source[1] * scaleFactor;
+ destination[2] = source[2] * scaleFactor;
+ destination[3] = source[3];
+ source += 4;
+ destination += 4;
+ }
+}
+
void packOneRowOfRGBA32FToA32F(const float* source, float* destination, unsigned int pixelsPerRow)
{
for (unsigned int i = 0; i < pixelsPerRow; ++i) {
@@ -1103,6 +1172,15 @@
}
}
+void packOneRowOfRGBA32FToR32FUnmultiply(const float* source, float* destination, unsigned int pixelsPerRow)
+{
+ for (unsigned int i = 0; i < pixelsPerRow; ++i) {
+ float scaleFactor = source[3] ? 1.0f / source[3] : 1.0f;
+ destination[0] = source[0] * scaleFactor;
+ source += 4;
+ destination += 1;
+ }
+}
void packOneRowOfRGBA32FToRA32F(const float* source, float* destination, unsigned int pixelsPerRow)
{
@@ -1119,12 +1197,23 @@
for (unsigned int i = 0; i < pixelsPerRow; ++i) {
float scaleFactor = source[3];
destination[0] = source[0] * scaleFactor;
- destination[1] = scaleFactor;
+ destination[1] = source[3];
source += 4;
destination += 2;
}
}
+void packOneRowOfRGBA32FToRA32FUnmultiply(const float* source, float* destination, unsigned int pixelsPerRow)
+{
+ for (unsigned int i = 0; i < pixelsPerRow; ++i) {
+ float scaleFactor = source[3] ? 1.0f / source[3] : 1.0f;
+ destination[0] = source[0] * scaleFactor;
+ destination[1] = source[3];
+ source += 4;
+ destination += 2;
+ }
+}
+
} // anonymous namespace
// This is used whenever unpacking is necessary; i.e., the source data
@@ -1367,6 +1456,16 @@
{
switch (sourceDataFormat) {
case GraphicsContext3D::SourceFormatRGBA8: {
+ unsigned int sourceElementsPerRow = computeSourceElementsPerRow<uint8_t>(width, 4, sourceUnpackAlignment);
+ doUnpackingAndPacking<uint8_t, float, float>(static_cast<const uint8_t*>(sourceData), unpackOneRowOfRGBA8ToRGBA32F, width, height, sourceElementsPerRow, destinationData, rowPackingFunc, destinationElementsPerPixel);
+ break;
+ }
+ case GraphicsContext3D::SourceFormatBGRA8: {
+ unsigned int sourceElementsPerRow = computeSourceElementsPerRow<uint8_t>(width, 4, sourceUnpackAlignment);
+ doUnpackingAndPacking<uint8_t, float, float>(static_cast<const uint8_t*>(sourceData), unpackOneRowOfBGRA8ToRGBA32F, width, height, sourceElementsPerRow, destinationData, rowPackingFunc, destinationElementsPerPixel);
+ break;
+ }
+ case GraphicsContext3D::SourceFormatRGBA32F: {
unsigned int sourceElementsPerRow = computeSourceElementsPerRow<float>(width, 4, sourceUnpackAlignment);
const float* source = static_cast<const float*>(sourceData);
const float* endPointer = source + height * sourceElementsPerRow;
@@ -1403,6 +1502,23 @@
}
}
+
+#if !ASSERT_DISABLED
+static bool isFloatingPointSource(GraphicsContext3D::SourceDataFormat format)
+{
+ switch (format) {
+ case GraphicsContext3D::SourceFormatRGBA32F:
+ case GraphicsContext3D::SourceFormatRGB32F:
+ case GraphicsContext3D::SourceFormatRA32F:
+ case GraphicsContext3D::SourceFormatR32F:
+ case GraphicsContext3D::SourceFormatA32F:
+ return true;
+ default:
+ return false;
+ }
+}
+#endif
+
bool GraphicsContext3D::packPixels(const uint8_t* sourceData,
GraphicsContext3D::SourceDataFormat sourceDataFormat,
unsigned int width,
@@ -1539,16 +1655,21 @@
}
case FLOAT: {
// OpenGL ES, and therefore WebGL, require that the format and
- // internalformat be identical, which implies that the source and
- // destination formats will both be floating-point in this branch -- at
- // least, until WebKit supports floating-point image formats natively.
- ASSERT(sourceDataFormat == SourceFormatRGBA32F || sourceDataFormat == SourceFormatRGB32F
- || sourceDataFormat == SourceFormatRA32F || sourceDataFormat == SourceFormatR32F
- || sourceDataFormat == SourceFormatA32F);
- // Because WebKit doesn't use floating-point color channels for anything
- // internally, there's no chance we have to do a (lossy) unmultiply
- // operation.
- ASSERT(alphaOp == AlphaDoNothing || alphaOp == AlphaDoPremultiply);
+ // internalformat be identical. This means that whenever the
+ // developer supplies an ArrayBufferView on this code path,
+ // the source data will be in a floating-point format.
+ //
+ // The only time the source data will not be floating-point is
+ // when uploading a DOM element or ImageData as a
+ // floating-point texture. Only RGBA8 and BGRA8 are handled in
+ // this case.
+ ASSERT(isFloatingPointSource(sourceDataFormat)
+ || sourceDataFormat == SourceFormatRGBA8
+ || sourceDataFormat == SourceFormatBGRA8);
+ // When uploading a canvas into a floating-point texture,
+ // unmultiplication may be necessary.
+ ASSERT((alphaOp == AlphaDoNothing || alphaOp == AlphaDoPremultiply)
+ || !isFloatingPointSource(sourceDataFormat));
// For the source formats with an even number of channels (RGBA32F,
// RA32F) it is guaranteed that the pixel data is tightly packed because
// unpack alignment <= sizeof(float) * number of channels.
@@ -1570,14 +1691,25 @@
case AlphaDoPremultiply:
doFloatingPointPacking(sourceData, sourceDataFormat, width, height, sourceUnpackAlignment, destination, packOneRowOfRGBA32FToRGB32FPremultiply, 3);
break;
- default:
- ASSERT_NOT_REACHED();
+ case AlphaDoUnmultiply:
+ doFloatingPointPacking(sourceData, sourceDataFormat, width, height, sourceUnpackAlignment, destination, packOneRowOfRGBA32FToRGB32FUnmultiply, 3);
+ break;
}
break;
case RGBA:
- // AlphaDoNothing is handled above with fast path.
- ASSERT(alphaOp == AlphaDoPremultiply);
- doFloatingPointPacking(sourceData, sourceDataFormat, width, height, sourceUnpackAlignment, destination, packOneRowOfRGBA32FToRGBA32FPremultiply, 4);
+ // AlphaDoNothing for RGBA32F -> RGBA is handled above with fast path.
+ ASSERT(alphaOp != AlphaDoNothing || sourceDataFormat != SourceFormatRGBA32F);
+ switch (alphaOp) {
+ case AlphaDoNothing:
+ doFloatingPointPacking(sourceData, sourceDataFormat, width, height, sourceUnpackAlignment, destination, packOneRowOfRGBA32FToRGBA32F, 4);
+ break;
+ case AlphaDoPremultiply:
+ doFloatingPointPacking(sourceData, sourceDataFormat, width, height, sourceUnpackAlignment, destination, packOneRowOfRGBA32FToRGBA32FPremultiply, 4);
+ break;
+ case AlphaDoUnmultiply:
+ doFloatingPointPacking(sourceData, sourceDataFormat, width, height, sourceUnpackAlignment, destination, packOneRowOfRGBA32FToRGBA32FUnmultiply, 4);
+ break;
+ }
break;
case ALPHA:
// From the desktop OpenGL conversion rules (OpenGL 2.1
@@ -1596,8 +1728,9 @@
case AlphaDoPremultiply:
doFloatingPointPacking(sourceData, sourceDataFormat, width, height, sourceUnpackAlignment, destination, packOneRowOfRGBA32FToR32FPremultiply, 1);
break;
- default:
- ASSERT_NOT_REACHED();
+ case AlphaDoUnmultiply:
+ doFloatingPointPacking(sourceData, sourceDataFormat, width, height, sourceUnpackAlignment, destination, packOneRowOfRGBA32FToR32FUnmultiply, 1);
+ break;
}
break;
case LUMINANCE_ALPHA:
@@ -1611,8 +1744,9 @@
case AlphaDoPremultiply:
doFloatingPointPacking(sourceData, sourceDataFormat, width, height, sourceUnpackAlignment, destination, packOneRowOfRGBA32FToRA32FPremultiply, 2);
break;
- default:
- ASSERT_NOT_REACHED();
+ case AlphaDoUnmultiply:
+ doFloatingPointPacking(sourceData, sourceDataFormat, width, height, sourceUnpackAlignment, destination, packOneRowOfRGBA32FToRA32FUnmultiply, 2);
+ break;
}
break;
}
Modified: trunk/Source/WebCore/platform/graphics/cairo/GraphicsContext3DCairo.cpp (117190 => 117191)
--- trunk/Source/WebCore/platform/graphics/cairo/GraphicsContext3DCairo.cpp 2012-05-15 23:58:55 UTC (rev 117190)
+++ trunk/Source/WebCore/platform/graphics/cairo/GraphicsContext3DCairo.cpp 2012-05-16 00:00:52 UTC (rev 117191)
@@ -185,7 +185,12 @@
++srcUnpackAlignment;
}
- outputVector.resize(width * height * 4);
+ unsigned int packedSize;
+ // Output data is tightly packed (alignment == 1).
+ if (computeImageSizeInBytes(format, type, width, height, 1, &packedSize, 0) != GraphicsContext3D::NO_ERROR)
+ return false;
+ outputVector.resize(packedSize);
+
return packPixels(cairo_image_surface_get_data(imageSurface.get()), SourceFormatBGRA8,
width, height, srcUnpackAlignment, format, type, alphaOp, outputVector.data());
}
Modified: trunk/Source/WebCore/platform/graphics/cg/GraphicsContext3DCG.cpp (117190 => 117191)
--- trunk/Source/WebCore/platform/graphics/cg/GraphicsContext3DCG.cpp 2012-05-15 23:58:55 UTC (rev 117190)
+++ trunk/Source/WebCore/platform/graphics/cg/GraphicsContext3DCG.cpp 2012-05-16 00:00:52 UTC (rev 117191)
@@ -240,7 +240,13 @@
if (!pixelData)
return false;
const UInt8* rgba = CFDataGetBytePtr(pixelData.get());
- outputVector.resize(width * height * 4);
+
+ unsigned int packedSize;
+ // Output data is tightly packed (alignment == 1).
+ if (computeImageSizeInBytes(format, type, width, height, 1, &packedSize, 0) != GraphicsContext3D::NO_ERROR)
+ return false;
+ outputVector.resize(packedSize);
+
unsigned int srcUnpackAlignment = 0;
size_t bytesPerRow = CGImageGetBytesPerRow(cgImage);
unsigned int padding = bytesPerRow - bitsPerPixel / 8 * width;
Modified: trunk/Source/WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp (117190 => 117191)
--- trunk/Source/WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp 2012-05-15 23:58:55 UTC (rev 117190)
+++ trunk/Source/WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp 2012-05-16 00:00:52 UTC (rev 117191)
@@ -1614,7 +1614,13 @@
AlphaOp neededAlphaOp = AlphaDoNothing;
if (premultiplyAlpha)
neededAlphaOp = AlphaDoPremultiply;
- outputVector.resize(nativeImage.byteCount());
+
+ unsigned int packedSize;
+ // Output data is tightly packed (alignment == 1).
+ if (computeImageSizeInBytes(format, type, image->width(), image->height(), 1, &packedSize, 0) != GraphicsContext3D::NO_ERROR)
+ return false;
+ outputVector.resize(packedSize);
+
return packPixels(nativeImage.bits(), SourceFormatBGRA8, image->width(), image->height(), 0, format, type, neededAlphaOp, outputVector.data());
}
Modified: trunk/Source/WebCore/platform/graphics/skia/GraphicsContext3DSkia.cpp (117190 => 117191)
--- trunk/Source/WebCore/platform/graphics/skia/GraphicsContext3DSkia.cpp 2012-05-15 23:58:55 UTC (rev 117190)
+++ trunk/Source/WebCore/platform/graphics/skia/GraphicsContext3DSkia.cpp 2012-05-16 00:00:52 UTC (rev 117191)
@@ -79,7 +79,11 @@
const SkBitmap& skiaImageRef = skiaImage->bitmap();
SkAutoLockPixels lock(skiaImageRef);
ASSERT(skiaImageRef.rowBytes() == skiaImageRef.width() * 4);
- outputVector.resize(skiaImageRef.rowBytes() * skiaImageRef.height());
+ unsigned int packedSize;
+ // Output data is tightly packed (alignment == 1).
+ if (computeImageSizeInBytes(format, type, skiaImageRef.width(), skiaImageRef.height(), 1, &packedSize, 0) != GraphicsContext3D::NO_ERROR)
+ return false;
+ outputVector.resize(packedSize);
return packPixels(reinterpret_cast<const uint8_t*>(skiaImageRef.getPixels()),
SK_B32_SHIFT ? SourceFormatRGBA8 : SourceFormatBGRA8,
skiaImageRef.width(), skiaImageRef.height(), 0,
_______________________________________________ webkit-changes mailing list [email protected] http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
