⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 282192 in webkit


Ignore:
Timestamp:
Sep 8, 2021, 10:27:51 PM (5 years ago)
Author:
commit-queue@webkit.org
Message:

webgl/2.0.y/conformance2/vertex_arrays/vertex-array-object.html fails
​https://bugs.webkit.org/show_bug.cgi?id=223360
<rdar://problem/75774546>

Patch by Kimmo Kinnunen <​kkinnunen@apple.com> on 2021-09-08
Reviewed by Dean Jackson.

Source/WebCore:

Deleting a buffer will remove the buffer from currently active
vertex array buffer bindings. This worked.
Drawing with a vertex array object that has enabled indexes
but missing buffers for those indexes should fail. This
WebGL specific constraint is checked by ANGLE. However, WebCore
relies on buffer deletion to update the ANGLE VAO. The deletion
does not happen due to the buffers in the test being bound to
multiple VAOs.
Add this check also to WebCore.

Fixed tests:
webgl/1.0.x/conformance/extensions/oes-vertex-array-object.html
webgl/2.0.y/conformance2/vertex_arrays/vertex-array-object.html

  • html/canvas/WebGL2RenderingContext.cpp:

(WebCore::WebGL2RenderingContext::initializeVertexArrayObjects):
(WebCore::WebGL2RenderingContext::drawRangeElements):

  • html/canvas/WebGLRenderingContext.cpp:

(WebCore::WebGLRenderingContext::initializeVertexArrayObjects):

  • html/canvas/WebGLRenderingContextBase.cpp:

(WebCore::WebGLRenderingContextBase::disableVertexAttribArray):
(WebCore::WebGLRenderingContextBase::validateVertexArrayObject):
(WebCore::WebGLRenderingContextBase::drawArrays):
(WebCore::WebGLRenderingContextBase::drawElements):
(WebCore::WebGLRenderingContextBase::enableVertexAttribArray):
(WebCore::WebGLRenderingContextBase::vertexAttribPointer):
(WebCore::WebGLRenderingContextBase::initVertexAttrib0):
(WebCore::WebGLRenderingContextBase::drawArraysInstanced):
(WebCore::WebGLRenderingContextBase::drawElementsInstanced):

  • html/canvas/WebGLRenderingContextBase.h:
  • html/canvas/WebGLVertexArrayObjectBase.cpp:

(WebCore::WebGLVertexArrayObjectBase::WebGLVertexArrayObjectBase):
(WebCore::WebGLVertexArrayObjectBase::setVertexAttribEnabled):
(WebCore::WebGLVertexArrayObjectBase::setVertexAttribState):
(WebCore::WebGLVertexArrayObjectBase::unbindBuffer):
(WebCore::WebGLVertexArrayObjectBase::updateVertexAttrib0):
(WebCore::WebGLVertexArrayObjectBase::areAllEnabledAttribBuffersBound):

  • html/canvas/WebGLVertexArrayObjectBase.h:

(WebCore::WebGLVertexArrayObjectBase::getVertexAttribState):
Make VertexAttribState& a const so that we can maintain confidence
that the cached value of "all enabled attrib buffers are bound"
is maintained, e.g. that the callers cannot modify enabled or bound
buffer status.

LayoutTests:

Test webgl/2.0.y/conformance2/vertex_arrays/vertex-array-object.html explicitly
until 2.0.y suite is run by default. Same for 1.0.x.

  • TestExpectations:
  • fast/canvas/webgl/drawElements-empty-vertex-data-expected.txt:
  • fast/canvas/webgl/vertexAttribPointer-with-bad-offset-expected.txt:

Add console logs for the added errors.

  • platform/ios/TestExpectations:
  • platform/mac/TestExpectations:

For consistency, remove all skips for vertex-array-object tests.
They should now work.

Location:
trunk
Files:
14 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r282188 r282192  
     12021-09-08  Kimmo Kinnunen  <kkinnunen@apple.com>
     2
     3        webgl/2.0.y/conformance2/vertex_arrays/vertex-array-object.html fails
     4        https://bugs.webkit.org/show_bug.cgi?id=223360
     5        <rdar://problem/75774546>
     6
     7        Reviewed by Dean Jackson.
     8
     9        Test webgl/2.0.y/conformance2/vertex_arrays/vertex-array-object.html explicitly
     10        until 2.0.y suite is run by default. Same for 1.0.x.
     11
     12        * TestExpectations:
     13        * fast/canvas/webgl/drawElements-empty-vertex-data-expected.txt:
     14        * fast/canvas/webgl/vertexAttribPointer-with-bad-offset-expected.txt:
     15        Add console logs for the added errors.
     16        * platform/ios/TestExpectations:
     17        * platform/mac/TestExpectations:
     18        For consistency, remove all skips for vertex-array-object tests.
     19        They should now work.
     20
    1212021-09-08  Arcady Goldmints-Orlov  <agoldmints@igalia.com>
    222

  • trunk/LayoutTests/TestExpectations

    r282170 r282192  
    36923692webgl/2.0.y/conformance/glsl/misc/fragcolor-fragdata-invariant.html [ Pass ]
    36933693webgl/2.0.y/conformance2/context/constants-and-properties-2.html [ Pass ]
     3694webgl/2.0.y/conformance2/vertex_arrays/vertex-array-object.html [ Pass ]
     3695webgl/1.0.x/conformance/extensions/oes-vertex-array-object.html [ Pass ]
    36943696
    36953697# WebGL 1.0.3 and 2.0.0 tests where behavior is obsolete and WebKit contains implementation

  • trunk/LayoutTests/fast/canvas/webgl/drawElements-empty-vertex-data-expected.txt

    r256784 r282192  
     1CONSOLE MESSAGE: WebGL: INVALID_OPERATION: drawElements: no buffer is bound to enabled attribute
    12PASS: Unable to draw with invalid vertexAttribArray0

  • trunk/LayoutTests/fast/canvas/webgl/vertexAttribPointer-with-bad-offset-expected.txt

    r259900 r282192  
    11CONSOLE MESSAGE: WebGL: INVALID_VALUE: vertexAttribPointer: bad offset
     2CONSOLE MESSAGE: WebGL: INVALID_OPERATION: drawArrays: no buffer is bound to enabled attribute
    23PASS: vertexAttribPointer should have an error.
    34

  • trunk/LayoutTests/fast/canvas/webgl/webgl-drawarrays-crash-2-expected.txt

    r256784 r282192  
     1CONSOLE MESSAGE: WebGL: INVALID_OPERATION: drawArrays: no buffer is bound to enabled attribute
    12PASS. You didn't crash.
    23

  • trunk/LayoutTests/platform/ios/TestExpectations

    r282092 r282192  
    32023202webkit.org/b/207858 fast/canvas/webgl/uninitialized-test.html [ Pass Failure ]
    32033203webkit.org/b/207858 fast/canvas/webgl/oes-texture-float-linear.html [ Pass Failure ]
    3204 webkit.org/b/207858 fast/canvas/webgl/oes-vertex-array-object.html [ Pass Failure ]
    32053204webkit.org/b/207858 fast/canvas/webgl/tex-image-and-sub-image-2d-with-video.html [ Pass Failure ]
    32063205webkit.org/b/207858 webgl/1.0.3/conformance/context/context-lost-restored.html [ Pass Failure ]

  • trunk/LayoutTests/platform/mac/TestExpectations

    r282171 r282192  
    406406# Sending the mouse down event to the scrollbar starts a nested run loop which causes a hang.
    407407fast/events/mousedown-in-subframe-scrollbar.html [ Skip ]
    408 
    409 webkit.org/b/96828 fast/canvas/webgl/oes-vertex-array-object.html [ Skip ]
    410 webkit.org/b/96828 webgl/1.0.3/conformance/extensions/oes-vertex-array-object.html [ Skip ]
    411408
    412409http/tests/misc/willCacheResponse-delegate-callback.html [ Failure ]

  • trunk/Source/WebCore/ChangeLog

    r282191 r282192  
     12021-09-08  Kimmo Kinnunen  <kkinnunen@apple.com>
     2
     3        webgl/2.0.y/conformance2/vertex_arrays/vertex-array-object.html fails
     4        https://bugs.webkit.org/show_bug.cgi?id=223360
     5        <rdar://problem/75774546>
     6
     7        Reviewed by Dean Jackson.
     8
     9        Deleting a buffer will remove the buffer from currently active
     10        vertex array buffer bindings. This worked.
     11        Drawing with a vertex array object that has enabled indexes
     12        but missing buffers for those indexes should fail. This
     13        WebGL specific constraint is checked by ANGLE. However, WebCore
     14        relies on buffer deletion to update the ANGLE VAO. The deletion
     15        does not happen due to the buffers in the test being bound to
     16        multiple VAOs.
     17        Add this check also to WebCore.
     18
     19        Fixed tests:
     20        webgl/1.0.x/conformance/extensions/oes-vertex-array-object.html
     21        webgl/2.0.y/conformance2/vertex_arrays/vertex-array-object.html
     22
     23        * html/canvas/WebGL2RenderingContext.cpp:
     24        (WebCore::WebGL2RenderingContext::initializeVertexArrayObjects):
     25        (WebCore::WebGL2RenderingContext::drawRangeElements):
     26        * html/canvas/WebGLRenderingContext.cpp:
     27        (WebCore::WebGLRenderingContext::initializeVertexArrayObjects):
     28        * html/canvas/WebGLRenderingContextBase.cpp:
     29        (WebCore::WebGLRenderingContextBase::disableVertexAttribArray):
     30        (WebCore::WebGLRenderingContextBase::validateVertexArrayObject):
     31        (WebCore::WebGLRenderingContextBase::drawArrays):
     32        (WebCore::WebGLRenderingContextBase::drawElements):
     33        (WebCore::WebGLRenderingContextBase::enableVertexAttribArray):
     34        (WebCore::WebGLRenderingContextBase::vertexAttribPointer):
     35        (WebCore::WebGLRenderingContextBase::initVertexAttrib0):
     36        (WebCore::WebGLRenderingContextBase::drawArraysInstanced):
     37        (WebCore::WebGLRenderingContextBase::drawElementsInstanced):
     38        * html/canvas/WebGLRenderingContextBase.h:
     39        * html/canvas/WebGLVertexArrayObjectBase.cpp:
     40        (WebCore::WebGLVertexArrayObjectBase::WebGLVertexArrayObjectBase):
     41        (WebCore::WebGLVertexArrayObjectBase::setVertexAttribEnabled):
     42        (WebCore::WebGLVertexArrayObjectBase::setVertexAttribState):
     43        (WebCore::WebGLVertexArrayObjectBase::unbindBuffer):
     44        (WebCore::WebGLVertexArrayObjectBase::updateVertexAttrib0):
     45        (WebCore::WebGLVertexArrayObjectBase::areAllEnabledAttribBuffersBound):
     46        * html/canvas/WebGLVertexArrayObjectBase.h:
     47        (WebCore::WebGLVertexArrayObjectBase::getVertexAttribState):
     48        Make VertexAttribState& a const so that we can maintain confidence
     49        that the cached value of "all enabled attrib buffers are bound"
     50        is maintained, e.g. that the callers cannot modify enabled or bound
     51        buffer status.
     52
    1532021-09-08  Ryosuke Niwa  <rniwa@webkit.org>
    254

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

    r282160 r282192  
    224224void WebGL2RenderingContext::initializeVertexArrayObjects()
    225225{
     226#if !USE(ANGLE)
     227    if (!isGLES2Compliant())
     228        initVertexAttrib0();
     229#endif
    226230    m_defaultVertexArrayObject = WebGLVertexArrayObject::create(*this, WebGLVertexArrayObject::Type::Default);
    227231    addContextObject(*m_defaultVertexArrayObject);
    … …  
    231235    bindVertexArray(nullptr); // The default VAO was removed in OpenGL 3.3 but not from WebGL 2; bind the default for WebGL to use.
    232236#endif
    233 #if !USE(ANGLE)
    234     if (!isGLES2Compliant())
    235         initVertexAttrib0();
    236 #endif
     237
    237238}
    238239
    … …  
    17981799    if (isContextLostOrPending())
    17991800        return;
    1800 
     1801    if (!validateVertexArrayObject("drawRangeElements"))
     1802        return;
    18011803    m_context->drawRangeElements(mode, start, end, count, type, offset);
    18021804}

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

    r278253 r282192  
    125125void WebGLRenderingContext::initializeVertexArrayObjects()
    126126{
    127     m_defaultVertexArrayObject = WebGLVertexArrayObjectOES::create(*this, WebGLVertexArrayObjectOES::Type::Default);
    128     addContextObject(*m_defaultVertexArrayObject);
    129     m_boundVertexArrayObject = m_defaultVertexArrayObject;
    130127#if !USE(ANGLE)
    131128    if (!isGLES2Compliant())
    132129        initVertexAttrib0();
    133130#endif
     131    m_defaultVertexArrayObject = WebGLVertexArrayObjectOES::create(*this, WebGLVertexArrayObjectOES::Type::Default);
     132    addContextObject(*m_defaultVertexArrayObject);
     133    m_boundVertexArrayObject = m_defaultVertexArrayObject;
    134134}
    135135

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

    r282160 r282192  
    23382338        return;
    23392339    }
    2340 
    2341     WebGLVertexArrayObjectBase::VertexAttribState& state = m_boundVertexArrayObject->getVertexAttribState(index);
    2342     state.enabled = false;
    2343 
     2340    m_boundVertexArrayObject->setVertexAttribEnabled(index, false);
    23442341#if !USE(ANGLE)
    23452342    if (index > 0 || isGLES2Compliant())
    … …  
    23592356}
    23602357#endif
     2358
     2359bool WebGLRenderingContextBase::validateVertexArrayObject(const char* functionName)
     2360{
     2361    if (!m_boundVertexArrayObject->areAllEnabledAttribBuffersBound()) {
     2362        synthesizeGLError(GraphicsContextGL::INVALID_OPERATION, functionName, "no buffer is bound to enabled attribute");
     2363        return false;
     2364    }
     2365    return true;
     2366}
    23612367
    23622368bool WebGLRenderingContextBase::validateElementArraySize(GCGLsizei count, GCGLenum type, GCGLintptr offset)
    … …  
    27012707        return;
    27022708#endif
     2709    if (!validateVertexArrayObject("drawArrays"))
     2710        return;
    27032711
    27042712    if (m_currentProgram && InspectorInstrumentation::isWebGLProgramDisabled(*this, *m_currentProgram))
    … …  
    27642772        return;
    27652773#endif
     2774    if (!validateVertexArrayObject("drawElements"))
     2775        return;
    27662776
    27672777    if (m_currentProgram && InspectorInstrumentation::isWebGLProgramDisabled(*this, *m_currentProgram))
    … …  
    28322842        return;
    28332843    }
    2834 
    2835     WebGLVertexArrayObjectBase::VertexAttribState& state = m_boundVertexArrayObject->getVertexAttribState(index);
    2836     state.enabled = true;
    2837 
     2844    m_boundVertexArrayObject->setVertexAttribEnabled(index, true);
    28382845    m_context->enableVertexAttribArray(index);
    28392846}
    … …  
    63356342    }
    63366343    GCGLsizei bytesPerElement = size * typeSize;
    6337 
    63386344    m_boundVertexArrayObject->setVertexAttribState(locker, index, bytesPerElement, size, type, normalized, stride, static_cast<GCGLintptr>(offset), false, m_boundArrayBuffer.get());
    63396345    m_context->vertexAttribPointer(index, size, type, normalized, stride, static_cast<GCGLintptr>(offset));
    … …  
    75387544void WebGLRenderingContextBase::initVertexAttrib0()
    75397545{
    7540     WebGLVertexArrayObjectBase::VertexAttribState& state = m_boundVertexArrayObject->getVertexAttribState(0);
    7541    
    75427546    m_vertexAttrib0Buffer = createBuffer();
    75437547    m_context->bindBuffer(GraphicsContextGL::ARRAY_BUFFER, m_vertexAttrib0Buffer->object());
    75447548    m_context->bufferData(GraphicsContextGL::ARRAY_BUFFER, 0, GraphicsContextGL::DYNAMIC_DRAW);
    75457549    m_context->vertexAttribPointer(0, 4, GraphicsContextGL::FLOAT, false, 0, 0);
    7546     state.bufferBinding = m_vertexAttrib0Buffer;
    75477550    m_context->bindBuffer(GraphicsContextGL::ARRAY_BUFFER, 0);
    75487551    m_context->enableVertexAttribArray(0);
    … …  
    79377940        return;
    79387941#endif // !USE(ANGLE)
     7942    if (!validateVertexArrayObject("drawArraysInstanced"))
     7943        return;
    79397944
    79407945    clearIfComposited(ClearCallerDrawOrClear);
    … …  
    79767981        return;
    79777982#endif // !USE(ANGLE)
     7983    if (!validateVertexArrayObject("drawElementsInstanced"))
     7984        return;
    79787985
    79797986    clearIfComposited(ClearCallerDrawOrClear);

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

    r282160 r282192  
    513513    bool validateNPOTTextureLevel(GCGLsizei width, GCGLsizei height, GCGLint level, const char* functionName);
    514514#endif
     515    bool validateVertexArrayObject(const char* functionName);
    515516
    516517    // Adds a compressed texture format.

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

    r273138 r282192  
    4040{
    4141    m_vertexAttribState.resize(context.getMaxVertexAttribs());
     42#if !USE(ANGLE)
     43    updateVertexAttrib0();
     44#endif
    4245}
    4346
    … …  
    5154   
    5255}
     56void WebGLVertexArrayObjectBase::setVertexAttribEnabled(int index, bool flag)
     57{
     58    auto& state = m_vertexAttribState[index];
     59    if (state.enabled == flag)
     60        return;
     61    state.enabled = flag;
     62    if (!state.validateBinding())
     63        m_allEnabledAttribBuffersBoundCache = false;
     64    else
     65        m_allEnabledAttribBuffersBoundCache.reset();
     66}
    5367
    5468void WebGLVertexArrayObjectBase::setVertexAttribState(const AbstractLocker& locker, GCGLuint index, GCGLsizei bytesPerElement, GCGLint size, GCGLenum type, GCGLboolean normalized, GCGLsizei stride, GCGLintptr offset, bool isInteger, WebGLBuffer* buffer)
    5569{
    56     GCGLsizei validatedStride = stride ? stride : bytesPerElement;
    57    
    5870    auto& state = m_vertexAttribState[index];
    59    
     71    bool bindingWasValid = state.validateBinding();
    6072    if (buffer)
    6173        buffer->onAttached();
    6274    if (state.bufferBinding)
    6375        state.bufferBinding->onDetached(locker, context()->graphicsContextGL());
    64    
    6576    state.bufferBinding = buffer;
     77    if (!state.validateBinding())
     78        m_allEnabledAttribBuffersBoundCache = false;
     79    else if (!bindingWasValid)
     80        m_allEnabledAttribBuffersBoundCache.reset();
    6681    state.bytesPerElement = bytesPerElement;
    6782    state.size = size;
    6883    state.type = type;
    6984    state.normalized = normalized;
    70     state.stride = validatedStride;
     85    state.stride = stride ? stride : bytesPerElement;
    7186    state.originalStride = stride;
    7287    state.offset = offset;
    … …  
    8196    }
    8297   
    83     for (size_t i = 0; i < m_vertexAttribState.size(); ++i) {
    84         auto& state = m_vertexAttribState[i];
     98    for (auto& state : m_vertexAttribState) {
    8599        if (state.bufferBinding == &buffer) {
    86100            buffer.onDetached(locker, context()->graphicsContextGL());
    87            
    88 #if !USE(ANGLE)
    89             if (!i && !context()->isGLES2Compliant()) {
    90                 state.bufferBinding = context()->m_vertexAttrib0Buffer;
    91                 state.bufferBinding->onAttached();
    92                 state.bytesPerElement = 0;
    93                 state.size = 4;
    94                 state.type = GraphicsContextGL::FLOAT;
    95                 state.normalized = false;
    96                 state.stride = 16;
    97                 state.originalStride = 0;
    98                 state.offset = 0;
    99             } else
    100 #endif
    101                 state.bufferBinding = nullptr;
     101            state.bufferBinding = nullptr;
     102            if (!state.validateBinding())
     103                m_allEnabledAttribBuffersBoundCache = false;
    102104        }
    103105    }
     106#if !USE(ANGLE)
     107    updateVertexAttrib0();
     108#endif
    104109}
     110
     111#if !USE(ANGLE)
     112void WebGLVertexArrayObjectBase::updateVertexAttrib0()
     113{
     114    auto& state = m_vertexAttribState[0];
     115    if (!state.bufferBinding && !context()->isGLES2Compliant()) {
     116        state.bufferBinding = context()->m_vertexAttrib0Buffer;
     117        state.bufferBinding->onAttached();
     118        state.bytesPerElement = 0;
     119        state.size = 4;
     120        state.type = GraphicsContextGL::FLOAT;
     121        state.normalized = false;
     122        state.stride = 16;
     123        state.originalStride = 0;
     124        state.offset = 0;
     125        m_allEnabledAttribBuffersBoundCache.reset();
     126    }
     127}
     128#endif
    105129
    106130void WebGLVertexArrayObjectBase::setVertexAttribDivisor(GCGLuint index, GCGLuint divisor)
    … …  
    116140}
    117141
     142bool WebGLVertexArrayObjectBase::areAllEnabledAttribBuffersBound()
     143{
     144    if (!m_allEnabledAttribBuffersBoundCache) {
     145        m_allEnabledAttribBuffersBoundCache = [&] {
     146            for (auto const& state : m_vertexAttribState) {
     147                if (!state.validateBinding())
     148                    return false;
     149            }
     150            return true;
     151        }();
     152    }
     153    return *m_allEnabledAttribBuffersBoundCache;
     154}
     155
    118156}
    119157

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

    r273138 r282192  
    3131#include "WebGLBuffer.h"
    3232#include "WebGLContextObject.h"
     33#include <optional>
    3334
    3435namespace JSC {
    … …  
    7273    void setElementArrayBuffer(const WTF::AbstractLocker&, WebGLBuffer*);
    7374
    74     VertexAttribState& getVertexAttribState(int index) { return m_vertexAttribState[index]; }
     75    void setVertexAttribEnabled(int index, bool flag);
     76    const VertexAttribState& getVertexAttribState(int index) { return m_vertexAttribState[index]; }
    7577    void setVertexAttribState(const WTF::AbstractLocker&, GCGLuint, GCGLsizei, GCGLint, GCGLenum, GCGLboolean, GCGLsizei, GCGLintptr, bool, WebGLBuffer*);
    7678    void unbindBuffer(const WTF::AbstractLocker&, WebGLBuffer&);
    … …  
    8082    void addMembersToOpaqueRoots(const WTF::AbstractLocker&, JSC::AbstractSlotVisitor&);
    8183
     84    bool areAllEnabledAttribBuffersBound();
    8285protected:
    8386    WebGLVertexArrayObjectBase(WebGLRenderingContextBase&, Type);
    8487    void deleteObjectImpl(const WTF::AbstractLocker&, GraphicsContextGL*, PlatformGLObject) override = 0;
     88#if !USE(ANGLE)
     89    void updateVertexAttrib0();
     90#endif
    8591
    8692    Type m_type;
    … …  
    8894    RefPtr<WebGLBuffer> m_boundElementArrayBuffer;
    8995    Vector<VertexAttribState> m_vertexAttribState;
     96    std::optional<bool> m_allEnabledAttribBuffersBoundCache;
    9097};
    9198
Note: See TracChangeset for help on using the changeset viewer.