Skip to content

Commit

Permalink
Some error handling and assert improvements, trying to understand #10662
Browse files Browse the repository at this point in the history
  • Loading branch information
hrydgard committed Mar 1, 2018
1 parent c7dcb7c commit ee752f5
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 15 deletions.
18 changes: 11 additions & 7 deletions Common/Vulkan/VulkanImage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ bool VulkanTexture::CreateDirect(VkCommandBuffer cmd, int w, int h, int numMips,

VkResult res = vkCreateImage(vulkan_->GetDevice(), &image_create_info, NULL, &image_);
if (res != VK_SUCCESS) {
assert(res == VK_ERROR_OUT_OF_HOST_MEMORY || res == VK_ERROR_OUT_OF_DEVICE_MEMORY || res == VK_ERROR_TOO_MANY_OBJECTS);
_assert_(res == VK_ERROR_OUT_OF_HOST_MEMORY || res == VK_ERROR_OUT_OF_DEVICE_MEMORY || res == VK_ERROR_TOO_MANY_OBJECTS);
return false;
}

Expand All @@ -79,12 +79,14 @@ bool VulkanTexture::CreateDirect(VkCommandBuffer cmd, int w, int h, int numMips,

// Find memory type - don't specify any mapping requirements
bool pass = vulkan_->MemoryTypeFromProperties(mem_reqs.memoryTypeBits, VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT, &mem_alloc.memoryTypeIndex);
assert(pass);
_assert_(pass);

res = vkAllocateMemory(vulkan_->GetDevice(), &mem_alloc, NULL, &mem_);
if (res != VK_SUCCESS) {
_assert_msg_(G3D, res != VK_ERROR_TOO_MANY_OBJECTS, "Too many Vulkan memory objects!");
assert(res == VK_ERROR_OUT_OF_HOST_MEMORY || res == VK_ERROR_OUT_OF_DEVICE_MEMORY || res == VK_ERROR_TOO_MANY_OBJECTS);
_assert_(res == VK_ERROR_OUT_OF_HOST_MEMORY || res == VK_ERROR_OUT_OF_DEVICE_MEMORY || res == VK_ERROR_TOO_MANY_OBJECTS);
vkDestroyImage(vulkan_->GetDevice(), image_, nullptr);
image_ = nullptr;
return false;
}

Expand All @@ -93,7 +95,8 @@ bool VulkanTexture::CreateDirect(VkCommandBuffer cmd, int w, int h, int numMips,

res = vkBindImageMemory(vulkan_->GetDevice(), image_, mem_, offset_);
if (res != VK_SUCCESS) {
assert(res == VK_ERROR_OUT_OF_HOST_MEMORY || res == VK_ERROR_OUT_OF_DEVICE_MEMORY || res == VK_ERROR_TOO_MANY_OBJECTS);
// This leaks the image and memory. Should not really happen though...
_assert_(res == VK_ERROR_OUT_OF_HOST_MEMORY || res == VK_ERROR_OUT_OF_DEVICE_MEMORY || res == VK_ERROR_TOO_MANY_OBJECTS);
return false;
}

Expand All @@ -108,8 +111,8 @@ bool VulkanTexture::CreateDirect(VkCommandBuffer cmd, int w, int h, int numMips,
break;
default:
// If you planned to use UploadMip, you want VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL. After the
// upload, you can transition.
assert(false);
// upload, you can transition using EndCreate.
_assert_(false);
break;
}
}
Expand All @@ -135,7 +138,8 @@ bool VulkanTexture::CreateDirect(VkCommandBuffer cmd, int w, int h, int numMips,

res = vkCreateImageView(vulkan_->GetDevice(), &view_info, NULL, &view_);
if (res != VK_SUCCESS) {
assert(res == VK_ERROR_OUT_OF_HOST_MEMORY || res == VK_ERROR_OUT_OF_DEVICE_MEMORY || res == VK_ERROR_TOO_MANY_OBJECTS);
// This leaks the image.
_assert_(res == VK_ERROR_OUT_OF_HOST_MEMORY || res == VK_ERROR_OUT_OF_DEVICE_MEMORY || res == VK_ERROR_TOO_MANY_OBJECTS);
return false;
}
return true;
Expand Down
5 changes: 2 additions & 3 deletions UI/TextureUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,7 @@ bool ManagedTexture::LoadFromFileData(const uint8_t *data, size_t dataSize, Imag
if (image[i])
free(image[i]);
}

return true;
return texture_ != nullptr;
}

bool ManagedTexture::LoadFromFile(const std::string &filename, ImageFileType type, bool generateMips) {
Expand Down Expand Up @@ -172,4 +171,4 @@ std::unique_ptr<ManagedTexture> CreateTextureFromFileData(Draw::DrawContext *dra
ManagedTexture *mtex = new ManagedTexture(draw);
mtex->LoadFromFileData(data, size, type, generateMips);
return std::move(std::unique_ptr<ManagedTexture>(mtex));
}
}
17 changes: 13 additions & 4 deletions ext/native/thin3d/thin3d_vulkan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ class VKTexture : public Texture {
VKTexture(VulkanContext *vulkan, VkCommandBuffer cmd, VulkanPushBuffer *pushBuffer, const TextureDesc &desc, VulkanDeviceAllocator *alloc)
: vulkan_(vulkan), mipLevels_(desc.mipLevels), format_(desc.format) {
bool result = Create(cmd, pushBuffer, desc, alloc);
assert(result);
_assert_(result);
}

~VKTexture() {
Expand Down Expand Up @@ -650,8 +650,8 @@ enum class TextureState {

bool VKTexture::Create(VkCommandBuffer cmd, VulkanPushBuffer *push, const TextureDesc &desc, VulkanDeviceAllocator *alloc) {
// Zero-sized textures not allowed.
assert(desc.width * desc.height * desc.depth > 0);
assert(push);
_assert_(desc.width * desc.height * desc.depth > 0); // remember to set depth to 1!
_assert_(push);
format_ = desc.format;
mipLevels_ = desc.mipLevels;
width_ = desc.width;
Expand All @@ -667,7 +667,10 @@ bool VKTexture::Create(VkCommandBuffer cmd, VulkanPushBuffer *push, const Textur
// Gonna have to generate some, which requires TRANSFER_SRC
usageBits |= VK_IMAGE_USAGE_TRANSFER_SRC_BIT;
}
vkTex_->CreateDirect(cmd, width_, height_, mipLevels_, vulkanFormat, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, usageBits);
if (!vkTex_->CreateDirect(cmd, width_, height_, mipLevels_, vulkanFormat, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, usageBits)) {
ELOG("Failed to create VulkanTexture: %dx%dx%d fmt %d, %d levels", width_, height_, depth_, (int)vulkanFormat, mipLevels_);
return false;
}
if (desc.initData.size()) {
int w = width_;
int h = height_;
Expand Down Expand Up @@ -1017,6 +1020,12 @@ InputLayout *VKContext::CreateInputLayout(const InputLayoutDesc &desc) {
}

Texture *VKContext::CreateTexture(const TextureDesc &desc) {
if (!push_) {
// Too early! Fail.
ELOG("Can't create textures before the first frame has started.");
return nullptr;
}
_assert_(renderManager_.GetInitCmd());
return new VKTexture(vulkan_, renderManager_.GetInitCmd(), push_, desc, allocator_);
}

Expand Down
2 changes: 1 addition & 1 deletion ext/native/ui/ui_context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ void UIContext::Init(Draw::DrawContext *thin3d, Draw::Pipeline *uipipe, Draw::Pi

void UIContext::BeginFrame() {
if (!uitexture_) {
uitexture_ = CreateTextureFromFile(draw_, "ui_atlas.zim", ImageFileType::ZIM);
uitexture_ = CreateTextureFromFile(draw_, "ui_atlas.zim", ImageFileType::ZIM, false);
if (!uitexture_) {
PanicAlert("Failed to load ui_atlas.zim.\n\nPlace it in the directory \"assets\" under your PPSSPP directory.");
FLOG("Failed to load ui_atlas.zim");
Expand Down

6 comments on commit ee752f5

@lim2222
Copy link

@lim2222 lim2222 commented on ee752f5 Mar 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i am here

@hrydgard
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lim2222 try it when it's on the buildbot please and report back

@lim2222
Copy link

@lim2222 lim2222 commented on ee752f5 Mar 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried v1.5.4-621-g1e940f497,vulkan can start,need new adb logs?

@hrydgard
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh it works now? If so, I'm a bit confused, but hey. If you still have any kind of problems with 621, post logs.

@lim2222
Copy link

@lim2222 lim2222 commented on ee752f5 Mar 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Every things fine now,THX bro.
Here is my new logs.
ADB NEW.txt

@hrydgard
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks!

Please sign in to comment.