Changeset 139928 in webkit


Ignore:
Timestamp:
Jan 16, 2013 3:08:53 PM (11 years ago)
Author:
kbr@google.com
Message:

Simplify validation and data copying in WebGLBuffer
https://bugs.webkit.org/show_bug.cgi?id=106975

Reviewed by Dean Jackson.

Re-landing after testing locally; was not able to reproduce the
crash seen once with the earlier patch.

No new tests; covered by existing tests. Ran WebGL layout tests and conformance tests.

  • html/canvas/WebGLBuffer.cpp:

(WebCore::WebGLBuffer::associateBufferDataImpl):

Take (void*, int) pair instead of ArrayBuffer and offset. Simplifies code significantly.

(WebCore::WebGLBuffer::associateBufferData):

Pass down base pointers and sizes rather than ArrayBuffer and optional offset.

(WebCore::WebGLBuffer::associateBufferSubDataImpl):

Take (void*, int) pair instead of ArrayBuffer and offset. Simplifies code significantly.

(WebCore::WebGLBuffer::associateBufferSubData):

Pass down base pointers and sizes rather than ArrayBuffer and optional offset.

  • html/canvas/WebGLBuffer.h:

(WebGLBuffer):

Change signatures of associateBufferDataImpl and associateBufferSubDataImpl.

Location:
trunk/Source/WebCore
Files:
3 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r139927 r139928  
     12013-01-16  Kenneth Russell  <kbr@google.com>
     2
     3        Simplify validation and data copying in WebGLBuffer
     4        https://bugs.webkit.org/show_bug.cgi?id=106975
     5
     6        Reviewed by Dean Jackson.
     7
     8        Re-landing after testing locally; was not able to reproduce the
     9        crash seen once with the earlier patch.
     10
     11        No new tests; covered by existing tests. Ran WebGL layout tests and conformance tests.
     12
     13        * html/canvas/WebGLBuffer.cpp:
     14        (WebCore::WebGLBuffer::associateBufferDataImpl):
     15            Take (void*, int) pair instead of ArrayBuffer and offset. Simplifies code significantly.
     16        (WebCore::WebGLBuffer::associateBufferData):
     17            Pass down base pointers and sizes rather than ArrayBuffer and optional offset.
     18        (WebCore::WebGLBuffer::associateBufferSubDataImpl):
     19            Take (void*, int) pair instead of ArrayBuffer and offset. Simplifies code significantly.
     20        (WebCore::WebGLBuffer::associateBufferSubData):
     21            Pass down base pointers and sizes rather than ArrayBuffer and optional offset.
     22        * html/canvas/WebGLBuffer.h:
     23        (WebGLBuffer):
     24            Change signatures of associateBufferDataImpl and associateBufferSubDataImpl.
     25
    1262013-01-16  Timothy Hatcher  <timothy@apple.com>
    227
  • trunk/Source/WebCore/html/canvas/WebGLBuffer.cpp

    r139923 r139928  
    6262}
    6363
    64 bool WebGLBuffer::associateBufferDataImpl(ArrayBuffer* array, GC3Dintptr byteOffset, GC3Dsizeiptr byteLength)
    65 {
    66     if (byteLength < 0 || byteOffset < 0)
    67         return false;
    68 
    69     if (array && byteLength) {
    70         CheckedInt<GC3Dintptr> checkedOffset(byteOffset);
    71         CheckedInt<GC3Dsizeiptr> checkedLength(byteLength);
    72         CheckedInt<GC3Dintptr> checkedMax = checkedOffset + checkedLength;
    73         if (!checkedMax.isValid() || checkedMax.value() > static_cast<int32_t>(array->byteLength()))
    74             return false;
    75     }
     64bool WebGLBuffer::associateBufferDataImpl(const void* data, GC3Dsizeiptr byteLength)
     65{
     66    if (byteLength < 0)
     67        return false;
    7668
    7769    switch (m_target) {
     
    8577                return false;
    8678            }
    87             if (array) {
     79            if (data) {
    8880                // We must always clone the incoming data because client-side
    8981                // modifications without calling bufferData or bufferSubData
    9082                // must never be able to change the validation results.
    91                 memcpy(static_cast<unsigned char*>(m_elementArrayBuffer->data()),
    92                        static_cast<unsigned char*>(array->data()) + byteOffset,
    93                        byteLength);
     83                memcpy(m_elementArrayBuffer->data(), data, byteLength);
    9484            }
    9585        } else
     
    10696bool WebGLBuffer::associateBufferData(GC3Dsizeiptr size)
    10797{
    108     if (size < 0)
    109         return false;
    110     return associateBufferDataImpl(0, 0, size);
     98    return associateBufferDataImpl(0, size);
    11199}
    112100
     
    115103    if (!array)
    116104        return false;
    117     return associateBufferDataImpl(array, 0, array->byteLength());
     105    return associateBufferDataImpl(array ? array->data() : 0, array ? array->byteLength() : 0);
    118106}
    119107
     
    122110    if (!array)
    123111        return false;
    124     return associateBufferDataImpl(array->buffer().get(), array->byteOffset(), array->byteLength());
    125 }
    126 
    127 bool WebGLBuffer::associateBufferSubDataImpl(GC3Dintptr offset, ArrayBuffer* array, GC3Dintptr arrayByteOffset, GC3Dsizeiptr byteLength)
    128 {
    129     if (!array || offset < 0 || arrayByteOffset < 0 || byteLength < 0)
     112    return associateBufferDataImpl(array ? array->baseAddress() : 0, array ? array->byteLength() : 0);
     113}
     114
     115bool WebGLBuffer::associateBufferSubDataImpl(GC3Dintptr offset, const void* data, GC3Dsizeiptr byteLength)
     116{
     117    if (!data || offset < 0 || byteLength < 0)
    130118        return false;
    131119
    132120    if (byteLength) {
    133121        CheckedInt<GC3Dintptr> checkedBufferOffset(offset);
    134         CheckedInt<GC3Dintptr> checkedArrayOffset(arrayByteOffset);
    135         CheckedInt<GC3Dsizeiptr> checkedLength(byteLength);
    136         CheckedInt<GC3Dintptr> checkedArrayMax = checkedArrayOffset + checkedLength;
    137         CheckedInt<GC3Dintptr> checkedBufferMax = checkedBufferOffset + checkedLength;
    138         if (!checkedArrayMax.isValid() || checkedArrayMax.value() > static_cast<int32_t>(array->byteLength()) || !checkedBufferMax.isValid() || checkedBufferMax.value() > m_byteLength)
     122        CheckedInt<GC3Dsizeiptr> checkedDataLength(byteLength);
     123        CheckedInt<GC3Dintptr> checkedBufferMax = checkedBufferOffset + checkedDataLength;
     124        if (!checkedBufferMax.isValid() || offset > m_byteLength || checkedBufferMax.value() > m_byteLength)
    139125            return false;
    140126    }
     
    146132            if (!m_elementArrayBuffer)
    147133                return false;
    148             memcpy(static_cast<unsigned char*>(m_elementArrayBuffer->data()) + offset,
    149                    static_cast<unsigned char*>(array->data()) + arrayByteOffset,
    150                    byteLength);
     134            memcpy(static_cast<unsigned char*>(m_elementArrayBuffer->data()) + offset, data, byteLength);
    151135        }
    152136        return true;
     
    162146    if (!array)
    163147        return false;
    164     return associateBufferSubDataImpl(offset, array, 0, array->byteLength());
     148    return associateBufferSubDataImpl(offset, array->data(), array->byteLength());
    165149}
    166150
     
    169153    if (!array)
    170154        return false;
    171     return associateBufferSubDataImpl(offset, array->buffer().get(), array->byteOffset(), array->byteLength());
     155    return associateBufferSubDataImpl(offset, array->baseAddress(), array->byteLength());
    172156}
    173157
  • trunk/Source/WebCore/html/canvas/WebGLBuffer.h

    r139923 r139928  
    9696
    9797    // Helper function called by the three associateBufferData().
    98     bool associateBufferDataImpl(ArrayBuffer* array, GC3Dintptr byteOffset, GC3Dsizeiptr byteLength);
     98    bool associateBufferDataImpl(const void* data, GC3Dsizeiptr byteLength);
    9999    // Helper function called by the two associateBufferSubData().
    100     bool associateBufferSubDataImpl(GC3Dintptr offset, ArrayBuffer* array, GC3Dintptr arrayByteOffset, GC3Dsizeiptr byteLength);
     100    bool associateBufferSubDataImpl(GC3Dintptr offset, const void* data, GC3Dsizeiptr byteLength);
    101101};
    102102
Note: See TracChangeset for help on using the changeset viewer.