gsdx ogl: clean various comment and old code

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
+	glDisable(GL_CULL_FACE);
-#if 0
+	glEnable(GL_SCISSOR_TEST);
-	rd.FrontCounterClockwise = false;
+	glDisable(GL_MULTISAMPLE);
-	rd.DepthBias = false;
-	rd.DepthBiasClamp = 0;
-	rd.SlopeScaledDepthBias = 0;
-	rd.DepthClipEnable = false; // ???
-	rd.AntialiasedLineEnable = false;
-#endif
 
 	// ****************************************************************
 	// 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<GSTextureOGL*>(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<GSTextureOGL*>(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<GSTextureOGL*>(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_MIN_LOD, 0);
-	gl_SamplerParameterf(sampler, GL_TEXTURE_MAX_LOD, FLT_MAX);
+	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<GSTextureOGL*>(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
-		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");
 			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();

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);

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<GSVertexHWOGL>
 {
 	private:
 		GSVector2 m_pixelcenter;

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;
+	// OpenGL didn't like dimensions of size 0
-	// m_size.y = h;
-	// FIXME
 	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);

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());
-
-	}
 };

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
+// I don't remember why I set this parameter but it is surely useless
-layout(pixel_center_integer) in vec4 gl_FragCoord;
+//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

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"
+	"// I don't remember why I set this parameter but it is surely useless\n"
-	"layout(pixel_center_integer) in vec4 gl_FragCoord;\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"