Changeset 57574 in webkit


Ignore:
Timestamp:
Apr 14, 2010 3:23:49 AM (14 years ago)
Author:
eric@webkit.org
Message:

2010-04-14 Zhenyao Mo <​zmo@google.com>

Reviewed by Dimitri Glazkov.

readPixels must take PACK_ALIGNMENT into account
​https://bugs.webkit.org/show_bug.cgi?id=34718

  • fast/canvas/webgl/read-pixels-expected.txt: Verify pixelStorei and readPixels.
  • fast/canvas/webgl/read-pixels.html: Ditto.

2010-04-14 Zhenyao Mo <​zmo@google.com>

Reviewed by Dimitri Glazkov.

readPixels must take PACK_ALIGNMENT into account
​https://bugs.webkit.org/show_bug.cgi?id=34718

Test: fast/canvas/webgl/read-pixels.html

  • html/canvas/WebGLRenderingContext.cpp: (WebCore::WebGLRenderingContext::WebGLRenderingContext): Init members to support pack_alignment. (WebCore::WebGLRenderingContext::pixelStorei): Save pack/unpack_alignment. (WebCore::WebGLRenderingContext::readPixels): Validate enum and deal with pack_alignment.
  • html/canvas/WebGLRenderingContext.h: Add members to support pack_alignment.
  • platform/graphics/GraphicsContext3D.h: Refactor readPixels.
  • platform/graphics/mac/GraphicsContext3DMac.cpp: (WebCore::GraphicsContext3D::readPixels): Move array allocation and alpha fix to WebGLRenderingContext; flush before read pixels.

2010-04-14 Zhenyao Mo <​zmo@google.com>

Reviewed by Dimitri Glazkov.

readPixels must take PACK_ALIGNMENT into account
​https://bugs.webkit.org/show_bug.cgi?id=34718

  • src/GraphicsContext3D.cpp: Refactor readPixels.
  • src/WebGraphicsContext3DDefaultImpl.cpp: (WebKit::WebGraphicsContext3DDefaultImpl::readBackFramebuffer): Temporarily disable pack alignment for glReadPixels. (WebKit::WebGraphicsContext3DDefaultImpl::readPixels): Move array allocation and alpha fix to WebGLRenderingContext; flush before read pixels.
Location:
trunk
Files:
2 added
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r57573 r57574  
     12010-04-14  Zhenyao Mo  <zmo@google.com>
     2
     3        Reviewed by Dimitri Glazkov.
     4
     5        readPixels must take PACK_ALIGNMENT into account
     6        https://bugs.webkit.org/show_bug.cgi?id=34718
     7
     8        * fast/canvas/webgl/read-pixels-expected.txt: Verify pixelStorei and readPixels.
     9        * fast/canvas/webgl/read-pixels.html: Ditto.
     10
    1112010-04-14  Nicolas Weber  <thakis@chromium.org>
    212

  • trunk/WebCore/ChangeLog

    r57564 r57574  
     12010-04-14  Zhenyao Mo  <zmo@google.com>
     2
     3        Reviewed by Dimitri Glazkov.
     4
     5        readPixels must take PACK_ALIGNMENT into account
     6        https://bugs.webkit.org/show_bug.cgi?id=34718
     7
     8        Test: fast/canvas/webgl/read-pixels.html
     9
     10        * html/canvas/WebGLRenderingContext.cpp:
     11        (WebCore::WebGLRenderingContext::WebGLRenderingContext): Init members to support pack_alignment.
     12        (WebCore::WebGLRenderingContext::pixelStorei): Save pack/unpack_alignment.
     13        (WebCore::WebGLRenderingContext::readPixels): Validate enum and deal with pack_alignment.
     14        * html/canvas/WebGLRenderingContext.h: Add members to support pack_alignment.
     15        * platform/graphics/GraphicsContext3D.h: Refactor readPixels.
     16        * platform/graphics/mac/GraphicsContext3DMac.cpp:
     17        (WebCore::GraphicsContext3D::readPixels): Move array allocation and alpha fix to WebGLRenderingContext; flush before read pixels.
     18
    1192010-04-13  Dirk Schulze  <krit@webkit.org>
    220

  • trunk/WebCore/html/canvas/WebGLRenderingContext.cpp

    r57324 r57574  
    3939#include "RenderLayer.h"
    4040#include "WebGLActiveInfo.h"
     41#include "WebGLUnsignedShortArray.h"
    4142#include "WebGLBuffer.h"
    4243#include "WebGLContextAttributes.h"
    … …  
    8687    , m_markedCanvasDirty(false)
    8788    , m_activeTextureUnit(0)
     89    , m_packAlignment(4)
     90    , m_unpackAlignment(4)
    8891{
    8992    ASSERT(m_context);
    … …  
    9194    m_context->getIntegerv(GraphicsContext3D::MAX_VERTEX_ATTRIBS, &numVertexAttribs);
    9295    m_maxVertexAttribs = numVertexAttribs;
     96    int implementationColorReadFormat = GraphicsContext3D::RGBA;
     97    m_context->getIntegerv(GraphicsContext3D::IMPLEMENTATION_COLOR_READ_FORMAT, &implementationColorReadFormat);
     98    m_implementationColorReadFormat = implementationColorReadFormat;
     99    int implementationColorReadType = GraphicsContext3D::UNSIGNED_BYTE;
     100    m_context->getIntegerv(GraphicsContext3D::IMPLEMENTATION_COLOR_READ_TYPE, &implementationColorReadType);
     101    // FIXME: remove the getError() when IMPLEMENTATION_COLOR_READ_FORMAT/TYPE are supported.
     102    m_context->getError();
     103    m_implementationColorReadType = implementationColorReadType;
    93104    m_context->reshape(canvas()->width(), canvas()->height());
    94105    m_context->viewport(0, 0, canvas()->width(), canvas()->height());
    … …  
    15651576{
    15661577    m_context->pixelStorei(pname, param);
     1578    if (param == 1 || param == 2 || param == 4 || param == 8) {
     1579        switch (pname) {
     1580        case GraphicsContext3D::PACK_ALIGNMENT:
     1581            m_packAlignment = static_cast<int>(param);
     1582            break;
     1583        case GraphicsContext3D::UNPACK_ALIGNMENT:
     1584            m_unpackAlignment = static_cast<int>(param);
     1585            break;
     1586        }
     1587    }
    15671588    cleanupAfterGraphicsCall(false);
    15681589}
    … …  
    15761597PassRefPtr<WebGLArray> WebGLRenderingContext::readPixels(long x, long y, unsigned long width, unsigned long height, unsigned long format, unsigned long type)
    15771598{
    1578     RefPtr<WebGLArray> array = m_context->readPixels(x, y, width, height, format, type);
     1599    // Validate enums.
     1600    unsigned long componentsPerPixel = 0;
     1601    switch (format) {
     1602    case GraphicsContext3D::ALPHA:
     1603        componentsPerPixel = 1;
     1604        break;
     1605    case GraphicsContext3D::RGB:
     1606        componentsPerPixel = 3;
     1607        break;
     1608    case GraphicsContext3D::RGBA:
     1609        componentsPerPixel = 4;
     1610        break;
     1611    default:
     1612        m_context->synthesizeGLError(GraphicsContext3D::INVALID_ENUM);
     1613        return 0;
     1614    }
     1615    unsigned long bytesPerComponent = 0;
     1616    switch (type) {
     1617    case GraphicsContext3D::UNSIGNED_BYTE:
     1618        bytesPerComponent = sizeof(unsigned char);
     1619        break;
     1620    case GraphicsContext3D::UNSIGNED_SHORT_5_6_5:
     1621    case GraphicsContext3D::UNSIGNED_SHORT_4_4_4_4:
     1622    case GraphicsContext3D::UNSIGNED_SHORT_5_5_5_1:
     1623        componentsPerPixel = 1;
     1624        bytesPerComponent = sizeof(unsigned short);
     1625        break;
     1626    default:
     1627        m_context->synthesizeGLError(GraphicsContext3D::INVALID_ENUM);
     1628        return 0;
     1629    }
     1630    if (!(format == GraphicsContext3D::RGBA && type == GraphicsContext3D::UNSIGNED_BYTE || format == m_implementationColorReadFormat && type == m_implementationColorReadFormat)) {
     1631        m_context->synthesizeGLError(GraphicsContext3D::INVALID_OPERATION);
     1632        return 0;
     1633    }
     1634    // Calculate array size, taking into consideration of PACK_ALIGNMENT.
     1635    unsigned long bytesPerRow = componentsPerPixel * bytesPerComponent * width;
     1636    unsigned long padding = 0;
     1637    unsigned long residualBytes = bytesPerRow % m_packAlignment;
     1638    if (residualBytes) {
     1639        padding = m_packAlignment - residualBytes;
     1640        bytesPerRow += padding;
     1641    }
     1642    // The last row needs no padding.
     1643    unsigned long totalBytes = bytesPerRow * height - padding;
     1644    unsigned long num = totalBytes / bytesPerComponent;
     1645    RefPtr<WebGLArray> array;
     1646    if (type == GraphicsContext3D::UNSIGNED_BYTE)
     1647        array = WebGLUnsignedByteArray::create(num);
     1648    else
     1649        array = WebGLUnsignedShortArray::create(num);
     1650    void* data = array->baseAddress();
     1651    m_context->readPixels(x, y, width, height, format, type, data);
     1652#if PLATFORM(CG)
     1653    // FIXME: remove this section when GL driver bug on Mac is fixed, i.e.,
     1654    // when alpha is off, readPixels should set alpha to 255 instead of 0.
     1655    if ((format == GraphicsContext3D::ALPHA || format == GraphicsContext3D::RGBA) && !m_context->getContextAttributes().alpha) {
     1656        if (type == GraphicsContext3D::UNSIGNED_BYTE) {
     1657            unsigned char* pixels = reinterpret_cast<unsigned char*>(data);
     1658            for (unsigned long iy = 0; iy < height; ++iy) {
     1659                for (unsigned long ix = 0; ix < width; ++ix) {
     1660                    pixels[componentsPerPixel - 1] = 255;
     1661                    pixels += componentsPerPixel;
     1662                }
     1663                pixels += padding;
     1664            }
     1665        }
     1666        // FIXME: check whether we need to do the same with UNSIGNED_SHORT.
     1667    }
     1668#endif
    15791669    cleanupAfterGraphicsCall(false);
    15801670    return array;

  • trunk/WebCore/html/canvas/WebGLRenderingContext.h

    r57324 r57574  
    345345        unsigned long m_activeTextureUnit;
    346346
     347        int m_packAlignment;
     348        int m_unpackAlignment;
     349        unsigned long m_implementationColorReadFormat;
     350        unsigned long m_implementationColorReadType;
     351
    347352        // Helpers for getParameter and others
    348353        WebGLGetInfo getBooleanParameter(unsigned long pname);

  • trunk/WebCore/platform/graphics/GraphicsContext3D.h

    r57174 r57574  
    593593        void polygonOffset(double factor, double units);
    594594       
    595         PassRefPtr<WebGLArray> readPixels(long x, long y, unsigned long width, unsigned long height, unsigned long format, unsigned long type);
     595        void readPixels(long x, long y, unsigned long width, unsigned long height, unsigned long format, unsigned long type, void* data);
    596596       
    597597        void releaseShaderCompiler();

  • trunk/WebCore/platform/graphics/mac/GraphicsContext3DMac.cpp

    r57322 r57574  
    802802}
    803803
    804 PassRefPtr<WebGLArray> GraphicsContext3D::readPixels(long x, long y, unsigned long width, unsigned long height, unsigned long format, unsigned long type)
    805 {
    806     ensureContext(m_contextObj);
    807    
    808     // FIXME: For now we only accept GL_UNSIGNED_BYTE/GL_RGBA. In reality OpenGL ES 2.0 accepts that pair and one other
    809     // as specified by GL_IMPLEMENTATION_COLOR_READ_FORMAT and GL_IMPLEMENTATION_COLOR_READ_TYPE. But for now we will
    810     // not accept those.
    811     // FIXME: Also, we should throw when an unacceptable value is passed
    812     if (type != GL_UNSIGNED_BYTE || format != GL_RGBA)
    813         return 0;
    814        
     804void GraphicsContext3D::readPixels(long x, long y, unsigned long width, unsigned long height, unsigned long format, unsigned long type, void* data)
     805{
     806    // FIXME: remove the two glFlush calls when the driver bug is fixed, i.e.,
     807    // all previous rendering calls should be done before reading pixels.
     808    ensureContext(m_contextObj);
     809    ::glFlush();
    815810    if (m_attrs.antialias && m_boundFBO == m_multisampleFBO) {
    816811        ::glBindFramebufferEXT(GL_READ_FRAMEBUFFER_EXT, m_multisampleFBO);
    … …  
    818813        ::glBlitFramebufferEXT(x, y, x + width, y + height, x, y, x + width, y + height, GL_COLOR_BUFFER_BIT, GL_LINEAR);
    819814        ::glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, m_fbo);
    820     }
    821     RefPtr<WebGLUnsignedByteArray> array = WebGLUnsignedByteArray::create(width * height * 4);
    822     ::glReadPixels(x, y, width, height, format, type, (GLvoid*) array->data());
     815        ::glFlush();
     816    }
     817    ::glReadPixels(x, y, width, height, format, type, data);
    823818    if (m_attrs.antialias && m_boundFBO == m_multisampleFBO)
    824819        ::glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, m_multisampleFBO);
    825     if (!m_attrs.alpha) {
    826         // If alpha is off, by default glReadPixels should set the alpha to 255 instead of 0.
    827         // This is a hack until ::glReadPixels fixes its behavior.
    828         GLubyte* data = reinterpret_cast<GLubyte*>(array->data());
    829         unsigned byteLength = array->byteLength();
    830         for (unsigned i = 3; i < byteLength; i += 4)
    831             data[i] = 255;
    832     }
    833     return array;   
    834820}
    835821

  • trunk/WebKit/chromium/ChangeLog

    r57571 r57574  
     12010-04-14  Zhenyao Mo  <zmo@google.com>
     2
     3        Reviewed by Dimitri Glazkov.
     4
     5        readPixels must take PACK_ALIGNMENT into account
     6        https://bugs.webkit.org/show_bug.cgi?id=34718
     7
     8        * src/GraphicsContext3D.cpp: Refactor readPixels.
     9        * src/WebGraphicsContext3DDefaultImpl.cpp:
     10        (WebKit::WebGraphicsContext3DDefaultImpl::readBackFramebuffer): Temporarily disable pack alignment for glReadPixels.
     11        (WebKit::WebGraphicsContext3DDefaultImpl::readPixels): Move array allocation and alpha fix to WebGLRenderingContext; flush before read pixels.
     12
    1132010-04-14  Mikhail Naganov  <mnaganov@chromium.org>
    214

  • trunk/WebKit/chromium/src/GraphicsContext3D.cpp

    r56825 r57574  
    219219    void polygonOffset(double factor, double units);
    220220
    221     PassRefPtr<WebGLArray> readPixels(long x, long y, unsigned long width, unsigned long height, unsigned long format, unsigned long type);
     221    void readPixels(long x, long y, unsigned long width, unsigned long height, unsigned long format, unsigned long type, void* data);
    222222
    223223    void releaseShaderCompiler();
    … …  
    591591}
    592592
     593#define DELEGATE_TO_IMPL_7(name, t1, t2, t3, t4, t5, t6, t7) \
     594void GraphicsContext3DInternal::name(t1 a1, t2 a2, t3 a3, t4 a4, t5 a5, t6 a6, t7 a7) \
     595{ \
     596    m_impl->name(a1, a2, a3, a4, a5, a6, a7);   \
     597}
     598
    593599#define DELEGATE_TO_IMPL_7R(name, t1, t2, t3, t4, t5, t6, t7, rt) \
    594600rt GraphicsContext3DInternal::name(t1 a1, t2 a2, t3 a3, t4 a4, t5 a5, t6 a6, t7 a7) \
    … …  
    805811DELEGATE_TO_IMPL_2(pixelStorei, unsigned long, long)
    806812DELEGATE_TO_IMPL_2(polygonOffset, double, double)
    807 
    808 PassRefPtr<WebGLArray> GraphicsContext3DInternal::readPixels(long x, long y, unsigned long width, unsigned long height, unsigned long format, unsigned long type)
    809 {
    810     // FIXME: take into account pack alignment.
    811     RefPtr<WebGLUnsignedByteArray> array = WebGLUnsignedByteArray::create(width * height * 4);
    812     m_impl->readPixels(x, y, width, height, format, type, array->baseAddress());
    813     return array;
    814 }
    815 
     813DELEGATE_TO_IMPL_7(readPixels, long, long, unsigned long, unsigned long, unsigned long, unsigned long, void*)
    816814DELEGATE_TO_IMPL(releaseShaderCompiler)
    817815DELEGATE_TO_IMPL_4(renderbufferStorage, unsigned long, unsigned long, unsigned long, unsigned long)
    … …  
    10251023{ \
    10261024    return m_internal->name(a1, a2, a3, a4, a5, a6);       \
     1025}
     1026
     1027#define DELEGATE_TO_INTERNAL_7(name, t1, t2, t3, t4, t5, t6, t7) \
     1028void GraphicsContext3D::name(t1 a1, t2 a2, t3 a3, t4 a4, t5 a5, t6 a6, t7 a7) \
     1029{ \
     1030    m_internal->name(a1, a2, a3, a4, a5, a6, a7);   \
    10271031}
    10281032
    … …  
    11841188DELEGATE_TO_INTERNAL_2(polygonOffset, double, double)
    11851189
    1186 DELEGATE_TO_INTERNAL_6R(readPixels, long, long, unsigned long, unsigned long, unsigned long, unsigned long, PassRefPtr<WebGLArray>)
     1190DELEGATE_TO_INTERNAL_7(readPixels, long, long, unsigned long, unsigned long, unsigned long, unsigned long, void*)
    11871191
    11881192DELEGATE_TO_INTERNAL(releaseShaderCompiler)

  • trunk/WebKit/chromium/src/WebGraphicsContext3DDefaultImpl.cpp

    r57322 r57574  
    662662        }
    663663    }
     664
     665    GLint packAlignment = 4;
     666    bool mustRestorePackAlignment = false;
     667    glGetIntegerv(GL_PACK_ALIGNMENT, &packAlignment);
     668    if (packAlignment > 4) {
     669        glPixelStorei(GL_PACK_ALIGNMENT, 4);
     670        mustRestorePackAlignment = true;
     671    }
     672
    664673#if PLATFORM(SKIA)
    665674    glReadPixels(0, 0, m_cachedWidth, m_cachedHeight, GL_BGRA, GL_UNSIGNED_BYTE, pixels);
    … …  
    669678#error Must port to your platform
    670679#endif
     680
     681    if (mustRestorePackAlignment)
     682        glPixelStorei(GL_PACK_ALIGNMENT, packAlignment);
    671683
    672684    if (mustRestoreFBO)
    … …  
    11531165void WebGraphicsContext3DDefaultImpl::readPixels(long x, long y, unsigned long width, unsigned long height, unsigned long format, unsigned long type, void* pixels)
    11541166{
     1167    // FIXME: remove the two glFlush calls when the driver bug is fixed, i.e.,
     1168    // all previous rendering calls should be done before reading pixels.
     1169    glFlush();
    11551170#ifndef RENDER_TO_DEBUGGING_WINDOW
    11561171    if (m_attributes.antialias && m_boundFBO == m_multisampleFBO) {
    … …  
    11591174        glBlitFramebufferEXT(x, y, x + width, y + height, x, y, x + width, y + height, GL_COLOR_BUFFER_BIT, GL_LINEAR);
    11601175        glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, m_fbo);
     1176        glFlush();
    11611177    }
    11621178#endif
    11631179    glReadPixels(x, y, width, height, format, type, pixels);
    1164 #if PLATFORM(CG)
    1165     if (!m_attributes.alpha) {
    1166         // If alpha is off, by default glReadPixels should set the alpha to 255 instead of 0.
    1167         // This is a hack until glReadPixels fixes its behavior.
    1168         // Pixels are stored in WebGLUnsignedByteArray, which is unsigned char array.
    1169         unsigned char* data = reinterpret_cast<unsigned char*>(pixels);
    1170         // FIXME: take into account pack alignment.
    1171         unsigned long byteLength = width * height * 4 * sizeof(unsigned char);
    1172         for (unsigned long i = 3; i < byteLength; i += 4)
    1173             data[i] = 255;
    1174     }
    1175 #endif
    11761180#ifndef RENDER_TO_DEBUGGING_WINDOW
    11771181    if (m_attributes.antialias && m_boundFBO == m_multisampleFBO)
Note: See TracChangeset for help on using the changeset viewer.