Skip to content

Commit

Permalink
Vulkan: Make sure textures/samplers are unbound at the end of Present…
Browse files Browse the repository at this point in the history
…ationCommon::CopyToOutput.

Validation caught an issue where old stuff lingered in sampler 1 and texture 1.

Bug probably introduced in #12921, but could also be others.
  • Loading branch information
hrydgard committed Jul 13, 2020
1 parent 53b2ab1 commit 131a1ee
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 2 deletions.
1 change: 1 addition & 0 deletions GPU/Common/PresentationCommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -748,6 +748,7 @@ void PresentationCommon::CopyToOutput(OutputFlags flags, int uvRotation, float u
DoRelease(srcFramebuffer_);
DoRelease(srcTexture_);

// Unbinds all textures and samplers too, needed since sometimes a MakePixelTexture is deleted etc.
draw_->BindPipeline(nullptr);
}

Expand Down
4 changes: 2 additions & 2 deletions ext/native/thin3d/thin3d.h
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,7 @@ class DrawContext {
BindTextures(stage, 1, textures);
} // from sampler 0 and upwards

// Call this with 0 to signal that you have been drawing on your own, and need the state reset on the next pipeline bind.
// Call this with nullptr to signal that you're done with the stuff you've bound, like textures and samplers and stuff.
virtual void BindPipeline(Pipeline *pipeline) = 0;

virtual void Draw(int vertexCount, int offset) = 0;
Expand All @@ -652,7 +652,7 @@ class DrawContext {

// Frame management (for the purposes of sync and resource management, necessary with modern APIs). Default implementations here.
virtual void BeginFrame() {}
virtual void EndFrame() {}
virtual void EndFrame() = 0;
virtual void WipeQueue() {}

// This should be avoided as much as possible, in favor of clearing when binding a render target, which is native
Expand Down
6 changes: 6 additions & 0 deletions ext/native/thin3d/thin3d_d3d11.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ class D3D11DrawContext : public DrawContext {
stencilRefDirty_ = true;
}

void EndFrame() override;

void Draw(int vertexCount, int offset) override;
void DrawIndexed(int vertexCount, int offset) override;
void DrawUP(const void *vdata, int vertexCount) override;
Expand Down Expand Up @@ -363,6 +365,10 @@ void D3D11DrawContext::HandleEvent(Event ev, int width, int height, void *param1
}
}

void D3D11DrawContext::EndFrame() {
curPipeline_ = nullptr;
}

void D3D11DrawContext::SetViewports(int count, Viewport *viewports) {
D3D11_VIEWPORT vp[4];
for (int i = 0; i < count; i++) {
Expand Down
6 changes: 6 additions & 0 deletions ext/native/thin3d/thin3d_d3d9.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,8 @@ class D3D9Context : public DrawContext {
curPipeline_ = (D3D9Pipeline *)pipeline;
}

void EndFrame() override;

void UpdateDynamicUniformBuffer(const void *ub, size_t size) override;

// Raster state
Expand Down Expand Up @@ -802,6 +804,10 @@ void D3D9Context::BindTextures(int start, int count, Texture **textures) {
}
}

void D3D9Context::EndFrame() {
curPipeline_ = nullptr;
}

static void SemanticToD3D9UsageAndIndex(int semantic, BYTE *usage, BYTE *index) {
*index = 0;
switch (semantic) {
Expand Down
9 changes: 9 additions & 0 deletions ext/native/thin3d/thin3d_gl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,15 @@ void OpenGLContext::EndFrame() {
FrameData &frameData = frameData_[renderManager_.GetCurFrame()];
renderManager_.EndPushBuffer(frameData.push); // upload the data!
renderManager_.Finish();

// Unbind stuff.
for (auto &texture : boundTextures_) {
texture = nullptr;
}
for (auto &sampler : boundSamplers_) {
sampler = nullptr;
}
curPipeline_ = nullptr;
}

InputLayout *OpenGLContext::CreateInputLayout(const InputLayoutDesc &desc) {
Expand Down
24 changes: 24 additions & 0 deletions ext/native/thin3d/thin3d_vulkan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -406,8 +406,13 @@ class VKContext : public DrawContext {

void BindSamplerStates(int start, int count, SamplerState **state) override;
void BindTextures(int start, int count, Texture **textures) override;

void BindPipeline(Pipeline *pipeline) override {
curPipeline_ = (VKPipeline *)pipeline;

if (!pipeline) {
UnbindBoundState();
}
}

// TODO: Make VKBuffers proper buffers, and do a proper binding model. This is just silly.
Expand Down Expand Up @@ -494,6 +499,8 @@ class VKContext : public DrawContext {
}

private:
void UnbindBoundState();

VulkanTexture *GetNullTexture();
VulkanContext *vulkan_ = nullptr;

Expand Down Expand Up @@ -913,6 +920,23 @@ void VKContext::EndFrame() {
renderManager_.Finish();

push_ = nullptr;

// Unbind stuff, to avoid accidentally relying on it across frames (and provide some protection against forgotten unbinds of deleted things).
UnbindBoundState();
}

void VKContext::UnbindBoundState() {
curPipeline_ = nullptr;

for (auto &view : boundImageView_) {
view = nullptr;
}
for (auto &sampler : boundSamplers_) {
sampler = nullptr;
}
for (auto &texture : boundTextures_) {
texture = nullptr;
}
}

void VKContext::WipeQueue() {
Expand Down

0 comments on commit 131a1ee

Please sign in to comment.