Skip to content

Commit

Permalink
Minor cleanup in GL backend, fixes hrydgard#13647
Browse files Browse the repository at this point in the history
Was a stray old texture in boundTextures_ in thin3d. Now makes sure to
invalidate them, and also make it possible to look up bound framebuffer
textures when checking for valid tex parameters.
  • Loading branch information
hrydgard committed Nov 9, 2020
1 parent 9f33a82 commit 5eea743
Show file tree
Hide file tree
Showing 9 changed files with 68 additions and 41 deletions.
5 changes: 0 additions & 5 deletions Common/GPU/OpenGL/GLQueueRunner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -438,11 +438,6 @@ void GLQueueRunner::InitCreateFramebuffer(const GLRInitStep &step) {
tex.wrapT = GL_CLAMP_TO_EDGE;
tex.magFilter = linear ? GL_LINEAR : GL_NEAREST;
tex.minFilter = linear ? GL_LINEAR : GL_NEAREST;
if (gl_extensions.OES_texture_npot) {
tex.canWrap = true;
} else {
tex.canWrap = isPowerOf2(fbo->width) && isPowerOf2(fbo->height);
}

glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, tex.wrapS);
glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, tex.wrapT);
Expand Down
18 changes: 18 additions & 0 deletions Common/GPU/OpenGL/GLRenderManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include "Common/Log.h"
#include "Common/MemoryUtil.h"
#include "Common/Math/math_util.h"

#if 0 // def _DEBUG
#define VLOG(...) INFO_LOG(G3D, __VA_ARGS__)
Expand All @@ -19,6 +20,23 @@ static bool OnRenderThread() {
}
#endif

GLRTexture::GLRTexture(int width, int height, int numMips) {
if (gl_extensions.OES_texture_npot) {
canWrap = true;
} else {
canWrap = isPowerOf2(width) && isPowerOf2(height);
}
w = width;
h = height;
this->numMips = numMips;
}

GLRTexture::~GLRTexture() {
if (texture) {
glDeleteTextures(1, &texture);
}
}

void GLDeleter::Take(GLDeleter &other) {
_assert_msg_(IsEmpty(), "Deleter already has stuff");
shaders = std::move(other.shaders);
Expand Down
19 changes: 11 additions & 8 deletions Common/GPU/OpenGL/GLRenderManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,20 @@ class DrawContext;

class GLRTexture {
public:
~GLRTexture() {
if (texture) {
glDeleteTextures(1, &texture);
}
}
GLRTexture(int width, int height, int numMips);
~GLRTexture();

GLuint texture = 0;
uint16_t w;
uint16_t h;

// We don't trust OpenGL defaults - setting wildly off values ensures that we'll end up overwriting these parameters.
GLenum target = 0xFFFF;
GLenum wrapS = 0xFFFF;
GLenum wrapT = 0xFFFF;
GLenum magFilter = 0xFFFF;
GLenum minFilter = 0xFFFF;
uint8_t numMips = 0;
bool canWrap = true;
float anisotropy = -100000.0f;
float minLod = -1000.0f;
Expand All @@ -47,7 +47,8 @@ class GLRTexture {
class GLRFramebuffer {
public:
GLRFramebuffer(int _width, int _height, bool z_stencil)
: width(_width), height(_height), z_stencil_(z_stencil) {
: width(_width), height(_height), z_stencil_(z_stencil),
color_texture(_width, _height, 1), z_stencil_texture(_width, _height, 1) {
}

~GLRFramebuffer();
Expand Down Expand Up @@ -378,9 +379,11 @@ class GLRenderManager {
void WaitUntilQueueIdle();

// Creation commands. These were not needed in Vulkan since there we can do that on the main thread.
GLRTexture *CreateTexture(GLenum target) {
// We pass in width/height here even though it's not strictly needed until we support glTextureStorage
// and then we'll also need formats and stuff.
GLRTexture *CreateTexture(GLenum target, int width, int height, int numMips) {
GLRInitStep step{ GLRInitStepType::CREATE_TEXTURE };
step.create_texture.texture = new GLRTexture();
step.create_texture.texture = new GLRTexture(width, height, numMips);
step.create_texture.texture->target = target;
initSteps_.push_back(step);
return step.create_texture.texture;
Expand Down
41 changes: 24 additions & 17 deletions Common/GPU/OpenGL/thin3d_gl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ class OpenGLContext : public DrawContext {
void GetFramebufferDimensions(Framebuffer *fbo, int *w, int *h) override;

void BindSamplerStates(int start, int count, SamplerState **states) override {
if (start + count >= MAX_TEXTURE_SLOTS) {
if (start + count > MAX_TEXTURE_SLOTS) {
return;
}
for (int i = 0; i < count; i++) {
Expand Down Expand Up @@ -497,7 +497,9 @@ class OpenGLContext : public DrawContext {

// Bound state
OpenGLSamplerState *boundSamplers_[MAX_TEXTURE_SLOTS]{};
OpenGLTexture *boundTextures_[MAX_TEXTURE_SLOTS]{};
// Point to GLRTexture directly because they can point to the textures
// in framebuffers too (which also can be bound).
const GLRTexture *boundTextures_[MAX_TEXTURE_SLOTS]{};

OpenGLPipeline *curPipeline_ = nullptr;
OpenGLBuffer *curVBuffers_[4]{};
Expand Down Expand Up @@ -739,16 +741,17 @@ class OpenGLTexture : public Texture {
bool HasMips() const {
return mipLevels_ > 1 || generatedMips_;
}
bool CanWrap() const {
return canWrap_;
}

TextureType GetType() const { return type_; }
void Bind(int stage) {
render_->BindTexture(stage, tex_);
}
int NumMipmaps() const {
return mipLevels_;
}
const GLRTexture *GetTex() const {
return tex_;
}

private:
void SetImageData(int x, int y, int z, int width, int height, int depth, int level, int stride, const uint8_t *data, TextureCallback callback);
Expand All @@ -760,7 +763,6 @@ class OpenGLTexture : public Texture {
TextureType type_;
int mipLevels_;
bool generatedMips_;
bool canWrap_;
};

OpenGLTexture::OpenGLTexture(GLRenderManager *render, const TextureDesc &desc) : render_(render) {
Expand All @@ -771,9 +773,8 @@ OpenGLTexture::OpenGLTexture(GLRenderManager *render, const TextureDesc &desc) :
format_ = desc.format;
type_ = desc.type;
GLenum target = TypeToTarget(desc.type);
tex_ = render->CreateTexture(target);
tex_ = render->CreateTexture(target, desc.width, desc.height, desc.mipLevels);

canWrap_ = isPowerOf2(width_) && isPowerOf2(height_);
mipLevels_ = desc.mipLevels;
if (desc.initData.empty())
return;
Expand Down Expand Up @@ -1091,7 +1092,7 @@ Pipeline *OpenGLContext::CreateGraphicsPipeline(const PipelineDesc &desc) {
}

void OpenGLContext::BindTextures(int start, int count, Texture **textures) {
if (start + count >= MAX_TEXTURE_SLOTS) {
if (start + count > MAX_TEXTURE_SLOTS) {
return;
}
for (int i = start; i < start + count; i++) {
Expand All @@ -1102,32 +1103,32 @@ void OpenGLContext::BindTextures(int start, int count, Texture **textures) {
continue;
}
glTex->Bind(i);
boundTextures_[i] = glTex;
boundTextures_[i] = glTex->GetTex();
}
}

void OpenGLContext::ApplySamplers() {
for (int i = 0; i < MAX_TEXTURE_SLOTS; i++) {
const OpenGLSamplerState *samp = boundSamplers_[i];
const OpenGLTexture *tex = boundTextures_[i];
const GLRTexture *tex = boundTextures_[i];
if (tex) {
_assert_(samp);
} else {
continue;
}
GLenum wrapS;
GLenum wrapT;
if (tex->CanWrap()) {
if (tex->canWrap) {
wrapS = samp->wrapU;
wrapT = samp->wrapV;
} else {
wrapS = GL_CLAMP_TO_EDGE;
wrapT = GL_CLAMP_TO_EDGE;
}
GLenum magFilt = samp->magFilt;
GLenum minFilt = tex->HasMips() ? samp->mipMinFilt : samp->minFilt;
GLenum minFilt = tex->numMips > 1 ? samp->mipMinFilt : samp->minFilt;
renderManager_.SetTextureSampler(i, wrapS, wrapT, magFilt, minFilt, 0.0f);
renderManager_.SetTextureLod(i, 0.0, (float)(tex->NumMipmaps() - 1), 0.0);
renderManager_.SetTextureLod(i, 0.0, (float)(tex->numMips - 1), 0.0);
}
}

Expand Down Expand Up @@ -1376,12 +1377,18 @@ void OpenGLContext::BindFramebufferAsTexture(Framebuffer *fbo, int binding, FBCh
OpenGLFramebuffer *fb = (OpenGLFramebuffer *)fbo;

GLuint aspect = 0;
if (channelBit & FB_COLOR_BIT)
if (channelBit & FB_COLOR_BIT) {
aspect |= GL_COLOR_BUFFER_BIT;
if (channelBit & FB_DEPTH_BIT)
boundTextures_[binding] = &fb->framebuffer_->color_texture;
}
if (channelBit & FB_DEPTH_BIT) {
aspect |= GL_DEPTH_BUFFER_BIT;
if (channelBit & FB_STENCIL_BIT)
boundTextures_[binding] = &fb->framebuffer_->z_stencil_texture;
}
if (channelBit & FB_STENCIL_BIT) {
aspect |= GL_STENCIL_BUFFER_BIT;
boundTextures_[binding] = &fb->framebuffer_->z_stencil_texture;
}
renderManager_.BindFramebufferAsTexture(fb->framebuffer_, binding, aspect, color);
}

Expand Down
1 change: 1 addition & 0 deletions GPU/Common/FramebufferManagerCommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -605,6 +605,7 @@ void FramebufferManagerCommon::ReinterpretFramebufferFrom(VirtualFramebuffer *vf
// Copy to a temp framebuffer.
Draw::Framebuffer *temp = GetTempFBO(TempFBO::REINTERPRET, vfb->renderWidth, vfb->renderHeight);

draw_->InvalidateCachedState();
draw_->CopyFramebufferImage(vfb->fbo, 0, 0, 0, 0, temp, 0, 0, 0, 0, vfb->renderWidth, vfb->renderHeight, 1, Draw::FBChannel::FB_COLOR_BIT, "reinterpret_prep");
draw_->BindFramebufferAsRenderTarget(vfb->fbo, { Draw::RPAction::DONT_CARE, Draw::RPAction::DONT_CARE, Draw::RPAction::DONT_CARE }, "reinterpret");
draw_->BindPipeline(pipeline);
Expand Down
2 changes: 1 addition & 1 deletion GPU/GLES/DepalettizeShaderGLES.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ GLRTexture *DepalShaderCacheGLES::GetClutTexture(GEPaletteFormat clutFormat, con
int texturePixels = clutFormat == GE_CMODE_32BIT_ABGR8888 ? 256 : 512;

DepalTexture *tex = new DepalTexture();
tex->texture = render_->CreateTexture(GL_TEXTURE_2D);
tex->texture = render_->CreateTexture(GL_TEXTURE_2D, texturePixels, 1, 1);

uint8_t *clutCopy = new uint8_t[1024];
memcpy(clutCopy, rawClut, 1024);
Expand Down
6 changes: 3 additions & 3 deletions GPU/GLES/DrawEngineGLES.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -696,7 +696,7 @@ void TessellationDataTransferGLES::SendDataToShader(const SimpleVertex *const *p
prevSizeU = size_u;
prevSizeV = size_v;
if (!data_tex[0])
data_tex[0] = renderManager_->CreateTexture(GL_TEXTURE_2D);
data_tex[0] = renderManager_->CreateTexture(GL_TEXTURE_2D, size_u * 3, size_v, 1);
renderManager_->TextureImage(data_tex[0], 0, size_u * 3, size_v, Draw::DataFormat::R32G32B32A32_FLOAT, nullptr, GLRAllocType::NONE, false);
renderManager_->FinalizeTexture(data_tex[0], 0, false);
}
Expand All @@ -714,7 +714,7 @@ void TessellationDataTransferGLES::SendDataToShader(const SimpleVertex *const *p
if (prevSizeWU < weights.size_u) {
prevSizeWU = weights.size_u;
if (!data_tex[1])
data_tex[1] = renderManager_->CreateTexture(GL_TEXTURE_2D);
data_tex[1] = renderManager_->CreateTexture(GL_TEXTURE_2D, weights.size_u * 2, 1, 1);
renderManager_->TextureImage(data_tex[1], 0, weights.size_u * 2, 1, Draw::DataFormat::R32G32B32A32_FLOAT, nullptr, GLRAllocType::NONE, false);
renderManager_->FinalizeTexture(data_tex[1], 0, false);
}
Expand All @@ -725,7 +725,7 @@ void TessellationDataTransferGLES::SendDataToShader(const SimpleVertex *const *p
if (prevSizeWV < weights.size_v) {
prevSizeWV = weights.size_v;
if (!data_tex[2])
data_tex[2] = renderManager_->CreateTexture(GL_TEXTURE_2D);
data_tex[2] = renderManager_->CreateTexture(GL_TEXTURE_2D, weights.size_v * 2, 1, 1);
renderManager_->TextureImage(data_tex[2], 0, weights.size_v * 2, 1, Draw::DataFormat::R32G32B32A32_FLOAT, nullptr, GLRAllocType::NONE, false);
renderManager_->FinalizeTexture(data_tex[2], 0, false);
}
Expand Down
2 changes: 1 addition & 1 deletion GPU/GLES/FragmentTestCacheGLES.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ GLRTexture *FragmentTestCacheGLES::CreateTestTexture(const GEComparison funcs[4]
}
}

GLRTexture *tex = render_->CreateTexture(GL_TEXTURE_2D);
GLRTexture *tex = render_->CreateTexture(GL_TEXTURE_2D, 256, 1, 1);
render_->TextureImage(tex, 0, 256, 1, Draw::DataFormat::R8G8B8A8_UNORM, data);
return tex;
}
Expand Down
15 changes: 9 additions & 6 deletions GPU/GLES/TextureCacheGLES.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -445,11 +445,6 @@ void TextureCacheGLES::BuildTexture(TexCacheEntry *const entry) {
// For the estimate, we assume cluts always point to 8888 for simplicity.
cacheSizeEstimate_ += EstimateTexMemoryUsage(entry);

// Always generate a texture name unless it's a framebuffer, we might need it if the texture is replaced later.
if (!entry->textureName) {
entry->textureName = render_->CreateTexture(GL_TEXTURE_2D);
}

if ((entry->bufw == 0 || (gstate.texbufwidth[0] & 0xf800) != 0) && entry->addr >= PSP_GetKernelMemoryEnd()) {
ERROR_LOG_REPORT(G3D, "Texture with unexpected bufw (full=%d)", gstate.texbufwidth[0] & 0xffff);
// Proceeding here can cause a crash.
Expand Down Expand Up @@ -535,7 +530,7 @@ void TextureCacheGLES::BuildTexture(TexCacheEntry *const entry) {
texelsScaledThisFrame_ += w * h;
}
}

// GLES2 doesn't have support for a "Max lod" which is critical as PSP games often
// don't specify mips all the way down. As a result, we either need to manually generate
// the bottom few levels or rely on OpenGL's autogen mipmaps instead, which might not
Expand Down Expand Up @@ -645,6 +640,14 @@ void TextureCacheGLES::LoadTextureLevel(TexCacheEntry &entry, ReplacedTexture &r

gpuStats.numTexturesDecoded++;

if (!entry.textureName) {
// TODO: Actually pass in correct size here. The size here is not yet used for anything else
// than determining if we can wrap this texture size, that is, it's pow2 or not on very old hardware, else true.
// This will be easy after .. well, yet another refactoring, where I hoist the size calculation out of LoadTextureLevel
// and unify BuildTexture.
entry.textureName = render_->CreateTexture(GL_TEXTURE_2D, 16, 16, 1);
}

if (replaced.GetSize(level, w, h)) {
PROFILE_THIS_SCOPE("replacetex");

Expand Down

0 comments on commit 5eea743

Please sign in to comment.