From 73d04e33e920e51ea67d9492017497bcb17b11ed Mon Sep 17 00:00:00 2001 From: Gregory Hainaut Date: Fri, 1 May 2015 20:04:23 +0200 Subject: [PATCH] gsdx ogl: clean various comment and old code --- plugins/GSdx/GSDeviceOGL.cpp | 70 ++++++++++--------------------- plugins/GSdx/GSRendererOGL.cpp | 10 +---- plugins/GSdx/GSRendererOGL.h | 4 -- plugins/GSdx/GSTextureOGL.cpp | 48 +++------------------ plugins/GSdx/GSVertexArrayOGL.h | 27 ------------ plugins/GSdx/res/glsl/tfx_fs.glsl | 7 +--- plugins/GSdx/res/glsl_source.h | 7 +--- 7 files changed, 33 insertions(+), 140 deletions(-) diff --git a/plugins/GSdx/GSDeviceOGL.cpp b/plugins/GSdx/GSDeviceOGL.cpp index bd8712df21..8cea230873 100644 --- a/plugins/GSdx/GSDeviceOGL.cpp +++ b/plugins/GSdx/GSDeviceOGL.cpp @@ -34,6 +34,7 @@ static uint32 g_draw_count = 0; static uint32 g_frame_count = 1; +// TODO port those value into PerfMon API #ifdef ENABLE_OGL_DEBUG_MEM_BW uint64 g_texture_upload_byte = 0; uint64 g_real_texture_upload_byte = 0; @@ -151,6 +152,7 @@ GSTexture* GSDeviceOGL::CreateSurface(int type, int w, int h, bool msaa, int for GSTextureOGL* t = NULL; t = new GSTextureOGL(type, w, h, format, m_fbo_read); + // NOTE: I'm not sure RenderTarget always need to be cleared. It could be costly for big upscale. switch(type) { case GSTexture::RenderTarget: @@ -275,23 +277,15 @@ bool GSDeviceOGL::Create(GSWnd* wnd) // **************************************************************** // rasterization configuration // **************************************************************** - glPolygonMode(GL_FRONT_AND_BACK, GL_FILL); - glDisable(GL_CULL_FACE); - glEnable(GL_SCISSOR_TEST); - glDisable(GL_MULTISAMPLE); #ifdef ONLY_LINES glLineWidth(5.0); glPolygonMode(GL_FRONT_AND_BACK, GL_LINE); +#else + glPolygonMode(GL_FRONT_AND_BACK, GL_FILL); #endif - // Hum I don't know for those options but let's hope there are not activated -#if 0 - rd.FrontCounterClockwise = false; - rd.DepthBias = false; - rd.DepthBiasClamp = 0; - rd.SlopeScaledDepthBias = 0; - rd.DepthClipEnable = false; // ??? - rd.AntialiasedLineEnable = false; -#endif + glDisable(GL_CULL_FACE); + glEnable(GL_SCISSOR_TEST); + glDisable(GL_MULTISAMPLE); // **************************************************************** // DATE @@ -345,7 +339,6 @@ bool GSDeviceOGL::Reset(int w, int h) if(!GSDevice::Reset(w, h)) return false; - // TODO // Opengl allocate the backbuffer with the window. The render is done in the backbuffer when // there isn't any FBO. Only a dummy texture is created to easily detect when the rendering is done // in the backbuffer @@ -361,7 +354,6 @@ void GSDeviceOGL::SetVSync(bool enable) void GSDeviceOGL::Flip() { - // FIXME: disable it when code is working #ifdef ENABLE_OGL_DEBUG CheckDebugLog(); #endif @@ -395,11 +387,6 @@ void GSDeviceOGL::BeforeDraw() #ifdef _DEBUG ASSERT(gl_CheckFramebufferStatus(GL_DRAW_FRAMEBUFFER) == GL_FRAMEBUFFER_COMPLETE); #endif -#if 0 - // Ensure VBOs are uploaded - if (GLLoader::found_GL_ARB_buffer_storage) - Barrier(GL_CLIENT_MAPPED_BUFFER_BARRIER_BIT); -#endif } void GSDeviceOGL::AfterDraw() @@ -434,6 +421,7 @@ void GSDeviceOGL::DrawIndexedPrimitive(int offset, int count) void GSDeviceOGL::ClearRenderTarget(GSTexture* t, const GSVector4& c) { + // TODO: check size of scissor before toggling it glDisable(GL_SCISSOR_TEST); if (static_cast(t)->IsBackbuffer()) { OMSetFBO(0); @@ -470,11 +458,10 @@ void GSDeviceOGL::ClearRenderTarget_ui(GSTexture* t, uint32 c) void GSDeviceOGL::ClearDepth(GSTexture* t, float c) { - // Keep SCISSOR_TEST enabled on purpose to reduce the size - // of clean in DATE (impact big upscaling) OMSetFBO(m_fbo); OMAttachDs(static_cast(t)->GetID()); + // TODO: check size of scissor before toggling it glDisable(GL_SCISSOR_TEST); if (GLState::depth_mask) { gl_ClearBufferfv(GL_DEPTH, 0, &c); @@ -488,6 +475,8 @@ void GSDeviceOGL::ClearDepth(GSTexture* t, float c) void GSDeviceOGL::ClearStencil(GSTexture* t, uint8 c) { + // Keep SCISSOR_TEST enabled on purpose to reduce the size + // of clean in DATE (impact big upscaling) OMSetFBO(m_fbo); OMAttachDs(static_cast(t)->GetID()); GLint color = c; @@ -512,7 +501,6 @@ GLuint GSDeviceOGL::CreateSampler(bool bilinear, bool tau, bool tav) gl_SamplerParameteri(sampler, GL_TEXTURE_MAG_FILTER, GL_NEAREST); } - // FIXME ensure U -> S, V -> T and W->R if (tau) gl_SamplerParameteri(sampler, GL_TEXTURE_WRAP_S, GL_REPEAT); else @@ -524,17 +512,9 @@ GLuint GSDeviceOGL::CreateSampler(bool bilinear, bool tau, bool tav) gl_SamplerParameteri(sampler, GL_TEXTURE_WRAP_R, GL_CLAMP_TO_EDGE); - // FIXME which value for GL_TEXTURE_MIN_LOD - gl_SamplerParameterf(sampler, GL_TEXTURE_MAX_LOD, FLT_MAX); + gl_SamplerParameterf(sampler, GL_TEXTURE_MIN_LOD, 0); + gl_SamplerParameterf(sampler, GL_TEXTURE_MAX_LOD, 6); - // FIXME: seems there is 2 possibility in opengl - // DX: sd.ComparisonFunc = D3D11_COMPARISON_NEVER; - // gl_SamplerParameteri(sampler, GL_TEXTURE_COMPARE_MODE, GL_NONE); -#if 0 - // Message:Program undefined behavior warning: Sampler object 5 has depth compare enabled. It is being used with non-depth texture 7, by a program that samples it with a regular sampler. This is undefined behavior. - gl_SamplerParameteri(sampler, GL_TEXTURE_COMPARE_MODE, GL_COMPARE_REF_TO_TEXTURE); - gl_SamplerParameteri(sampler, GL_TEXTURE_COMPARE_FUNC, GL_NEVER); -#endif // FIXME: need ogl extension sd.MaxAnisotropy = 16; return sampler; @@ -558,7 +538,6 @@ void GSDeviceOGL::RecycleDateTexture() if (m_date.t) { //static_cast(m_date.t)->Save(format("/tmp/date_adv_%04ld.csv", g_draw_count)); - // FIXME invalidate data Recycle(m_date.t); m_date.t = NULL; } @@ -660,9 +639,6 @@ GSTexture* GSDeviceOGL::CopyOffscreen(GSTexture* src, const GSVector4& sr, int w } // Copy a sub part of a texture into another -// Several question to answer did texture have same size? -// From a sub-part to the same sub-part -// From a sub-part to a full texture void GSDeviceOGL::CopyRect(GSTexture* st, GSTexture* dt, const GSVector4i& r) { ASSERT(st && dt); @@ -701,7 +677,6 @@ void GSDeviceOGL::StretchRect(GSTexture* st, const GSVector4& sr, GSTexture* dt, void GSDeviceOGL::StretchRect(GSTexture* st, const GSVector4& sr, GSTexture* dt, const GSVector4& dr, GLuint ps, GSBlendStateOGL* bs, bool linear) { - if(!st || !dt) { ASSERT(0); @@ -834,15 +809,12 @@ void GSDeviceOGL::DoFXAA(GSTexture* st, GSTexture* dt) { // Lazy compile if (!m_fxaa.ps) { - std::string fxaa_macro = "#define FXAA_GLSL_130 1\n"; - if (GLLoader::found_GL_ARB_gpu_shader5) { // GL4.0 extension - // Hardcoded in the new shader - //fxaa_macro += "#define FXAA_GATHER4_ALPHA 1\n"; - fxaa_macro += "#extension GL_ARB_gpu_shader5 : enable\n"; - } else { - fprintf(stderr, "FXAA requires the GL_ARB_gpu_shader5 extension. Please either disable FXAA or upgrade your GPU/driver.\n"); + if (!GLLoader::found_GL_ARB_gpu_shader5) { // GL4.0 extension return; } + + std::string fxaa_macro = "#define FXAA_GLSL_130 1\n"; + fxaa_macro += "#extension GL_ARB_gpu_shader5 : enable\n"; m_fxaa.ps = m_shader->Compile("fxaa.fx", "ps_main", GL_FRAGMENT_SHADER, fxaa_fx, fxaa_macro); } @@ -858,6 +830,10 @@ void GSDeviceOGL::DoExternalFX(GSTexture* st, GSTexture* dt) { // Lazy compile if (!m_shaderfx.ps) { + if (!GLLoader::found_GL_ARB_gpu_shader5) { // GL4.0 extension + return; + } + std::ifstream fconfig(theApp.GetConfig("shaderfx_conf", "dummy.ini")); std::stringstream config; if (fconfig.good()) @@ -928,6 +904,7 @@ void GSDeviceOGL::SetupDATE(GSTexture* rt, GSTexture* ds, const GSVertexPT1* ver OMSetDepthStencilState(m_date.dss, 1); OMSetBlendState(m_date.bs, 0); + // normally ok without any RT if GL_ARB_framebuffer_no_attachments is supported (minus driver bug) OMSetRenderTargets(t, ds, &GLState::scissor); // ia @@ -946,9 +923,6 @@ void GSDeviceOGL::SetupDATE(GSTexture* rt, GSTexture* ds, const GSVertexPT1* ver PSSetSamplerState(m_convert.pt); } - // - - // normally ok without it if GL_ARB_framebuffer_no_attachments is supported (minus driver bug) OMSetWriteBuffer(GL_NONE); DrawPrimitive(); OMSetWriteBuffer(); diff --git a/plugins/GSdx/GSRendererOGL.cpp b/plugins/GSdx/GSRendererOGL.cpp index 4a189604af..2a5124ae20 100644 --- a/plugins/GSdx/GSRendererOGL.cpp +++ b/plugins/GSdx/GSRendererOGL.cpp @@ -167,10 +167,6 @@ bool GSRendererOGL::PrimitiveOverlap() GSVector4i inter = vi.rintersect(vj); if (!inter.rempty()) { //fprintf(stderr, "Overlap found between %d and %d (draw of %d vertices)\n", i, j, count); - //vi.print(); - //vj.print(); - //inter.print(); - //exit(0); return true; } } @@ -473,8 +469,6 @@ void GSRendererOGL::DrawPrims(GSTexture* rt, GSTexture* ds, GSTextureCache::Sour if(PRIM->FST) { vs_cb.TextureScale = GSVector4(1.0f / 16) / WH.xyxy(); - //Maybe better? - //vs_cb.TextureScale = GSVector4(1.0f / 16) * GSVector4(tex->m_texture->GetScale()).xyxy() / WH.zwzw(); ps_sel.fst = 1; } @@ -564,7 +558,7 @@ void GSRendererOGL::DrawPrims(GSTexture* rt, GSTexture* ds, GSTextureCache::Sour { dev->DrawIndexedPrimitive(); - if (env.COLCLAMP.CLAMP == 0 && /* hack */ !tex && PRIM->PRIM != GS_POINTLIST) + if (env.COLCLAMP.CLAMP == 0 && !tex && PRIM->PRIM != GS_POINTLIST) { GSDeviceOGL::OMBlendSelector om_bselneg(om_bsel); GSDeviceOGL::PSSelector ps_selneg(ps_sel); @@ -617,7 +611,7 @@ void GSRendererOGL::DrawPrims(GSTexture* rt, GSTexture* ds, GSTextureCache::Sour dev->DrawIndexedPrimitive(); - if (env.COLCLAMP.CLAMP == 0 && /* hack */ !tex && PRIM->PRIM != GS_POINTLIST) + if (env.COLCLAMP.CLAMP == 0 && !tex && PRIM->PRIM != GS_POINTLIST) { GSDeviceOGL::OMBlendSelector om_bselneg(om_bsel); GSDeviceOGL::PSSelector ps_selneg(ps_sel); diff --git a/plugins/GSdx/GSRendererOGL.h b/plugins/GSdx/GSRendererOGL.h index 74680c5ff8..774aa16272 100644 --- a/plugins/GSdx/GSRendererOGL.h +++ b/plugins/GSdx/GSRendererOGL.h @@ -27,11 +27,7 @@ #include "GSTextureCacheOGL.h" #include "GSVertexHW.h" -// FIXME does it need a GSVertexHWOGL ??? Data order can be easily programmed on opengl (the only potential -// issue is the unsupported praga push/pop -// Note it impact GSVertexTrace.cpp => void GSVertexTrace::Update(const GSVertexHWOGL* v, int count, GS_PRIM_CLASS primclass) class GSRendererOGL : public GSRendererHW -//class GSRendererOGL : public GSRendererHW { private: GSVector2 m_pixelcenter; diff --git a/plugins/GSdx/GSTextureOGL.cpp b/plugins/GSdx/GSTextureOGL.cpp index 1478f1f77b..78a3de9db3 100644 --- a/plugins/GSdx/GSTextureOGL.cpp +++ b/plugins/GSdx/GSTextureOGL.cpp @@ -42,7 +42,6 @@ namespace PboPool { uint8* m_gpu_texture; // Option for buffer storage - // Note there is a barrier (but maybe coherent is faster) // XXX: actually does I really need coherent and barrier??? // As far as I understand glTexSubImage2D is a client-server transfer so no need to make // the value visible to the server @@ -168,9 +167,7 @@ namespace PboPool { GSTextureOGL::GSTextureOGL(int type, int w, int h, int format, GLuint fbo_read) : m_pbo_size(0), m_dirty(false) { - // m_size.x = w; - // m_size.y = h; - // FIXME + // OpenGL didn't like dimensions of size 0 m_size.x = max(1,w); m_size.y = max(1,h); m_format = format; @@ -217,32 +214,18 @@ GSTextureOGL::GSTextureOGL(int type, int w, int h, int format, GLuint fbo_read) ASSERT(0); } - // Generate the buffer + // Generate & Allocate the buffer switch (m_type) { case GSTexture::Offscreen: - //FIXME I not sure we need a pixel buffer object. It seems more a texture - // gl_GenBuffers(1, &m_texture_id); - // ASSERT(0); case GSTexture::Texture: case GSTexture::RenderTarget: case GSTexture::DepthStencil: gl_CreateTextures(GL_TEXTURE_2D, 1, &m_texture_id); - break; - case GSTexture::Backbuffer: - break; - default: - break; - } - - // Allocate the buffer - switch (m_type) { - case GSTexture::Offscreen: - case GSTexture::DepthStencil: - case GSTexture::RenderTarget: - case GSTexture::Texture: gl_TextureStorage2D(m_texture_id, 1+GL_TEX_LEVEL_0, m_format, m_size.x, m_size.y); break; - default: break; + case GSTexture::Backbuffer: + default: + break; } } @@ -311,8 +294,6 @@ bool GSTextureOGL::Update(const GSVector4i& r, const void* data, int pitch) glPixelStorei(GL_UNPACK_ROW_LENGTH, pitch >> m_int_shift); } gl_TextureSubImage2D(m_texture_id, GL_TEX_LEVEL_0, r.x, r.y, r.width(), r.height(), m_int_format, m_int_type, (const void*)PboPool::Offset()); - // Normally only affect TexSubImage call. (i.e. only the previous line) - //glPixelStorei(GL_UNPACK_ROW_LENGTH, 0); // FIXME OGL4: investigate, only 1 unpack buffer always bound PboPool::UnbindPbo(); @@ -377,23 +358,6 @@ bool GSTextureOGL::Map(GSMap& m, const GSVector4i* r) m.pitch = m_size.x << m_int_shift; return true; - -#if 0 - if(m_texture && m_desc.Usage == D3D11_USAGE_STAGING) - { - D3D11_MAPPED_SUBRESOURCE map; - - if(SUCCEEDED(m_ctx->Map(m_texture, 0, D3D11_MAP_READ_WRITE, 0, &map))) - { - m.bits = (uint8*)map.pData; - m.pitch = (int)map.RowPitch; - - return true; - } - } - - return false; -#endif } void GSTextureOGL::Unmap() @@ -584,8 +548,6 @@ bool GSTextureOGL::Save(const string& fn, bool dds) char* image = (char*)malloc(buf_size); bool status = true; - // FIXME instead of swapping manually B and R maybe you can request the driver to do it - // for us if (IsBackbuffer()) { //glReadBuffer(GL_BACK); //gl_BindFramebuffer(GL_READ_FRAMEBUFFER, 0); diff --git a/plugins/GSdx/GSVertexArrayOGL.h b/plugins/GSdx/GSVertexArrayOGL.h index 9322e99f95..1a8b3e9b05 100644 --- a/plugins/GSdx/GSVertexArrayOGL.h +++ b/plugins/GSdx/GSVertexArrayOGL.h @@ -237,11 +237,6 @@ class GSBufferOGL { size_t GetStart() { return m_start; } - void debug() - { - fprintf(stderr, "data buffer: start %d, count %d\n", m_start, m_count); - } - }; class GSVertexBufferStateOGL { @@ -325,26 +320,4 @@ public: delete m_ib; } - void debug() - { - string topo; - switch (m_topology) { - case GL_POINTS: - topo = "point"; - break; - case GL_LINES: - topo = "line"; - break; - case GL_TRIANGLES: - topo = "triangle"; - break; - case GL_TRIANGLE_STRIP: - topo = "triangle strip"; - break; - } - m_vb->debug(); - m_ib->debug(); - fprintf(stderr, "primitives of %s\n", topo.c_str()); - - } }; diff --git a/plugins/GSdx/res/glsl/tfx_fs.glsl b/plugins/GSdx/res/glsl/tfx_fs.glsl index 9f3fc1aae6..15bbcaa96e 100644 --- a/plugins/GSdx/res/glsl/tfx_fs.glsl +++ b/plugins/GSdx/res/glsl/tfx_fs.glsl @@ -77,8 +77,8 @@ layout(binding = 3) uniform sampler2D RtSampler; // note 2 already use by the im // FIXME how to declare memory access layout(r32i, binding = 2) coherent uniform iimage2D img_prim_min; layout(early_fragment_tests) in; -// origin_upper_left -layout(pixel_center_integer) in vec4 gl_FragCoord; +// I don't remember why I set this parameter but it is surely useless +//layout(pixel_center_integer) in vec4 gl_FragCoord; #endif #else // use basic stencil @@ -353,7 +353,6 @@ void atst(vec4 c) } #endif -// Note layout stuff might require gl4.3 #ifndef SUBROUTINE_GL40 void colclip(inout vec4 c) { @@ -361,8 +360,6 @@ void colclip(inout vec4 c) c.rgb = 256.0f/255.0f - c.rgb; #endif #if (PS_COLCLIP > 0) - // FIXME !!!! - //c.rgb *= c.rgb < 128./255; bvec3 factor = lessThan(c.rgb, vec3(128.0f/255.0f)); c.rgb *= vec3(factor); #endif diff --git a/plugins/GSdx/res/glsl_source.h b/plugins/GSdx/res/glsl_source.h index 40cece3ba9..b449012d7b 100644 --- a/plugins/GSdx/res/glsl_source.h +++ b/plugins/GSdx/res/glsl_source.h @@ -818,8 +818,8 @@ static const char* tfx_fs_all_glsl = "// FIXME how to declare memory access\n" "layout(r32i, binding = 2) coherent uniform iimage2D img_prim_min;\n" "layout(early_fragment_tests) in;\n" - "// origin_upper_left\n" - "layout(pixel_center_integer) in vec4 gl_FragCoord;\n" + "// I don't remember why I set this parameter but it is surely useless\n" + "//layout(pixel_center_integer) in vec4 gl_FragCoord;\n" "#endif\n" "#else\n" "// use basic stencil\n" @@ -1094,7 +1094,6 @@ static const char* tfx_fs_all_glsl = "}\n" "#endif\n" "\n" - "// Note layout stuff might require gl4.3\n" "#ifndef SUBROUTINE_GL40\n" "void colclip(inout vec4 c)\n" "{\n" @@ -1102,8 +1101,6 @@ static const char* tfx_fs_all_glsl = " c.rgb = 256.0f/255.0f - c.rgb;\n" "#endif\n" "#if (PS_COLCLIP > 0)\n" - " // FIXME !!!!\n" - " //c.rgb *= c.rgb < 128./255;\n" " bvec3 factor = lessThan(c.rgb, vec3(128.0f/255.0f));\n" " c.rgb *= vec3(factor);\n" "#endif\n"