From cd817be92216c631f7e44e1e64e29fe8a2d4cec4 Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Sun, 30 Aug 2015 08:47:50 -0300 Subject: [PATCH 1/4] citra-qt: Move system shutdown to run inside EmuThread This stops (for some reason sporadic) crashes and OpenGL errors during shutdown, when the OpenGL renderer tries to clean up objects from the UI thread, which has no OpenGL context active. --- src/citra_qt/bootmanager.cpp | 3 +++ src/citra_qt/main.cpp | 3 --- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/citra_qt/bootmanager.cpp b/src/citra_qt/bootmanager.cpp index d9bacfc3d..9aec16506 100644 --- a/src/citra_qt/bootmanager.cpp +++ b/src/citra_qt/bootmanager.cpp @@ -72,6 +72,9 @@ void EmuThread::run() { } } + // Shutdown the core emulation + System::Shutdown(); + MicroProfileOnThreadExit(); render_window->moveContext(); diff --git a/src/citra_qt/main.cpp b/src/citra_qt/main.cpp index 7fb1b0dcb..11813a2a8 100644 --- a/src/citra_qt/main.cpp +++ b/src/citra_qt/main.cpp @@ -283,9 +283,6 @@ void GMainWindow::ShutdownGame() { emu_thread->wait(); emu_thread = nullptr; - // Shutdown the core emulation - System::Shutdown(); - // Update the GUI ui.action_Start->setEnabled(false); ui.action_Start->setText(tr("Start")); From ec28f037e6cfe42c3285866572af075e1e72b3e9 Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Sun, 30 Aug 2015 08:41:28 -0300 Subject: [PATCH 2/4] OpenGL: Add support for Sampler Objects to state tracker --- .../renderer_opengl/gl_resource_manager.h | 24 +++++++++++++++++++ src/video_core/renderer_opengl/gl_state.cpp | 20 ++++++++++++---- src/video_core/renderer_opengl/gl_state.h | 2 ++ 3 files changed, 42 insertions(+), 4 deletions(-) diff --git a/src/video_core/renderer_opengl/gl_resource_manager.h b/src/video_core/renderer_opengl/gl_resource_manager.h index 68fc1e6d3..65034d40d 100644 --- a/src/video_core/renderer_opengl/gl_resource_manager.h +++ b/src/video_core/renderer_opengl/gl_resource_manager.h @@ -37,6 +37,30 @@ public: GLuint handle = 0; }; +class OGLSampler : private NonCopyable { +public: + OGLSampler() = default; + OGLSampler(OGLSampler&& o) { std::swap(handle, o.handle); } + ~OGLSampler() { Release(); } + OGLSampler& operator=(OGLSampler&& o) { std::swap(handle, o.handle); return *this; } + + /// Creates a new internal OpenGL resource and stores the handle + void Create() { + if (handle != 0) return; + glGenSamplers(1, &handle); + } + + /// Deletes the internal OpenGL resource + void Release() { + if (handle == 0) return; + glDeleteSamplers(1, &handle); + OpenGLState::ResetSampler(handle); + handle = 0; + } + + GLuint handle = 0; +}; + class OGLShader : private NonCopyable { public: OGLShader() = default; diff --git a/src/video_core/renderer_opengl/gl_state.cpp b/src/video_core/renderer_opengl/gl_state.cpp index ba47ce8b8..e02c27fbf 100644 --- a/src/video_core/renderer_opengl/gl_state.cpp +++ b/src/video_core/renderer_opengl/gl_state.cpp @@ -44,6 +44,7 @@ OpenGLState::OpenGLState() { for (auto& texture_unit : texture_units) { texture_unit.texture_2d = 0; + texture_unit.sampler = 0; } draw.framebuffer = 0; @@ -154,10 +155,13 @@ void OpenGLState::Apply() { } // Textures - for (unsigned texture_index = 0; texture_index < ARRAY_SIZE(texture_units); ++texture_index) { - if (texture_units[texture_index].texture_2d != cur_state.texture_units[texture_index].texture_2d) { - glActiveTexture(GL_TEXTURE0 + texture_index); - glBindTexture(GL_TEXTURE_2D, texture_units[texture_index].texture_2d); + for (unsigned i = 0; i < ARRAY_SIZE(texture_units); ++i) { + if (texture_units[i].texture_2d != cur_state.texture_units[i].texture_2d) { + glActiveTexture(GL_TEXTURE0 + i); + glBindTexture(GL_TEXTURE_2D, texture_units[i].texture_2d); + } + if (texture_units[i].sampler != cur_state.texture_units[i].sampler) { + glBindSampler(i, texture_units[i].sampler); } } @@ -192,6 +196,14 @@ void OpenGLState::ResetTexture(GLuint id) { } } +void OpenGLState::ResetSampler(GLuint id) { + for (auto& unit : cur_state.texture_units) { + if (unit.sampler == id) { + unit.sampler = 0; + } + } +} + void OpenGLState::ResetProgram(GLuint id) { if (cur_state.draw.shader_program == id) { cur_state.draw.shader_program = 0; diff --git a/src/video_core/renderer_opengl/gl_state.h b/src/video_core/renderer_opengl/gl_state.h index 43aa29a81..6ecbedbb4 100644 --- a/src/video_core/renderer_opengl/gl_state.h +++ b/src/video_core/renderer_opengl/gl_state.h @@ -57,6 +57,7 @@ public: // 3 texture units - one for each that is used in PICA fragment shader emulation struct { GLuint texture_2d; // GL_TEXTURE_BINDING_2D + GLuint sampler; // GL_SAMPLER_BINDING } texture_units[3]; struct { @@ -77,6 +78,7 @@ public: void Apply(); static void ResetTexture(GLuint id); + static void ResetSampler(GLuint id); static void ResetProgram(GLuint id); static void ResetBuffer(GLuint id); static void ResetVertexArray(GLuint id); From 466e608c19c5bd70392bd4f4049fc7ba9963a14c Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Sun, 30 Aug 2015 09:15:35 -0300 Subject: [PATCH 3/4] OpenGL: Remove ugly and endian-unsafe color pointer casts --- src/video_core/pica.h | 4 ++++ src/video_core/renderer_opengl/gl_rasterizer.cpp | 6 +++--- src/video_core/renderer_opengl/gl_rasterizer_cache.cpp | 2 +- src/video_core/renderer_opengl/pica_to_gl.h | 10 +++++----- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/video_core/pica.h b/src/video_core/pica.h index 855cb442e..c1dca5087 100644 --- a/src/video_core/pica.h +++ b/src/video_core/pica.h @@ -135,6 +135,7 @@ struct Regs { }; union { + u32 raw; BitField< 0, 8, u32> r; BitField< 8, 8, u32> g; BitField<16, 8, u32> b; @@ -339,6 +340,7 @@ struct Regs { }; union { + u32 const_color; BitField< 0, 8, u32> const_r; BitField< 8, 8, u32> const_g; BitField<16, 8, u32> const_b; @@ -389,6 +391,7 @@ struct Regs { TevStageConfig tev_stage5; union { + u32 raw; BitField< 0, 8, u32> r; BitField< 8, 8, u32> g; BitField<16, 8, u32> b; @@ -473,6 +476,7 @@ struct Regs { }; union { + u32 raw; BitField< 0, 8, u32> r; BitField< 8, 8, u32> g; BitField<16, 8, u32> b; diff --git a/src/video_core/renderer_opengl/gl_rasterizer.cpp b/src/video_core/renderer_opengl/gl_rasterizer.cpp index 80e773728..b556ea65b 100644 --- a/src/video_core/renderer_opengl/gl_rasterizer.cpp +++ b/src/video_core/renderer_opengl/gl_rasterizer.cpp @@ -658,7 +658,7 @@ void RasterizerOpenGL::SyncBlendFuncs() { } void RasterizerOpenGL::SyncBlendColor() { - auto blend_color = PicaToGL::ColorRGBA8((u8*)&Pica::g_state.regs.output_merger.blend_const.r); + auto blend_color = PicaToGL::ColorRGBA8(Pica::g_state.regs.output_merger.blend_const.raw); state.blend.color.red = blend_color[0]; state.blend.color.green = blend_color[1]; state.blend.color.blue = blend_color[2]; @@ -728,7 +728,7 @@ void RasterizerOpenGL::SyncTevOps(unsigned stage_index, const Pica::Regs::TevSta } void RasterizerOpenGL::SyncTevColor(unsigned stage_index, const Pica::Regs::TevStageConfig& config) { - auto const_color = PicaToGL::ColorRGBA8((u8*)&config.const_r); + auto const_color = PicaToGL::ColorRGBA8(config.const_color); glUniform4fv(uniform_tev_cfgs[stage_index].const_color, 1, const_color.data()); } @@ -737,7 +737,7 @@ void RasterizerOpenGL::SyncTevMultipliers(unsigned stage_index, const Pica::Regs } void RasterizerOpenGL::SyncCombinerColor() { - auto combiner_color = PicaToGL::ColorRGBA8((u8*)&Pica::g_state.regs.tev_combiner_buffer_color.r); + auto combiner_color = PicaToGL::ColorRGBA8(Pica::g_state.regs.tev_combiner_buffer_color.raw); glUniform4fv(uniform_tev_combiner_buffer_color, 1, combiner_color.data()); } diff --git a/src/video_core/renderer_opengl/gl_rasterizer_cache.cpp b/src/video_core/renderer_opengl/gl_rasterizer_cache.cpp index 1e38c2e6d..5d9a80cd4 100644 --- a/src/video_core/renderer_opengl/gl_rasterizer_cache.cpp +++ b/src/video_core/renderer_opengl/gl_rasterizer_cache.cpp @@ -46,7 +46,7 @@ void RasterizerCacheOpenGL::LoadAndBindTexture(OpenGLState &state, unsigned text glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, wrap_t); if (wrap_s == GL_CLAMP_TO_BORDER || wrap_t == GL_CLAMP_TO_BORDER) { - auto border_color = PicaToGL::ColorRGBA8((u8*)&config.config.border_color.r); + auto border_color = PicaToGL::ColorRGBA8(config.config.border_color.raw); glTexParameterfv(GL_TEXTURE_2D, GL_TEXTURE_BORDER_COLOR, border_color.data()); } diff --git a/src/video_core/renderer_opengl/pica_to_gl.h b/src/video_core/renderer_opengl/pica_to_gl.h index 6344f467f..04c1d1a34 100644 --- a/src/video_core/renderer_opengl/pica_to_gl.h +++ b/src/video_core/renderer_opengl/pica_to_gl.h @@ -175,11 +175,11 @@ inline GLenum StencilOp(Pica::Regs::StencilAction action) { return stencil_op_table[(unsigned)action]; } -inline std::array ColorRGBA8(const u8* bytes) { - return { { bytes[0] / 255.0f, - bytes[1] / 255.0f, - bytes[2] / 255.0f, - bytes[3] / 255.0f +inline std::array ColorRGBA8(const u32 color) { + return { { (color >> 0 & 0xFF) / 255.0f, + (color >> 8 & 0xFF) / 255.0f, + (color >> 16 & 0xFF) / 255.0f, + (color >> 24 & 0xFF) / 255.0f } }; } From b044c047c48469be479ba2633ae14eff8643041e Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Sun, 30 Aug 2015 10:05:56 -0300 Subject: [PATCH 4/4] OpenGL: Use Sampler Objects to decouple sampler config from textures Fixes #978 --- .../renderer_opengl/gl_rasterizer.cpp | 46 +++++++++++++++++++ .../renderer_opengl/gl_rasterizer.h | 19 ++++++++ .../renderer_opengl/gl_rasterizer_cache.cpp | 25 ++-------- .../renderer_opengl/gl_rasterizer_cache.h | 7 ++- 4 files changed, 76 insertions(+), 21 deletions(-) diff --git a/src/video_core/renderer_opengl/gl_rasterizer.cpp b/src/video_core/renderer_opengl/gl_rasterizer.cpp index b556ea65b..0260a28ce 100644 --- a/src/video_core/renderer_opengl/gl_rasterizer.cpp +++ b/src/video_core/renderer_opengl/gl_rasterizer.cpp @@ -68,6 +68,12 @@ void RasterizerOpenGL::InitObjects() { uniform_tev_cfg.updates_combiner_buffer_color_alpha = glGetUniformLocation(shader.handle, (tev_ref_str + ".updates_combiner_buffer_color_alpha").c_str()); } + // Create sampler objects + for (int i = 0; i < texture_samplers.size(); ++i) { + texture_samplers[i].Create(); + state.texture_units[i].sampler = texture_samplers[i].sampler.handle; + } + // Generate VBO and VAO vertex_buffer.Create(); vertex_array.Create(); @@ -445,6 +451,45 @@ void RasterizerOpenGL::NotifyFlush(PAddr addr, u32 size) { res_cache.NotifyFlush(addr, size); } +void RasterizerOpenGL::SamplerInfo::Create() { + sampler.Create(); + mag_filter = min_filter = TextureConfig::Linear; + wrap_s = wrap_t = TextureConfig::Repeat; + border_color = 0; + + glSamplerParameteri(sampler.handle, GL_TEXTURE_MIN_FILTER, GL_LINEAR); // default is GL_LINEAR_MIPMAP_LINEAR + // Other attributes have correct defaults +} + +void RasterizerOpenGL::SamplerInfo::SyncWithConfig(const Pica::Regs::TextureConfig& config) { + GLuint s = sampler.handle; + + if (mag_filter != config.mag_filter) { + mag_filter = config.mag_filter; + glSamplerParameteri(s, GL_TEXTURE_MAG_FILTER, PicaToGL::TextureFilterMode(mag_filter)); + } + if (min_filter != config.min_filter) { + min_filter = config.min_filter; + glSamplerParameteri(s, GL_TEXTURE_MIN_FILTER, PicaToGL::TextureFilterMode(min_filter)); + } + + if (wrap_s != config.wrap_s) { + wrap_s = config.wrap_s; + glSamplerParameteri(s, GL_TEXTURE_WRAP_S, PicaToGL::WrapMode(wrap_s)); + } + if (wrap_t != config.wrap_t) { + wrap_t = config.wrap_t; + glSamplerParameteri(s, GL_TEXTURE_WRAP_T, PicaToGL::WrapMode(wrap_t)); + } + + if (wrap_s == TextureConfig::ClampToBorder || wrap_t == TextureConfig::ClampToBorder) { + if (border_color != config.border_color.raw) { + auto gl_color = PicaToGL::ColorRGBA8(border_color); + glSamplerParameterfv(s, GL_TEXTURE_BORDER_COLOR, gl_color.data()); + } + } +} + void RasterizerOpenGL::ReconfigureColorTexture(TextureInfo& texture, Pica::Regs::ColorFormat format, u32 width, u32 height) { GLint internal_format; @@ -772,6 +817,7 @@ void RasterizerOpenGL::SyncDrawState() { const auto& texture = pica_textures[texture_index]; if (texture.enabled) { + texture_samplers[texture_index].SyncWithConfig(texture.config); res_cache.LoadAndBindTexture(state, texture_index, texture); } else { state.texture_units[texture_index].texture_2d = 0; diff --git a/src/video_core/renderer_opengl/gl_rasterizer.h b/src/video_core/renderer_opengl/gl_rasterizer.h index a02d5c856..24560d7f8 100644 --- a/src/video_core/renderer_opengl/gl_rasterizer.h +++ b/src/video_core/renderer_opengl/gl_rasterizer.h @@ -80,6 +80,24 @@ private: GLenum gl_type; }; + struct SamplerInfo { + using TextureConfig = Pica::Regs::TextureConfig; + + OGLSampler sampler; + + /// Creates the sampler object, initializing its state so that it's in sync with the SamplerInfo struct. + void Create(); + /// Syncs the sampler object with the config, updating any necessary state. + void SyncWithConfig(const TextureConfig& config); + + private: + TextureConfig::TextureFilter mag_filter; + TextureConfig::TextureFilter min_filter; + TextureConfig::WrapMode wrap_s; + TextureConfig::WrapMode wrap_t; + u32 border_color; + }; + /// Structure that the hardware rendered vertices are composed of struct HardwareVertex { HardwareVertex(const Pica::Shader::OutputVertex& v) { @@ -193,6 +211,7 @@ private: PAddr last_fb_depth_addr; // Hardware rasterizer + std::array texture_samplers; TextureInfo fb_color_texture; DepthTextureInfo fb_depth_texture; OGLShader shader; diff --git a/src/video_core/renderer_opengl/gl_rasterizer_cache.cpp b/src/video_core/renderer_opengl/gl_rasterizer_cache.cpp index 5d9a80cd4..d9ccf2a3f 100644 --- a/src/video_core/renderer_opengl/gl_rasterizer_cache.cpp +++ b/src/video_core/renderer_opengl/gl_rasterizer_cache.cpp @@ -20,9 +20,8 @@ RasterizerCacheOpenGL::~RasterizerCacheOpenGL() { MICROPROFILE_DEFINE(OpenGL_TextureUpload, "OpenGL", "Texture Upload", MP_RGB(128, 64, 192)); -void RasterizerCacheOpenGL::LoadAndBindTexture(OpenGLState &state, unsigned texture_unit, const Pica::Regs::FullTextureConfig& config) { - PAddr texture_addr = config.config.GetPhysicalAddress(); - const auto cached_texture = texture_cache.find(texture_addr); +void RasterizerCacheOpenGL::LoadAndBindTexture(OpenGLState &state, unsigned texture_unit, const Pica::DebugUtils::TextureInfo& info) { + const auto cached_texture = texture_cache.find(info.physical_address); if (cached_texture != texture_cache.end()) { state.texture_units[texture_unit].texture_2d = cached_texture->second->texture.handle; @@ -37,26 +36,12 @@ void RasterizerCacheOpenGL::LoadAndBindTexture(OpenGLState &state, unsigned text state.Apply(); glActiveTexture(GL_TEXTURE0 + texture_unit); - glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, PicaToGL::TextureFilterMode(config.config.mag_filter)); - glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, PicaToGL::TextureFilterMode(config.config.min_filter)); - - GLenum wrap_s = PicaToGL::WrapMode(config.config.wrap_s); - GLenum wrap_t = PicaToGL::WrapMode(config.config.wrap_t); - glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, wrap_s); - glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, wrap_t); - - if (wrap_s == GL_CLAMP_TO_BORDER || wrap_t == GL_CLAMP_TO_BORDER) { - auto border_color = PicaToGL::ColorRGBA8(config.config.border_color.raw); - glTexParameterfv(GL_TEXTURE_2D, GL_TEXTURE_BORDER_COLOR, border_color.data()); - } - - const auto info = Pica::DebugUtils::TextureInfo::FromPicaRegister(config.config, config.format); - u8* texture_src_data = Memory::GetPhysicalPointer(texture_addr); + u8* texture_src_data = Memory::GetPhysicalPointer(info.physical_address); new_texture->width = info.width; new_texture->height = info.height; new_texture->size = info.stride * info.height; - new_texture->addr = texture_addr; + new_texture->addr = info.physical_address; new_texture->hash = Common::ComputeHash64(texture_src_data, new_texture->size); std::unique_ptr[]> temp_texture_buffer_rgba(new Math::Vec4[info.width * info.height]); @@ -69,7 +54,7 @@ void RasterizerCacheOpenGL::LoadAndBindTexture(OpenGLState &state, unsigned text glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, info.width, info.height, 0, GL_RGBA, GL_UNSIGNED_BYTE, temp_texture_buffer_rgba.get()); - texture_cache.emplace(texture_addr, std::move(new_texture)); + texture_cache.emplace(info.physical_address, std::move(new_texture)); } } diff --git a/src/video_core/renderer_opengl/gl_rasterizer_cache.h b/src/video_core/renderer_opengl/gl_rasterizer_cache.h index d8f9edf59..ec56237b5 100644 --- a/src/video_core/renderer_opengl/gl_rasterizer_cache.h +++ b/src/video_core/renderer_opengl/gl_rasterizer_cache.h @@ -6,6 +6,7 @@ #include "gl_state.h" #include "gl_resource_manager.h" +#include "video_core/debug_utils/debug_utils.h" #include "video_core/pica.h" #include @@ -16,7 +17,11 @@ public: ~RasterizerCacheOpenGL(); /// Loads a texture from 3DS memory to OpenGL and caches it (if not already cached) - void LoadAndBindTexture(OpenGLState &state, unsigned texture_unit, const Pica::Regs::FullTextureConfig& config); + void LoadAndBindTexture(OpenGLState &state, unsigned texture_unit, const Pica::DebugUtils::TextureInfo& info); + + void LoadAndBindTexture(OpenGLState &state, unsigned texture_unit, const Pica::Regs::FullTextureConfig& config) { + LoadAndBindTexture(state, texture_unit, Pica::DebugUtils::TextureInfo::FromPicaRegister(config.config, config.format)); + } /// Flush any cached resource that touches the flushed region void NotifyFlush(PAddr addr, u32 size, bool ignore_hash = false);