Use typed integers for the Buffer MapRequestID
This will prevent mixing it up with other serial types in the future.
Bug: dawn:442
Change-Id: Ie655d57722fcd79c82acc5aac429aed2c2741c3e
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/28920
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
diff --git a/src/dawn_native/Buffer.cpp b/src/dawn_native/Buffer.cpp
index aa9af0a..9ec821e 100644
--- a/src/dawn_native/Buffer.cpp
+++ b/src/dawn_native/Buffer.cpp
@@ -138,7 +138,7 @@
BufferBase::~BufferBase() {
if (mState == BufferState::Mapped) {
ASSERT(!IsError());
- CallMapCallback(mMapSerial, WGPUBufferMapAsyncStatus_DestroyedBeforeCallback);
+ CallMapCallback(mLastMapID, WGPUBufferMapAsyncStatus_DestroyedBeforeCallback);
}
}
@@ -212,9 +212,9 @@
}
}
- void BufferBase::CallMapCallback(uint32_t serial, WGPUBufferMapAsyncStatus status) {
+ void BufferBase::CallMapCallback(MapRequestID mapID, WGPUBufferMapAsyncStatus status) {
ASSERT(!IsError());
- if (mMapCallback != nullptr && serial == mMapSerial) {
+ if (mMapCallback != nullptr && mapID == mLastMapID) {
// Tag the callback as fired before firing it, otherwise it could fire a second time if
// for example buffer.Unmap() is called inside the application-provided callback.
WGPUBufferMapCallback callback = mMapCallback;
@@ -249,8 +249,7 @@
}
ASSERT(!IsError());
- // TODO(cwallez@chromium.org): what to do on wraparound? Could cause crashes.
- mMapSerial++;
+ mLastMapID++;
mMapMode = mode;
mMapOffset = offset;
mMapSize = size;
@@ -259,12 +258,12 @@
mState = BufferState::Mapped;
if (GetDevice()->ConsumedError(MapAsyncImpl(mode, offset, size))) {
- CallMapCallback(mMapSerial, WGPUBufferMapAsyncStatus_DeviceLost);
+ CallMapCallback(mLastMapID, WGPUBufferMapAsyncStatus_DeviceLost);
return;
}
MapRequestTracker* tracker = GetDevice()->GetMapRequestTracker();
- tracker->Track(this, mMapSerial);
+ tracker->Track(this, mLastMapID);
}
void* BufferBase::GetMappedRange(size_t offset, size_t size) {
@@ -350,7 +349,7 @@
// completed before the Unmap.
// Callbacks are not fired if there is no callback registered, so this is correct for
// mappedAtCreation = true.
- CallMapCallback(mMapSerial, callbackStatus);
+ CallMapCallback(mLastMapID, callbackStatus);
UnmapImpl();
mMapCallback = nullptr;
@@ -514,8 +513,8 @@
mState = BufferState::Destroyed;
}
- void BufferBase::OnMapCommandSerialFinished(uint32_t mapSerial) {
- CallMapCallback(mapSerial, WGPUBufferMapAsyncStatus_Success);
+ void BufferBase::OnMapRequestCompleted(MapRequestID mapID) {
+ CallMapCallback(mapID, WGPUBufferMapAsyncStatus_Success);
}
bool BufferBase::IsDataInitialized() const {
diff --git a/src/dawn_native/Buffer.h b/src/dawn_native/Buffer.h
index fc6353c..675b071 100644
--- a/src/dawn_native/Buffer.h
+++ b/src/dawn_native/Buffer.h
@@ -17,6 +17,7 @@
#include "dawn_native/Error.h"
#include "dawn_native/Forward.h"
+#include "dawn_native/IntegerTypes.h"
#include "dawn_native/ObjectBase.h"
#include "dawn_native/dawn_platform.h"
@@ -52,7 +53,7 @@
wgpu::BufferUsage GetUsage() const;
MaybeError MapAtCreation();
- void OnMapCommandSerialFinished(uint32_t mapSerial);
+ void OnMapRequestCompleted(MapRequestID mapID);
MaybeError ValidateCanUseOnQueueNow() const;
@@ -91,7 +92,7 @@
virtual bool IsCPUWritableAtCreation() const = 0;
MaybeError CopyFromStagingBuffer();
void* GetMappedRangeInternal(bool writable, size_t offset, size_t size);
- void CallMapCallback(uint32_t serial, WGPUBufferMapAsyncStatus status);
+ void CallMapCallback(MapRequestID mapID, WGPUBufferMapAsyncStatus status);
MaybeError ValidateMap(wgpu::BufferUsage requiredUsage,
WGPUBufferMapAsyncStatus* status) const;
@@ -113,7 +114,7 @@
WGPUBufferMapCallback mMapCallback = nullptr;
void* mMapUserdata = 0;
- uint32_t mMapSerial = 0;
+ MapRequestID mLastMapID = MapRequestID(0);
wgpu::MapMode mMapMode = wgpu::MapMode::None;
size_t mMapOffset = 0;
size_t mMapSize = 0;
diff --git a/src/dawn_native/IntegerTypes.h b/src/dawn_native/IntegerTypes.h
index bd31c79..3b72722 100644
--- a/src/dawn_native/IntegerTypes.h
+++ b/src/dawn_native/IntegerTypes.h
@@ -43,6 +43,17 @@
constexpr VertexAttributeLocation kMaxVertexAttributesTyped =
VertexAttributeLocation(kMaxVertexAttributes);
+ // Serials are 64bit integers that are incremented by one each time to produce unique values.
+ // Some serials (like queue serials) are compared numerically to know which one is before
+ // another, while some serials are only checked for equality. We call serials only checked
+ // for equality IDs.
+
+ // Buffer mapping requests are stored outside of the buffer while they are being processed and
+ // cannot be invalidated. Instead they are associated with an ID, and when a map request is
+ // finished, the mapping callback is fired only if its ID matches the ID if the last request
+ // that was sent.
+ using MapRequestID = TypedInteger<struct MapRequestIDT, uint64_t>;
+
} // namespace dawn_native
#endif // DAWNNATIVE_INTEGERTYPES_H_
diff --git a/src/dawn_native/MapRequestTracker.cpp b/src/dawn_native/MapRequestTracker.cpp
index faf7034..ef7c49c 100644
--- a/src/dawn_native/MapRequestTracker.cpp
+++ b/src/dawn_native/MapRequestTracker.cpp
@@ -13,12 +13,11 @@
// limitations under the License.
#include "dawn_native/MapRequestTracker.h"
+
#include "dawn_native/Buffer.h"
#include "dawn_native/Device.h"
namespace dawn_native {
- struct Request;
- class DeviceBase;
MapRequestTracker::MapRequestTracker(DeviceBase* device) : mDevice(device) {
}
@@ -27,10 +26,10 @@
ASSERT(mInflightRequests.Empty());
}
- void MapRequestTracker::Track(BufferBase* buffer, uint32_t mapSerial) {
+ void MapRequestTracker::Track(BufferBase* buffer, MapRequestID mapID) {
Request request;
request.buffer = buffer;
- request.mapSerial = mapSerial;
+ request.id = mapID;
mInflightRequests.Enqueue(std::move(request), mDevice->GetPendingCommandSerial());
mDevice->AddFutureCallbackSerial(mDevice->GetPendingCommandSerial());
@@ -38,7 +37,7 @@
void MapRequestTracker::Tick(Serial finishedSerial) {
for (auto& request : mInflightRequests.IterateUpTo(finishedSerial)) {
- request.buffer->OnMapCommandSerialFinished(request.mapSerial);
+ request.buffer->OnMapRequestCompleted(request.id);
}
mInflightRequests.ClearUpTo(finishedSerial);
}
diff --git a/src/dawn_native/MapRequestTracker.h b/src/dawn_native/MapRequestTracker.h
index 06eae3d..2afad01 100644
--- a/src/dawn_native/MapRequestTracker.h
+++ b/src/dawn_native/MapRequestTracker.h
@@ -16,7 +16,9 @@
#define DAWNNATIVE_MAPREQUESTTRACKER_H_
#include "common/SerialQueue.h"
-#include "dawn_native/Device.h"
+#include "dawn_native/Buffer.h"
+#include "dawn_native/Forward.h"
+#include "dawn_native/IntegerTypes.h"
namespace dawn_native {
@@ -25,7 +27,7 @@
MapRequestTracker(DeviceBase* device);
~MapRequestTracker();
- void Track(BufferBase* buffer, uint32_t mapSerial);
+ void Track(BufferBase* buffer, MapRequestID mapID);
void Tick(Serial finishedSerial);
private:
@@ -33,7 +35,7 @@
struct Request {
Ref<BufferBase> buffer;
- uint32_t mapSerial;
+ MapRequestID id;
};
SerialQueue<Serial, Request> mInflightRequests;
};
