Nuke Builders Part 2: remove all builder code from wire
This removes blocks of code that were obviously builder-specific but
also removes the ObjectStorage::valid member that was used to implement
the maybe monad on the wire server side. This is no longer needed since
dawn_native handles the maybe monad internally now.
BUG=dawn:125
Change-Id: I8c30daae9fc70853bc1996d85a860b4877c5976c
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/6161
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
diff --git a/src/dawn_wire/client/ObjectAllocator.h b/src/dawn_wire/client/ObjectAllocator.h
index 3b1b60e..7caacbe 100644
--- a/src/dawn_wire/client/ObjectAllocator.h
+++ b/src/dawn_wire/client/ObjectAllocator.h
@@ -25,8 +25,6 @@
class Client;
class Device;
- // TODO(cwallez@chromium.org): Do something with objects before they are destroyed ?
- // - Call still uncalled builder callbacks
template <typename T>
class ObjectAllocator {
using ObjectOwner =
diff --git a/src/dawn_wire/client/ObjectBase.h b/src/dawn_wire/client/ObjectBase.h
index 42d0805..b97b040 100644
--- a/src/dawn_wire/client/ObjectBase.h
+++ b/src/dawn_wire/client/ObjectBase.h
@@ -21,24 +21,6 @@
class Device;
- struct BuilderCallbackData {
- bool Call(DawnBuilderErrorStatus status, const char* message) {
- if (canCall && callback != nullptr) {
- canCall = true;
- callback(status, message, userdata1, userdata2);
- return true;
- }
-
- return false;
- }
-
- // For help with development, prints all builder errors by default.
- DawnBuilderErrorCallback callback = nullptr;
- DawnCallbackUserdata userdata1 = 0;
- DawnCallbackUserdata userdata2 = 0;
- bool canCall = true;
- };
-
// All non-Device objects of the client side have:
// - A pointer to the device to get where to serialize commands
// - The external reference count
@@ -51,8 +33,6 @@
Device* device;
uint32_t refcount;
uint32_t id;
-
- BuilderCallbackData builderCallback;
};
}} // namespace dawn_wire::client
diff --git a/src/dawn_wire/server/ObjectStorage.h b/src/dawn_wire/server/ObjectStorage.h
index 4bfce34..689dc0b 100644
--- a/src/dawn_wire/server/ObjectStorage.h
+++ b/src/dawn_wire/server/ObjectStorage.h
@@ -15,7 +15,6 @@
#ifndef DAWNWIRE_SERVER_OBJECTSTORAGE_H_
#define DAWNWIRE_SERVER_OBJECTSTORAGE_H_
-#include "dawn_wire/TypeTraits_autogen.h"
#include "dawn_wire/WireCmd_autogen.h"
#include <algorithm>
@@ -29,26 +28,17 @@
T handle;
uint32_t serial = 0;
- // Used by the error-propagation mechanism to know if this object is an error.
- // TODO(cwallez@chromium.org): this is doubling the memory usage of
- // std::vector<ObjectDataBase> consider making it a special marker value in handle instead.
- bool valid;
// Whether this object has been allocated, used by the KnownObjects queries
// TODO(cwallez@chromium.org): make this an internal bit vector in KnownObjects.
bool allocated;
};
// Stores what the backend knows about the type.
- template <typename T, bool IsBuilder = IsBuilderType<T>::value>
+ template <typename T>
struct ObjectData : public ObjectDataBase<T> {};
- template <typename T>
- struct ObjectData<T, true> : public ObjectDataBase<T> {
- ObjectHandle builtObject = ObjectHandle{0, 0};
- };
-
template <>
- struct ObjectData<DawnBuffer, false> : public ObjectDataBase<DawnBuffer> {
+ struct ObjectData<DawnBuffer> : public ObjectDataBase<DawnBuffer> {
void* mappedData = nullptr;
size_t mappedDataSize = 0;
};
@@ -65,7 +55,6 @@
// KnownObjects for ID 0.
Data reservation;
reservation.handle = nullptr;
- reservation.valid = false;
reservation.allocated = false;
mKnown.push_back(reservation);
}
@@ -109,7 +98,6 @@
Data data;
data.allocated = true;
- data.valid = false;
data.handle = nullptr;
if (id >= mKnown.size()) {
@@ -136,7 +124,6 @@
for (Data& data : mKnown) {
if (data.allocated && data.handle != nullptr) {
objects.push_back(data.handle);
- data.valid = false;
data.allocated = false;
data.handle = nullptr;
}
diff --git a/src/dawn_wire/server/Server.cpp b/src/dawn_wire/server/Server.cpp
index 6d420ba..fdb6cec 100644
--- a/src/dawn_wire/server/Server.cpp
+++ b/src/dawn_wire/server/Server.cpp
@@ -21,7 +21,6 @@
// The client-server knowledge is bootstrapped with device 1.
auto* deviceData = DeviceObjects().Allocate(1);
deviceData->handle = device;
- deviceData->valid = true;
auto userdata = static_cast<DawnCallbackUserdata>(reinterpret_cast<intptr_t>(this));
mProcs.deviceSetErrorCallback(device, ForwardDeviceError, userdata);
@@ -43,7 +42,6 @@
data->handle = texture;
data->serial = generation;
- data->valid = true;
data->allocated = true;
// The texture is externally owned so it shouldn't be destroyed when we receive a destroy
diff --git a/src/dawn_wire/server/ServerBuffer.cpp b/src/dawn_wire/server/ServerBuffer.cpp
index cf337bb..22fd60e 100644
--- a/src/dawn_wire/server/ServerBuffer.cpp
+++ b/src/dawn_wire/server/ServerBuffer.cpp
@@ -52,17 +52,6 @@
auto userdata = static_cast<uint64_t>(reinterpret_cast<uintptr_t>(data));
- if (!buffer->valid) {
- // Fake the buffer returning a failure, data will be freed in this call.
- if (isWrite) {
- ForwardBufferMapWriteAsync(DAWN_BUFFER_MAP_ASYNC_STATUS_ERROR, nullptr, 0,
- userdata);
- } else {
- ForwardBufferMapReadAsync(DAWN_BUFFER_MAP_ASYNC_STATUS_ERROR, nullptr, 0, userdata);
- }
- return true;
- }
-
if (isWrite) {
mProcs.bufferMapWriteAsync(buffer->handle, ForwardBufferMapWriteAsync, userdata);
} else {
@@ -79,8 +68,7 @@
}
auto* buffer = BufferObjects().Get(bufferId);
- if (buffer == nullptr || !buffer->valid || buffer->mappedData == nullptr ||
- buffer->mappedDataSize != count) {
+ if (buffer == nullptr || buffer->mappedData == nullptr || buffer->mappedDataSize != count) {
return false;
}
diff --git a/src/tests/unittests/wire/WireArgumentTests.cpp b/src/tests/unittests/wire/WireArgumentTests.cpp
index 10dbd89..d6c5d73 100644
--- a/src/tests/unittests/wire/WireArgumentTests.cpp
+++ b/src/tests/unittests/wire/WireArgumentTests.cpp
@@ -158,12 +158,14 @@
pipelineDescriptor.depthStencilState = &depthStencilState;
dawnDeviceCreateRenderPipeline(device, &pipelineDescriptor);
+
+ DawnRenderPipeline apiDummyPipeline = api.GetNewRenderPipeline();
EXPECT_CALL(api,
DeviceCreateRenderPipeline(
apiDevice, MatchesLambda([](const DawnRenderPipelineDescriptor* desc) -> bool {
return desc->vertexStage->entryPoint == std::string("main");
})))
- .WillOnce(Return(nullptr));
+ .WillOnce(Return(apiDummyPipeline));
FlushClient();
}
@@ -247,6 +249,8 @@
descriptor.borderColor = DAWN_BORDER_COLOR_TRANSPARENT_BLACK;
dawnDeviceCreateSampler(device, &descriptor);
+
+ DawnSampler apiDummySampler = api.GetNewSampler();
EXPECT_CALL(api, DeviceCreateSampler(
apiDevice, MatchesLambda([](const DawnSamplerDescriptor* desc) -> bool {
return desc->nextInChain == nullptr &&
@@ -260,7 +264,7 @@
desc->borderColor == DAWN_BORDER_COLOR_TRANSPARENT_BLACK &&
desc->lodMinClamp == kLodMin && desc->lodMaxClamp == kLodMax;
})))
- .WillOnce(Return(nullptr));
+ .WillOnce(Return(apiDummySampler));
FlushClient();
}
@@ -281,6 +285,8 @@
descriptor.bindGroupLayouts = &bgl;
dawnDeviceCreatePipelineLayout(device, &descriptor);
+
+ DawnPipelineLayout apiDummyLayout = api.GetNewPipelineLayout();
EXPECT_CALL(api, DeviceCreatePipelineLayout(
apiDevice,
MatchesLambda([apiBgl](const DawnPipelineLayoutDescriptor* desc) -> bool {
@@ -288,7 +294,7 @@
desc->bindGroupLayoutCount == 1 &&
desc->bindGroupLayouts[0] == apiBgl;
})))
- .WillOnce(Return(nullptr));
+ .WillOnce(Return(apiDummyLayout));
FlushClient();
}
diff --git a/src/tests/unittests/wire/WireBufferMappingTests.cpp b/src/tests/unittests/wire/WireBufferMappingTests.cpp
index e1037ff..db3c61c 100644
--- a/src/tests/unittests/wire/WireBufferMappingTests.cpp
+++ b/src/tests/unittests/wire/WireBufferMappingTests.cpp
@@ -73,29 +73,16 @@
mockBufferMapReadCallback = std::make_unique<StrictMock<MockBufferMapReadCallback>>();
mockBufferMapWriteCallback = std::make_unique<StrictMock<MockBufferMapWriteCallback>>();
- {
- DawnBufferDescriptor descriptor;
- descriptor.nextInChain = nullptr;
+ DawnBufferDescriptor descriptor;
+ descriptor.nextInChain = nullptr;
- apiBuffer = api.GetNewBuffer();
- buffer = dawnDeviceCreateBuffer(device, &descriptor);
+ apiBuffer = api.GetNewBuffer();
+ buffer = dawnDeviceCreateBuffer(device, &descriptor);
- EXPECT_CALL(api, DeviceCreateBuffer(apiDevice, _))
- .WillOnce(Return(apiBuffer))
- .RetiresOnSaturation();
- FlushClient();
- }
- {
- DawnBufferDescriptor descriptor;
- descriptor.nextInChain = nullptr;
-
- errorBuffer = dawnDeviceCreateBuffer(device, &descriptor);
-
- EXPECT_CALL(api, DeviceCreateBuffer(apiDevice, _))
- .WillOnce(Return(nullptr))
- .RetiresOnSaturation();
- FlushClient();
- }
+ EXPECT_CALL(api, DeviceCreateBuffer(apiDevice, _))
+ .WillOnce(Return(apiBuffer))
+ .RetiresOnSaturation();
+ FlushClient();
}
void TearDown() override {
@@ -117,9 +104,6 @@
// A successfully created buffer
DawnBuffer buffer;
DawnBuffer apiBuffer;
-
- // An buffer that wasn't created on the server side
- DawnBuffer errorBuffer;
};
// MapRead-specific tests
@@ -171,35 +155,24 @@
FlushServer();
}
-// Check mapping for reading a buffer that didn't get created on the server side
-TEST_F(WireBufferMappingTests, MappingForReadErrorBuffer) {
- DawnCallbackUserdata userdata = 8655;
- dawnBufferMapReadAsync(errorBuffer, ToMockBufferMapReadCallback, userdata);
-
- FlushClient();
-
- EXPECT_CALL(*mockBufferMapReadCallback,
- Call(DAWN_BUFFER_MAP_ASYNC_STATUS_ERROR, nullptr, 0, userdata))
- .Times(1);
-
- FlushServer();
-
- dawnBufferUnmap(errorBuffer);
-
- FlushClient();
-}
-
// Check that the map read callback is called with UNKNOWN when the buffer is destroyed before the
// request is finished
TEST_F(WireBufferMappingTests, DestroyBeforeReadRequestEnd) {
DawnCallbackUserdata userdata = 8656;
- dawnBufferMapReadAsync(errorBuffer, ToMockBufferMapReadCallback, userdata);
+ dawnBufferMapReadAsync(buffer, ToMockBufferMapReadCallback, userdata);
+ // Return success
+ EXPECT_CALL(api, OnBufferMapReadAsyncCallback(apiBuffer, _, _))
+ .WillOnce(InvokeWithoutArgs([&]() {
+ api.CallMapReadCallback(apiBuffer, DAWN_BUFFER_MAP_ASYNC_STATUS_SUCCESS, nullptr, 0);
+ }));
+
+ // Destroy before the client gets the success, so the callback is called with unknown.
EXPECT_CALL(*mockBufferMapReadCallback,
Call(DAWN_BUFFER_MAP_ASYNC_STATUS_UNKNOWN, nullptr, 0, userdata))
.Times(1);
-
- dawnBufferRelease(errorBuffer);
+ dawnBufferRelease(buffer);
+ EXPECT_CALL(api, BufferRelease(apiBuffer));
FlushClient();
FlushServer();
@@ -381,35 +354,27 @@
FlushServer();
}
-// Check mapping for writing a buffer that didn't get created on the server side
-TEST_F(WireBufferMappingTests, MappingForWriteErrorBuffer) {
- DawnCallbackUserdata userdata = 8655;
- dawnBufferMapWriteAsync(errorBuffer, ToMockBufferMapWriteCallback, userdata);
-
- FlushClient();
-
- EXPECT_CALL(*mockBufferMapWriteCallback,
- Call(DAWN_BUFFER_MAP_ASYNC_STATUS_ERROR, nullptr, 0, userdata))
- .Times(1);
-
- FlushServer();
-
- dawnBufferUnmap(errorBuffer);
-
- FlushClient();
-}
-
// Check that the map write callback is called with UNKNOWN when the buffer is destroyed before the
// request is finished
TEST_F(WireBufferMappingTests, DestroyBeforeWriteRequestEnd) {
DawnCallbackUserdata userdata = 8656;
- dawnBufferMapWriteAsync(errorBuffer, ToMockBufferMapWriteCallback, userdata);
+ dawnBufferMapWriteAsync(buffer, ToMockBufferMapWriteCallback, userdata);
+ // Return success
+ EXPECT_CALL(api, OnBufferMapWriteAsyncCallback(apiBuffer, _, _))
+ .WillOnce(InvokeWithoutArgs([&]() {
+ api.CallMapWriteCallback(apiBuffer, DAWN_BUFFER_MAP_ASYNC_STATUS_SUCCESS, nullptr, 0);
+ }));
+
+ // Destroy before the client gets the success, so the callback is called with unknown.
EXPECT_CALL(*mockBufferMapWriteCallback,
Call(DAWN_BUFFER_MAP_ASYNC_STATUS_UNKNOWN, nullptr, 0, userdata))
.Times(1);
+ dawnBufferRelease(buffer);
+ EXPECT_CALL(api, BufferRelease(apiBuffer));
- dawnBufferRelease(errorBuffer);
+ FlushClient();
+ FlushServer();
}
// Check the map read callback is called with UNKNOWN when the map request would have worked, but
diff --git a/src/tests/unittests/wire/WireInjectTextureTests.cpp b/src/tests/unittests/wire/WireInjectTextureTests.cpp
index b8e0cbe..23da573 100644
--- a/src/tests/unittests/wire/WireInjectTextureTests.cpp
+++ b/src/tests/unittests/wire/WireInjectTextureTests.cpp
@@ -37,7 +37,8 @@
ASSERT_TRUE(GetWireServer()->InjectTexture(apiTexture, reservation.id, reservation.generation));
dawnTextureCreateDefaultTextureView(reservation.texture);
- EXPECT_CALL(api, TextureCreateDefaultTextureView(apiTexture)).WillOnce(Return(nullptr));
+ DawnTextureView apiDummyView = api.GetNewTextureView();
+ EXPECT_CALL(api, TextureCreateDefaultTextureView(apiTexture)).WillOnce(Return(apiDummyView));
FlushClient();
}
diff --git a/src/tests/unittests/wire/WireOptionalTests.cpp b/src/tests/unittests/wire/WireOptionalTests.cpp
index 55b67ed..29972b1 100644
--- a/src/tests/unittests/wire/WireOptionalTests.cpp
+++ b/src/tests/unittests/wire/WireOptionalTests.cpp
@@ -49,6 +49,8 @@
bgDesc.bindings = &binding;
dawnDeviceCreateBindGroup(device, &bgDesc);
+
+ DawnBindGroup apiDummyBindGroup = api.GetNewBindGroup();
EXPECT_CALL(api, DeviceCreateBindGroup(
apiDevice, MatchesLambda([](const DawnBindGroupDescriptor* desc) -> bool {
return desc->nextInChain == nullptr && desc->bindingCount == 1 &&
@@ -57,7 +59,7 @@
desc->bindings[0].buffer == nullptr &&
desc->bindings[0].textureView == nullptr;
})))
- .WillOnce(Return(nullptr));
+ .WillOnce(Return(apiDummyBindGroup));
FlushClient();
}
@@ -147,6 +149,8 @@
// First case: depthStencilState is not null.
pipelineDescriptor.depthStencilState = &depthStencilState;
dawnDeviceCreateRenderPipeline(device, &pipelineDescriptor);
+
+ DawnRenderPipeline apiDummyPipeline = api.GetNewRenderPipeline();
EXPECT_CALL(
api,
DeviceCreateRenderPipeline(
@@ -172,7 +176,7 @@
desc->depthStencilState->stencilReadMask == 0xff &&
desc->depthStencilState->stencilWriteMask == 0xff;
})))
- .WillOnce(Return(nullptr));
+ .WillOnce(Return(apiDummyPipeline));
FlushClient();
@@ -184,7 +188,7 @@
apiDevice, MatchesLambda([](const DawnRenderPipelineDescriptor* desc) -> bool {
return desc->depthStencilState == nullptr;
})))
- .WillOnce(Return(nullptr));
+ .WillOnce(Return(apiDummyPipeline));
FlushClient();
}
