Changeset 156970 in webkit


Ignore:
Timestamp:
Oct 5, 2013, 12:21:04 PM (12 years ago)
Author:
dino@apple.com
Message:

Undo texture unit code refactoring - it is ok to bind a texture to multiple locations
​https://bugs.webkit.org/show_bug.cgi?id=122369
<rdar://problem/15158465>

Reviewed by Darin Adler.

The optimisation in r152351 (​http://webkit.org/b/117868) incorrectly
assumes that a texture cannot be bound to more than one location.
That's true only within the same program object. It's legal to
address TEXTURE_BINDING_2D and TEXTURE_BINDING_CUBE_MAP with the
same texture in different programs.

See ​https://github.com/KhronosGroup/WebGL/pull/391 for more information.

This change reverts the optimisation, but also does some minor
cleanup (0 -> nullptr, class -> struct).

Covered by the following Khronos WebGL 1.0.1 tests, which
now pass:

  • conformance/more/functions/texImage2DHTML.html
  • conformance/more/functions/texSubImage2D.html
  • conformance/more/functions/texSubImage2DHTML.html
  • html/canvas/WebGLRenderingContext.cpp:

(WebCore::WebGLRenderingContext::~WebGLRenderingContext):
(WebCore::WebGLRenderingContext::reshape):
(WebCore::WebGLRenderingContext::bindTexture):
(WebCore::WebGLRenderingContext::deleteTexture):
(WebCore::WebGLRenderingContext::getParameter):
(WebCore::WebGLRenderingContext::handleNPOTTextures):
(WebCore::WebGLRenderingContext::validateTextureBinding):
(WebCore::WebGLRenderingContext::restoreCurrentTexture2D):

  • html/canvas/WebGLRenderingContext.h:
Location:
trunk/Source/WebCore
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r156968 r156970  
     12013-10-05  Dean Jackson  <dino@apple.com>
     2
     3        Undo texture unit code refactoring - it is ok to bind a texture to multiple locations
     4        https://bugs.webkit.org/show_bug.cgi?id=122369
     5        <rdar://problem/15158465>
     6
     7        Reviewed by Darin Adler.
     8
     9        The optimisation in r152351 (http://webkit.org/b/117868) incorrectly
     10        assumes that a texture cannot be bound to more than one location.
     11        That's true only within the same program object. It's legal to
     12        address TEXTURE_BINDING_2D and TEXTURE_BINDING_CUBE_MAP with the
     13        same texture in different programs.
     14
     15        See https://github.com/KhronosGroup/WebGL/pull/391 for more information.
     16
     17        This change reverts the optimisation, but also does some minor
     18        cleanup (0 -> nullptr, class -> struct).
     19
     20        Covered by the following Khronos WebGL 1.0.1 tests, which
     21        now pass:
     22        - conformance/more/functions/texImage2DHTML.html
     23        - conformance/more/functions/texSubImage2D.html
     24        - conformance/more/functions/texSubImage2DHTML.html
     25
     26        * html/canvas/WebGLRenderingContext.cpp:
     27        (WebCore::WebGLRenderingContext::~WebGLRenderingContext):
     28        (WebCore::WebGLRenderingContext::reshape):
     29        (WebCore::WebGLRenderingContext::bindTexture):
     30        (WebCore::WebGLRenderingContext::deleteTexture):
     31        (WebCore::WebGLRenderingContext::getParameter):
     32        (WebCore::WebGLRenderingContext::handleNPOTTextures):
     33        (WebCore::WebGLRenderingContext::validateTextureBinding):
     34        (WebCore::WebGLRenderingContext::restoreCurrentTexture2D):
     35        * html/canvas/WebGLRenderingContext.h:
     36
    1372013-10-05  Anders Carlsson  <andersca@apple.com>
    238

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

    r156778 r156970  
    595595    // Remove all references to WebGLObjects so if they are the last reference
    596596    // they will be freed before the last context is removed from the context group.
    597     m_boundArrayBuffer = 0;
    598     m_defaultVertexArrayObject = 0;
    599     m_boundVertexArrayObject = 0;
    600     m_vertexAttrib0Buffer = 0;
    601     m_currentProgram = 0;
    602     m_framebufferBinding = 0;
    603     m_renderbufferBinding = 0;
    604 
    605     for (size_t i = 0; i < m_textureUnits.size(); ++i)
    606         m_textureUnits[i].m_textureBinding = 0;
    607 
    608     m_blackTexture2D = 0;
    609     m_blackTextureCubeMap = 0;
     597    m_boundArrayBuffer = nullptr;
     598    m_defaultVertexArrayObject = nullptr;
     599    m_boundVertexArrayObject = nullptr;
     600    m_vertexAttrib0Buffer = nullptr;
     601    m_currentProgram = nullptr;
     602    m_framebufferBinding = nullptr;
     603    m_renderbufferBinding = nullptr;
     604
     605    for (size_t i = 0; i < m_textureUnits.size(); ++i) {
     606        m_textureUnits[i].texture2DBinding = nullptr;
     607        m_textureUnits[i].textureCubeMapBinding = nullptr;
     608    }
     609
     610    m_blackTexture2D = nullptr;
     611    m_blackTextureCubeMap = nullptr;
    610612
    611613    detachAndRemoveAllObjects();
    … …  
    810812        m_context->reshape(width, height);
    811813
    812     m_context->bindTexture(GraphicsContext3D::TEXTURE_2D, objectOrZero(m_textureUnits[m_activeTextureUnit].m_textureBinding.get()));
     814    m_context->bindTexture(GraphicsContext3D::TEXTURE_2D, objectOrZero(m_textureUnits[m_activeTextureUnit].texture2DBinding.get()));
    813815    m_context->bindRenderbuffer(GraphicsContext3D::RENDERBUFFER, objectOrZero(m_renderbufferBinding.get()));
    814816    if (m_framebufferBinding)
    … …  
    10081010    GC3Dint maxLevel = 0;
    10091011    if (target == GraphicsContext3D::TEXTURE_2D) {
    1010         m_textureUnits[m_activeTextureUnit].m_textureBinding = texture;
     1012        m_textureUnits[m_activeTextureUnit].texture2DBinding = texture;
    10111013        maxLevel = m_maxTextureLevel;
    10121014
    … …  
    10151017
    10161018    } else if (target == GraphicsContext3D::TEXTURE_CUBE_MAP) {
    1017         m_textureUnits[m_activeTextureUnit].m_textureBinding = texture;
     1019        m_textureUnits[m_activeTextureUnit].textureCubeMapBinding = texture;
    10181020        maxLevel = m_maxCubeMapTextureLevel;
    10191021    } else {
    … …  
    16291631        return;
    16301632    for (size_t i = 0; i < m_textureUnits.size(); ++i) {
    1631         if (texture == m_textureUnits[i].m_textureBinding)
    1632             m_textureUnits[i].m_textureBinding = 0;
     1633        if (texture == m_textureUnits[i].texture2DBinding)
     1634            m_textureUnits[i].texture2DBinding = nullptr;
     1635        if (texture == m_textureUnits[i].textureCubeMapBinding)
     1636            m_textureUnits[i].textureCubeMapBinding = nullptr;
    16331637    }
    16341638    if (m_framebufferBinding)
    … …  
    26742678        return getIntParameter(pname);
    26752679    case GraphicsContext3D::TEXTURE_BINDING_2D:
    2676         return WebGLGetInfo(PassRefPtr<WebGLTexture>(m_textureUnits[m_activeTextureUnit].m_textureBinding));
     2680        return WebGLGetInfo(PassRefPtr<WebGLTexture>(m_textureUnits[m_activeTextureUnit].texture2DBinding));
    26772681    case GraphicsContext3D::TEXTURE_BINDING_CUBE_MAP:
    2678         return WebGLGetInfo(PassRefPtr<WebGLTexture>(m_textureUnits[m_activeTextureUnit].m_textureBinding));
     2682        return WebGLGetInfo(PassRefPtr<WebGLTexture>(m_textureUnits[m_activeTextureUnit].textureCubeMapBinding));
    26792683    case GraphicsContext3D::UNPACK_ALIGNMENT:
    26802684        return getIntParameter(pname);
    … …  
    47824786    bool resetActiveUnit = false;
    47834787    for (unsigned ii = 0; ii < m_textureUnits.size(); ++ii) {
    4784         if (m_textureUnits[ii].m_textureBinding && m_textureUnits[ii].m_textureBinding->needToUseBlackTexture()) {
     4788        if ((m_textureUnits[ii].texture2DBinding && m_textureUnits[ii].texture2DBinding->needToUseBlackTexture())
     4789            || (m_textureUnits[ii].textureCubeMapBinding && m_textureUnits[ii].textureCubeMapBinding->needToUseBlackTexture())) {
    47854790            if (ii != m_activeTextureUnit) {
    47864791                m_context->activeTexture(ii);
    … …  
    47904795                resetActiveUnit = false;
    47914796            }
    4792             WebGLTexture* texture = 0;
    4793             GC3Denum target = m_textureUnits[ii].m_textureBinding->getTarget();
     4797            WebGLTexture* tex2D;
     4798            WebGLTexture* texCubeMap;
    47944799            if (prepareToDraw) {
    47954800                String msg(String("texture bound to texture unit ") + String::number(ii)
    47964801                    + " is not renderable. It maybe non-power-of-2 and have incompatible texture filtering or is not 'texture complete'");
    47974802                printGLWarningToConsole(functionName, msg.utf8().data());
    4798                 if (target == GraphicsContext3D::TEXTURE_2D)
    4799                     texture = m_blackTexture2D.get();
    4800                 else if (target == GraphicsContext3D::TEXTURE_CUBE_MAP)
    4801                     texture = m_blackTextureCubeMap.get();
    4802             } else
    4803                 texture = m_textureUnits[ii].m_textureBinding.get();
    4804             m_context->bindTexture(target, objectOrZero(texture));
     4803                tex2D = m_blackTexture2D.get();
     4804                texCubeMap = m_blackTextureCubeMap.get();
     4805            } else {
     4806                tex2D = m_textureUnits[ii].texture2DBinding.get();
     4807                texCubeMap = m_textureUnits[ii].textureCubeMapBinding.get();
     4808            }
     4809            if (m_textureUnits[ii].texture2DBinding && m_textureUnits[ii].texture2DBinding->needToUseBlackTexture())
     4810                m_context->bindTexture(GraphicsContext3D::TEXTURE_2D, objectOrZero(tex2D));
     4811            if (m_textureUnits[ii].textureCubeMapBinding && m_textureUnits[ii].textureCubeMapBinding->needToUseBlackTexture())
     4812                m_context->bindTexture(GraphicsContext3D::TEXTURE_CUBE_MAP, objectOrZero(texCubeMap));
    48054813        }
    48064814    }
    … …  
    48674875WebGLTexture* WebGLRenderingContext::validateTextureBinding(const char* functionName, GC3Denum target, bool useSixEnumsForCubeMap)
    48684876{
     4877    WebGLTexture* texture = nullptr;
    48694878    switch (target) {
    48704879    case GraphicsContext3D::TEXTURE_2D:
     4880        texture = m_textureUnits[m_activeTextureUnit].texture2DBinding.get();
    48714881        break;
    48724882    case GraphicsContext3D::TEXTURE_CUBE_MAP_POSITIVE_X:
    … …  
    48784888        if (!useSixEnumsForCubeMap) {
    48794889            synthesizeGLError(GraphicsContext3D::INVALID_ENUM, functionName, "invalid texture target");
    4880             return 0;
    4881         }
     4890            return nullptr;
     4891        }
     4892        texture = m_textureUnits[m_activeTextureUnit].textureCubeMapBinding.get();
    48824893        break;
    48834894    case GraphicsContext3D::TEXTURE_CUBE_MAP:
    48844895        if (useSixEnumsForCubeMap) {
    48854896            synthesizeGLError(GraphicsContext3D::INVALID_ENUM, functionName, "invalid texture target");
    4886             return 0;
    4887         }
     4897            return nullptr;
     4898        }
     4899        texture = m_textureUnits[m_activeTextureUnit].textureCubeMapBinding.get();
    48884900        break;
    48894901    default:
    48904902        synthesizeGLError(GraphicsContext3D::INVALID_ENUM, functionName, "invalid texture target");
    4891         return 0;
    4892     }
    4893     WebGLTexture* tex = m_textureUnits[m_activeTextureUnit].m_textureBinding.get();
    4894     if (!tex)
     4903        return nullptr;
     4904    }
     4905    if (!texture)
    48954906        synthesizeGLError(GraphicsContext3D::INVALID_OPERATION, functionName, "no texture");
    4896     return tex;
     4907    return texture;
    48974908}
    48984909
    … …  
    59976008{
    59986009    ExceptionCode ec;
    5999     bindTexture(GraphicsContext3D::TEXTURE_2D, m_textureUnits[m_activeTextureUnit].m_textureBinding.get(), ec);
     6010    bindTexture(GraphicsContext3D::TEXTURE_2D, m_textureUnits[m_activeTextureUnit].texture2DBinding.get(), ec);
    60006011}
    60016012

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

    r156015 r156970  
    451451    RefPtr<WebGLRenderbuffer> m_renderbufferBinding;
    452452    struct TextureUnitState {
    453         RefPtr<WebGLTexture> m_textureBinding;
     453        RefPtr<WebGLTexture> texture2DBinding;
     454        RefPtr<WebGLTexture> textureCubeMapBinding;
    454455    };
    455456    Vector<TextureUnitState> m_textureUnits;
Note: See TracChangeset for help on using the changeset viewer.