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

Changeset 249183 in webkit


Ignore:
Timestamp:
Aug 27, 2019, 4:39:38 PM (7 years ago)
Author:
Justin Fan
Message:

[WebGPU] Implement GPUErrors for and relax GPUBuffer validation rules
​https://bugs.webkit.org/show_bug.cgi?id=200852

Reviewed by Dean Jackson.

Source/WebCore:

Fix incorrect usage validation during GPUBuffer creation.
Implement GPUError reporting for GPUBuffer creation and methods.

Test: webgpu/buffer-errors.html

  • Modules/webgpu/WebGPUBuffer.cpp:

(WebCore::WebGPUBuffer::create):
(WebCore::WebGPUBuffer::WebGPUBuffer):
(WebCore::WebGPUBuffer::unmap):
(WebCore::WebGPUBuffer::destroy):
(WebCore::WebGPUBuffer::rejectOrRegisterPromiseCallback):

  • Modules/webgpu/WebGPUBuffer.h: Now inherits from GPUObjectBase.
  • Modules/webgpu/WebGPUDevice.cpp:

(WebCore::WebGPUDevice::createBuffer const):
(WebCore::WebGPUDevice::createBufferMapped const):

  • platform/graphics/gpu/GPUBuffer.h: No longer inherits from GPUObjectBase.
  • platform/graphics/gpu/GPUObjectBase.h:

(WebCore::GPUObjectBase::errorScopes):
(WebCore::GPUObjectBase::generateError): Deleted.

  • platform/graphics/gpu/cocoa/GPUBufferMetal.mm:

(WebCore::GPUBuffer::validateBufferUsage):
(WebCore::GPUBuffer::tryCreate): Alignment issue should be general WebGPU requirement.
(WebCore::GPUBuffer::GPUBuffer):
(WebCore::GPUBuffer::~GPUBuffer): Must do cleanup without generating errors.
(WebCore::GPUBuffer::registerMappingCallback):
(WebCore::GPUBuffer::copyStagingBufferToGPU):
(WebCore::GPUBuffer::unmap):
(WebCore::GPUBuffer::destroy):

LayoutTests:

Add a test to ensure GPUBuffer errors are generated properly.

  • webgpu/buffer-errors-expected.txt: Added.
  • webgpu/buffer-errors.html: Added.
Location:
trunk
Files:
2 added
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r249176 r249183  
     12019-08-27  Justin Fan  <justin_fan@apple.com>
     2
     3        [WebGPU] Implement GPUErrors for and relax GPUBuffer validation rules
     4        https://bugs.webkit.org/show_bug.cgi?id=200852
     5
     6        Reviewed by Dean Jackson.
     7
     8        Add a test to ensure GPUBuffer errors are generated properly.
     9
     10        * webgpu/buffer-errors-expected.txt: Added.
     11        * webgpu/buffer-errors.html: Added.
     12
    1132019-08-27  Russell Epstein  <repstein@apple.com>
    214

  • trunk/Source/WebCore/ChangeLog

    r249177 r249183  
     12019-08-27  Justin Fan  <justin_fan@apple.com>
     2
     3        [WebGPU] Implement GPUErrors for and relax GPUBuffer validation rules
     4        https://bugs.webkit.org/show_bug.cgi?id=200852
     5
     6        Reviewed by Dean Jackson.
     7
     8        Fix incorrect usage validation during GPUBuffer creation.
     9        Implement GPUError reporting for GPUBuffer creation and methods.
     10
     11        Test: webgpu/buffer-errors.html
     12
     13        * Modules/webgpu/WebGPUBuffer.cpp:
     14        (WebCore::WebGPUBuffer::create):
     15        (WebCore::WebGPUBuffer::WebGPUBuffer):
     16        (WebCore::WebGPUBuffer::unmap):
     17        (WebCore::WebGPUBuffer::destroy):
     18        (WebCore::WebGPUBuffer::rejectOrRegisterPromiseCallback):
     19        * Modules/webgpu/WebGPUBuffer.h: Now inherits from GPUObjectBase.
     20        * Modules/webgpu/WebGPUDevice.cpp:
     21        (WebCore::WebGPUDevice::createBuffer const):
     22        (WebCore::WebGPUDevice::createBufferMapped const):
     23        * platform/graphics/gpu/GPUBuffer.h: No longer inherits from GPUObjectBase.
     24        * platform/graphics/gpu/GPUObjectBase.h:
     25        (WebCore::GPUObjectBase::errorScopes):
     26        (WebCore::GPUObjectBase::generateError): Deleted.
     27        * platform/graphics/gpu/cocoa/GPUBufferMetal.mm:
     28        (WebCore::GPUBuffer::validateBufferUsage):
     29        (WebCore::GPUBuffer::tryCreate): Alignment issue should be general WebGPU requirement.
     30        (WebCore::GPUBuffer::GPUBuffer):
     31        (WebCore::GPUBuffer::~GPUBuffer): Must do cleanup without generating errors.
     32        (WebCore::GPUBuffer::registerMappingCallback):
     33        (WebCore::GPUBuffer::copyStagingBufferToGPU):
     34        (WebCore::GPUBuffer::unmap):
     35        (WebCore::GPUBuffer::destroy):
     36
    1372019-08-27  Zalan Bujtas  <zalan@apple.com>
    238

  • trunk/Source/WebCore/Modules/webgpu/WebGPUBuffer.cpp

    r246217 r249183  
    2929#if ENABLE(WEBGPU)
    3030
    31 #include "Logging.h"
     31#include "GPUErrorScopes.h"
     32#include <wtf/text/StringConcatenate.h>
    3233
    3334namespace WebCore {
    3435
    35 Ref<WebGPUBuffer> WebGPUBuffer::create(RefPtr<GPUBuffer>&& buffer)
     36Ref<WebGPUBuffer> WebGPUBuffer::create(RefPtr<GPUBuffer>&& buffer, GPUErrorScopes& errorScopes)
    3637{
    37     return adoptRef(*new WebGPUBuffer(WTFMove(buffer)));
     38    return adoptRef(*new WebGPUBuffer(WTFMove(buffer), errorScopes));
    3839}
    3940
    40 WebGPUBuffer::WebGPUBuffer(RefPtr<GPUBuffer>&& buffer)
    41     : m_buffer(WTFMove(buffer))
     41WebGPUBuffer::WebGPUBuffer(RefPtr<GPUBuffer>&& buffer, GPUErrorScopes& errorScopes)
     42    : GPUObjectBase(makeRef(errorScopes))
     43    , m_buffer(WTFMove(buffer))
    4244{
    4345}
    … …  
    5557void WebGPUBuffer::unmap()
    5658{
     59    errorScopes().setErrorPrefix("GPUBuffer.unmap(): ");
     60
    5761    if (!m_buffer)
    58         LOG(WebGPU, "GPUBuffer::unmap(): Invalid operation!");
     62        errorScopes().generatePrefixedError("Invalid operation: invalid GPUBuffer!");
    5963    else
    60         m_buffer->unmap();
     64        m_buffer->unmap(&errorScopes());
    6165}
    6266
    6367void WebGPUBuffer::destroy()
    6468{
     69    errorScopes().setErrorPrefix("GPUBuffer.destroy(): ");
     70
    6571    if (!m_buffer)
    66         LOG(WebGPU, "GPUBuffer::destroy(): Invalid operation!");
     72        errorScopes().generatePrefixedError("Invalid operation!");
    6773    else {
    68         m_buffer->destroy();
     74        m_buffer->destroy(&errorScopes());
    6975        m_buffer = nullptr;
    7076    }
    … …  
    7379void WebGPUBuffer::rejectOrRegisterPromiseCallback(BufferMappingPromise&& promise, bool isRead)
    7480{
     81    errorScopes().setErrorPrefix(makeString("GPUBuffer.map", isRead ? "Read" : "Write", "Async(): "));
     82
    7583    if (!m_buffer) {
    76         LOG(WebGPU, "GPUBuffer::map%sAsync(): Invalid operation!", isRead ? "Read" : "Write");
     84        errorScopes().generatePrefixedError("Invalid operation: invalid GPUBuffer!");
    7785        promise.reject();
    7886        return;
    7987    }
    8088
    81     m_buffer->registerMappingCallback([promise = WTFMove(promise)] (JSC::ArrayBuffer* arrayBuffer) mutable {
     89    m_buffer->registerMappingCallback([promise = WTFMove(promise), protectedErrorScopes = makeRef(errorScopes())] (JSC::ArrayBuffer* arrayBuffer) mutable {
    8290        if (arrayBuffer)
    8391            promise.resolve(*arrayBuffer);
    84         else
     92        else {
     93            protectedErrorScopes->generateError("", GPUErrorFilter::OutOfMemory);
    8594            promise.reject();
    86     }, isRead);
     95        }
     96    }, isRead, errorScopes());
    8797}
    8898

  • trunk/Source/WebCore/Modules/webgpu/WebGPUBuffer.h

    r246217 r249183  
    3030#include "GPUBuffer.h"
    3131#include "GPUBufferUsage.h"
     32#include "GPUObjectBase.h"
    3233#include "JSDOMPromiseDeferred.h"
    33 #include <wtf/RefCounted.h>
    3434#include <wtf/RefPtr.h>
    3535
    … …  
    4242struct GPUBufferDescriptor;
    4343
    44 class WebGPUBuffer : public RefCounted<WebGPUBuffer> {
     44class WebGPUBuffer : public GPUObjectBase {
    4545public:
    46     static Ref<WebGPUBuffer> create(RefPtr<GPUBuffer>&&);
     46    static Ref<WebGPUBuffer> create(RefPtr<GPUBuffer>&&, GPUErrorScopes&);
    4747
    4848    GPUBuffer* buffer() { return m_buffer.get(); }
    … …  
    5656
    5757private:
    58     explicit WebGPUBuffer(RefPtr<GPUBuffer>&&);
     58    explicit WebGPUBuffer(RefPtr<GPUBuffer>&&, GPUErrorScopes&);
    5959
    6060    void rejectOrRegisterPromiseCallback(BufferMappingPromise&&, bool);

  • trunk/Source/WebCore/Modules/webgpu/WebGPUDevice.cpp

    r249131 r249183  
    9292
    9393    auto buffer = m_device->tryCreateBuffer(descriptor, GPUBufferMappedOption::NotMapped, m_errorScopes);
    94     return WebGPUBuffer::create(WTFMove(buffer));
     94    return WebGPUBuffer::create(WTFMove(buffer), m_errorScopes);
    9595}
    9696
    … …  
    107107    }
    108108
    109     auto webBuffer = WebGPUBuffer::create(WTFMove(buffer));
     109    auto webBuffer = WebGPUBuffer::create(WTFMove(buffer), m_errorScopes);
    110110    auto wrappedWebBuffer = toJS(&state, JSC::jsCast<JSDOMGlobalObject*>(state.lexicalGlobalObject()), webBuffer);
    111111

  • trunk/Source/WebCore/platform/graphics/gpu/GPUBuffer.h

    r248606 r249183  
    3030#include "DeferrableTask.h"
    3131#include "GPUBufferUsage.h"
    32 #include "GPUObjectBase.h"
    3332#include <wtf/Function.h>
    3433#include <wtf/OptionSet.h>
    … …  
    5049
    5150class GPUDevice;
     51class GPUErrorScopes;
    5252
    5353struct GPUBufferDescriptor;
    … …  
    6262using PlatformBufferSmartPtr = RetainPtr<PlatformBuffer>;
    6363
    64 class GPUBuffer : public GPUObjectBase {
     64class GPUBuffer : public RefCounted<GPUBuffer> {
    6565public:
    6666    enum class State {
    … …  
    9595
    9696    using MappingCallback = WTF::Function<void(JSC::ArrayBuffer*)>;
    97     void registerMappingCallback(MappingCallback&&, bool);
    98     void unmap();
    99     void destroy();
     97    void registerMappingCallback(MappingCallback&&, bool, GPUErrorScopes&);
     98    void unmap(GPUErrorScopes*);
     99    void destroy(GPUErrorScopes*);
    100100
    101101private:
    … …  
    112112    };
    113113
    114     GPUBuffer(PlatformBufferSmartPtr&&, GPUDevice&, size_t, OptionSet<GPUBufferUsage::Flags>, GPUBufferMappedOption, GPUErrorScopes&);
     114    GPUBuffer(PlatformBufferSmartPtr&&, GPUDevice&, size_t, OptionSet<GPUBufferUsage::Flags>, GPUBufferMappedOption);
    115115    static bool validateBufferUsage(const GPUDevice&, OptionSet<GPUBufferUsage::Flags>, GPUErrorScopes&);
    116116
    … …  
    118118    JSC::ArrayBuffer* stagingBufferForWrite();
    119119    void runMappingCallback();
    120     void copyStagingBufferToGPU();
     120    void copyStagingBufferToGPU(GPUErrorScopes*);
    121121
    122122    bool isMapWrite() const { return m_usage.contains(GPUBufferUsage::Flags::MapWrite); }

  • trunk/Source/WebCore/platform/graphics/gpu/GPUObjectBase.h

    r247500 r249183  
    3434
    3535class GPUObjectBase : public RefCounted<GPUObjectBase> {
    36 public:
    37     void generateError(const String& message, GPUErrorFilter filter = GPUErrorFilter::Validation)
    38     {
    39         m_errorScopes->generateError(message, filter);
    40     }
    41 
    4236protected:
    4337    GPUObjectBase(Ref<GPUErrorScopes>&& reporter)
    4438        : m_errorScopes(WTFMove(reporter)) { }
     39
     40    GPUErrorScopes& errorScopes() { return m_errorScopes; }
    4541
    4642private:

  • trunk/Source/WebCore/platform/graphics/gpu/cocoa/GPUBufferMetal.mm

    r248606 r249183  
    3131#import "GPUBufferDescriptor.h"
    3232#import "GPUDevice.h"
    33 #import "Logging.h"
     33#import "GPUErrorScopes.h"
    3434#import <JavaScriptCore/ArrayBuffer.h>
    3535#import <Metal/Metal.h>
    … …  
    4747{
    4848    if (!device.platformDevice()) {
    49         LOG(WebGPU, "GPUBuffer::tryCreate(): Invalid GPUDevice!");
     49        errorScopes.generatePrefixedError("Invalid GPUDevice!");
    5050        return false;
    5151    }
    … …  
    5353    if (usage.containsAll({ GPUBufferUsage::Flags::MapWrite, GPUBufferUsage::Flags::MapRead })) {
    5454        errorScopes.generatePrefixedError("Buffer cannot have both MAP_READ and MAP_WRITE usage!");
    55         return false;
    56     }
    57 
    58     if (usage.containsAny(readOnlyFlags) && (usage & GPUBufferUsage::Flags::Storage)) {
    59         LOG(WebGPU, "GPUBuffer::tryCreate(): Buffer cannot have both STORAGE and a read-only usage!");
    6055        return false;
    6156    }
    … …  
    7772        return nullptr;
    7873
    79 #if PLATFORM(MAC)
    8074    // copyBufferToBuffer calls require 4-byte alignment. "Unmapping" a mapped-on-creation GPUBuffer
    8175    // that is otherwise unmappable requires such a copy to upload data.
    … …  
    8377        && !usage.containsAny({ GPUBufferUsage::Flags::MapWrite, GPUBufferUsage::Flags::MapRead })
    8478        && descriptor.size % 4) {
    85         LOG(WebGPU, "GPUBuffer::tryCreate(): Data must be aligned to a multiple of 4 bytes!");
    86         return nullptr;
    87     }
    88 #endif
     79        errorScopes.generatePrefixedError("Data must be aligned to a multiple of 4 bytes!");
     80        return nullptr;
     81    }
    8982
    9083    // FIXME: Metal best practices: Read-only one-time-use data less than 4 KB should not allocate a MTLBuffer and be used in [MTLCommandEncoder set*Bytes] calls instead.
    … …  
    108101    }
    109102
    110     return adoptRef(*new GPUBuffer(WTFMove(mtlBuffer), device, size, usage, isMapped, errorScopes));
    111 }
    112 
    113 GPUBuffer::GPUBuffer(RetainPtr<MTLBuffer>&& buffer, GPUDevice& device, size_t size, OptionSet<GPUBufferUsage::Flags> usage, GPUBufferMappedOption isMapped, GPUErrorScopes& errorScopes)
    114     : GPUObjectBase(makeRef(errorScopes))
    115     , m_platformBuffer(WTFMove(buffer))
     103    return adoptRef(*new GPUBuffer(WTFMove(mtlBuffer), device, size, usage, isMapped));
     104}
     105
     106GPUBuffer::GPUBuffer(RetainPtr<MTLBuffer>&& buffer, GPUDevice& device, size_t size, OptionSet<GPUBufferUsage::Flags> usage, GPUBufferMappedOption isMapped)
     107    : m_platformBuffer(WTFMove(buffer))
    116108    , m_device(makeRef(device))
    117109    , m_byteLength(size)
    … …  
    126118GPUBuffer::~GPUBuffer()
    127119{
    128     destroy();
     120    destroy(nullptr);
    129121}
    130122
    … …  
    178170#endif // USE(METAL)
    179171
    180 void GPUBuffer::registerMappingCallback(MappingCallback&& callback, bool isRead)
     172void GPUBuffer::registerMappingCallback(MappingCallback&& callback, bool isRead, GPUErrorScopes& errorScopes)
    181173{
    182174    // Reject if request is invalid.
    183175    if (isRead && !isMapReadable()) {
    184         LOG(WebGPU, "GPUBuffer::mapReadAsync(): Invalid operation!");
     176        errorScopes.generatePrefixedError("Invalid operation!");
    185177        callback(nullptr);
    186178        return;
    187179    }
    188180    if (!isRead && !isMapWriteable()) {
    189         LOG(WebGPU, "GPUBuffer::mapWriteAsync(): Invalid operation!");
     181        errorScopes.generatePrefixedError("Invalid operation!");
    190182        callback(nullptr);
    191183        return;
    … …  
    227219}
    228220
    229 void GPUBuffer::copyStagingBufferToGPU()
     221void GPUBuffer::copyStagingBufferToGPU(GPUErrorScopes* errorScopes)
    230222{
    231223    MTLCommandQueue *queue;
    … …  
    240232    END_BLOCK_OBJC_EXCEPTIONS;
    241233
    242     if (!stagingMtlBuffer) {
    243         LOG(WebGPU, "GPUBuffer::unmap(): Unable to create staging buffer!");
     234    if (!stagingMtlBuffer && errorScopes) {
     235        errorScopes->generateError("", GPUErrorFilter::OutOfMemory);
    244236        return;
    245237    }
    … …  
    259251}
    260252
    261 void GPUBuffer::unmap()
    262 {
    263     if (!m_isMappedFromCreation && !isMappable()) {
    264         LOG(WebGPU, "GPUBuffer::unmap(): Invalid operation: buffer is not mappable!");
     253void GPUBuffer::unmap(GPUErrorScopes* errorScopes)
     254{
     255    if (!m_isMappedFromCreation && !isMappable() && errorScopes) {
     256        errorScopes->generatePrefixedError("Invalid operation: GPUBuffer is not mappable!");
    265257        return;
    266258    }
    … …  
    272264            memcpy(m_platformBuffer.get().contents, m_stagingBuffer->data(), m_byteLength);
    273265        } else if (m_isMappedFromCreation)
    274             copyStagingBufferToGPU();
     266            copyStagingBufferToGPU(errorScopes);
    275267
    276268        m_isMappedFromCreation = false;
    … …  
    285277}
    286278
    287 void GPUBuffer::destroy()
     279void GPUBuffer::destroy(GPUErrorScopes* errorScopes)
    288280{
    289281    if (state() == State::Mapped)
    290         unmap();
     282        unmap(errorScopes);
    291283
    292284    m_platformBuffer = nullptr;
Note: See TracChangeset for help on using the changeset viewer.