Cleanup ColorSelector::colorButtonClicked()

Sat, 25 Feb 2017 14:30:10 +0200

author
Teemu Piippo <teemu@hecknology.net>
date
Sat, 25 Feb 2017 14:30:10 +0200
changeset 1173
6cd85b28f43b
parent 1172
3defab8cfd93
child 1174
91696a2e022c

Cleanup ColorSelector::colorButtonClicked()

src/dialogs/colorselector.cpp file | annotate | diff | comparison | revisions
src/dialogs/colorselector.h file | annotate | diff | comparison | revisions
src/glShared.h file | annotate | diff | comparison | revisions
src/glcompiler.cpp file | annotate | diff | comparison | revisions
src/glcompiler.h file | annotate | diff | comparison | revisions
src/glrenderer.cpp file | annotate | diff | comparison | revisions
--- a/src/dialogs/colorselector.cpp	Thu Feb 23 23:36:59 2017 +0200
+++ b/src/dialogs/colorselector.cpp	Sat Feb 25 14:30:10 2017 +0200
@@ -66,8 +66,8 @@
 			button->setCheckable (true);
 			button->setText (QString::number (ldcolor.index()));
 			button->setToolTip (format ("%1: %2", ldcolor.index(), ldcolor.name()));
-			m_buttons[ldcolor.index()] = button;
-			m_buttonsReversed[button] = ldcolor.index();
+			m_buttons[ldcolor] = button;
+			m_buttonsReversed[button] = ldcolor;
 			connect (button, SIGNAL (clicked(bool)), this, SLOT (colorButtonClicked()));
 
 			if (ldcolor == selection())
@@ -104,28 +104,25 @@
 
 void ColorSelector::colorButtonClicked()
 {
-	QPushButton* button = qobject_cast<QPushButton*> (sender());
-	auto it = m_buttonsReversed.find (button);
-	LDColor color;
+	QPushButton* button = qobject_cast<QPushButton*>(sender());
+	LDColor color = m_buttonsReversed.value(button, LDColor::nullColor);
 
-	if (Q_UNLIKELY (button == nullptr or it == m_buttonsReversed.end()
-		or not (color = *it).isValid()))
+	if (color.isValid())
 	{
-		print ("colorButtonClicked() called with invalid sender");
-		return;
-	}
+		// Uncheck the button we previously had pressed.
+		if (m_selection.isValid())
+		{
+			QPushButton* button = m_buttons.value(m_selection);
 
-	if (selection().isValid())
-	{
-		auto button = m_buttons.find (selection().index());
+			if (button)
+				button->setChecked(false);
+		}
 
-		if (button != m_buttons.end())
-			(*button)->setChecked (false);
+		// Select the new color.
+		m_selection = color;
+		button->setChecked (true);
+		drawColorInfo();
 	}
-
-	m_selection = color;
-	button->setChecked (true);
-	drawColorInfo();
 }
 
 void ColorSelector::drawColorInfo()
--- a/src/dialogs/colorselector.h	Thu Feb 23 23:36:59 2017 +0200
+++ b/src/dialogs/colorselector.h	Sat Feb 25 14:30:10 2017 +0200
@@ -34,8 +34,8 @@
 
 private:
 	class Ui_ColorSelUi& ui;
-	QMap<int, QPushButton*> m_buttons;
-	QMap<QPushButton*, int> m_buttonsReversed;
+	QMap<LDColor, QPushButton*> m_buttons;
+	QMap<QPushButton*, LDColor> m_buttonsReversed;
 	bool m_firstResize;
 	LDColor m_selection;
 
--- a/src/glShared.h	Thu Feb 23 23:36:59 2017 +0200
+++ b/src/glShared.h	Sat Feb 25 14:30:10 2017 +0200
@@ -75,7 +75,7 @@
 enum class VboSubclass
 {
 	Surfaces,
-	NormalColors,
+	RegularColors,
 	PickColors,
 	BfcFrontColors,
 	BfcBackColors,
@@ -90,16 +90,3 @@
 {
 	NumVbos = EnumLimits<VboClass>::Count * EnumLimits<VboSubclass>::Count
 };
-
-// KDevelop doesn't seem to understand some VBO stuff
-#ifdef IN_IDE_PARSER
-using GLint = int;
-using GLsizei = int;
-using GLenum = unsigned int;
-using GLuint = unsigned int;
-void glBindBuffer (GLenum, GLuint);
-void glGenBuffers (GLuint, GLuint*);
-void glDeleteBuffers (GLuint, GLuint*);
-void glBufferData (GLuint, GLuint, void*, GLuint);
-void glBufferSubData (GLenum, GLint, GLsizei, void*);
-#endif
--- a/src/glcompiler.cpp	Thu Feb 23 23:36:59 2017 +0200
+++ b/src/glcompiler.cpp	Sat Feb 25 14:30:10 2017 +0200
@@ -25,45 +25,48 @@
 #include "documentmanager.h"
 #include "grid.h"
 
-struct GLErrorInfo
-{
-	GLenum	value;
-	QString	text;
-};
-
-static const GLErrorInfo g_GLErrors[] =
+void checkGLError(HierarchyElement* element, QString file, int line)
 {
-	{ GL_NO_ERROR,						"No error" },
-	{ GL_INVALID_ENUM,					"Unacceptable enumerator passed" },
-	{ GL_INVALID_VALUE,					"Numeric argument out of range" },
-	{ GL_INVALID_OPERATION,				"The operation is not allowed to be done in this state" },
-	{ GL_INVALID_FRAMEBUFFER_OPERATION,	"Framebuffer object is not complete"},
-	{ GL_OUT_OF_MEMORY,					"Out of memory" },
-	{ GL_STACK_UNDERFLOW,				"The operation would have caused an underflow" },
-	{ GL_STACK_OVERFLOW,				"The operation would have caused an overflow" },
-};
+	struct ErrorInfo
+	{
+		GLenum	value;
+		QString	text;
+	};
 
-void CheckGLErrorImpl (HierarchyElement* element, const char* file, int line)
-{
-	QString errmsg;
-	GLenum errnum = glGetError();
+	static const ErrorInfo knownErrors[] =
+	{
+	    { GL_NO_ERROR,						"No error" },
+	    { GL_INVALID_ENUM,					"Unacceptable enumerator passed" },
+	    { GL_INVALID_VALUE,					"Numeric argument out of range" },
+	    { GL_INVALID_OPERATION,				"The operation is not allowed to be done in this state" },
+	    { GL_INVALID_FRAMEBUFFER_OPERATION,	"Framebuffer object is not complete"},
+	    { GL_OUT_OF_MEMORY,					"Out of memory" },
+	    { GL_STACK_UNDERFLOW,				"The operation would have caused an underflow" },
+	    { GL_STACK_OVERFLOW,				"The operation would have caused an overflow" },
+	};
 
-	if (errnum == GL_NO_ERROR)
-		return;
+	GLenum errorNumber = glGetError();
 
-	for (const GLErrorInfo& err : g_GLErrors)
+	if (errorNumber != GL_NO_ERROR)
 	{
-		if (err.value == errnum)
+		QString errorMessage;
+
+		for (const ErrorInfo& error : knownErrors)
 		{
-			errmsg = err.text;
-			break;
+			if (error.value == errorNumber)
+			{
+				errorMessage = error.text;
+				break;
+			}
 		}
+
+		element->print("OpenGL ERROR: at %1:%2: %3", Basename(file), line, errorMessage);
 	}
-
-	element->print ("OpenGL ERROR: at %1:%2: %3", Basename (QString (file)), line, errmsg);
 }
 
-
+/*
+ * Constructs a GL compiler.
+ */
 GLCompiler::GLCompiler (GLRenderer* renderer) :
 	HierarchyElement (renderer),
 	m_renderer (renderer)
@@ -78,23 +81,29 @@
 		stageForCompilation(object);
 }
 
-
+/*
+ * Initializes the VBOs after OpenGL is initialized.
+ */
 void GLCompiler::initialize()
 {
 	initializeOpenGLFunctions();
-	glGenBuffers (NumVbos, &m_vbo[0]);
+	glGenBuffers(countof(m_vbo), &m_vbo[0]);
 	CHECK_GL_ERROR();
 }
 
-
+/*
+ * Destructs the VBOs when the compiler is deleted.
+ */
 GLCompiler::~GLCompiler()
 {
-	glDeleteBuffers (NumVbos, &m_vbo[0]);
+	glDeleteBuffers(countof(m_vbo), &m_vbo[0]);
 	CHECK_GL_ERROR();
 }
 
-
-QColor GLCompiler::indexColorForID (int id) const
+/*
+ * Returns an index color for the LDObject ID given. This color represents the object in the picking scene.
+ */
+QColor GLCompiler::indexColorForID (qint32 id) const
 {
 	// Calculate a color based from this index. This method caters for
 	// 16777216 objects. I don't think that will be exceeded anytime soon. :)
@@ -139,7 +148,7 @@
 		color = polygonOwner->randomColor();
 		break;
 
-	case VboSubclass::NormalColors:
+	case VboSubclass::RegularColors:
 		// For normal colors, use the polygon's color.
 		if (polygon.color == MainColor)
 		{
@@ -206,26 +215,34 @@
 	return color;
 }
 
-
+/*
+ * Tells the compiler that a merge of VBOs is required.
+ */
 void GLCompiler::needMerge()
 {
 	for (int i = 0; i < countof (m_vboChanged); ++i)
 		m_vboChanged[i] = true;
 }
 
-
-void GLCompiler::stageForCompilation (LDObject* obj)
+/*
+ * Stages the given object for compilation.
+ */
+void GLCompiler::stageForCompilation(LDObject* obj)
 {
 	m_staged << obj;
 }
 
-
-void GLCompiler::unstage (LDObject* obj)
+/*
+ * Removes an object from the set of objects to be compiled.
+ */
+void GLCompiler::unstage(LDObject* obj)
 {
 	m_staged.remove (obj);
 }
 
-
+/*
+ * Compiles all staged objects.
+ */
 void GLCompiler::compileStaged()
 {
 	for (LDObject* object : m_staged)
@@ -234,66 +251,78 @@
 	m_staged.clear();
 }
 
-
-void GLCompiler::prepareVBO (int vbonum, const Model* model)
+/*
+ * Prepares a VBO for rendering. The VBO is merged if needed.
+ */
+void GLCompiler::prepareVBO (int vbonum)
 {
 	// Compile anything that still awaits it
 	compileStaged();
 
-	if (not m_vboChanged[vbonum])
-		return;
-
-	QVector<GLfloat> vbodata;
+	if (m_vboChanged[vbonum])
+	{
+		// Merge the VBO into a vector of floats.
+		QVector<GLfloat> vbodata;
 
-	for (auto it = m_objectInfo.begin(); it != m_objectInfo.end();)
-	{
-		if (it.key() == nullptr)
+		for (auto it = m_objectInfo.begin(); it != m_objectInfo.end();)
 		{
-			it = m_objectInfo.erase (it);
-			continue;
+			if (it.key() == nullptr)
+			{
+				it = m_objectInfo.erase(it);
+			}
+			else
+			{
+				if (not it.key()->isHidden())
+					vbodata += it->data[vbonum];
+
+				++it;
+			}
 		}
 
-		if (it.key()->model() == model and not it.key()->isHidden())
-			vbodata += it->data[vbonum];
-
-		++it;
+		// Transfer the VBO to the graphics processor.
+		glBindBuffer (GL_ARRAY_BUFFER, m_vbo[vbonum]);
+		glBufferData (GL_ARRAY_BUFFER, countof(vbodata) * sizeof(GLfloat), vbodata.constData(), GL_STATIC_DRAW);
+		glBindBuffer (GL_ARRAY_BUFFER, 0);
+		CHECK_GL_ERROR();
+		m_vboChanged[vbonum] = false;
+		m_vboSizes[vbonum] = countof(vbodata);
 	}
-
-	glBindBuffer (GL_ARRAY_BUFFER, m_vbo[vbonum]);
-	glBufferData (GL_ARRAY_BUFFER, countof(vbodata) * sizeof(GLfloat), vbodata.constData(), GL_STATIC_DRAW);
-	glBindBuffer (GL_ARRAY_BUFFER, 0);
-	CHECK_GL_ERROR();
-	m_vboChanged[vbonum] = false;
-	m_vboSizes[vbonum] = countof(vbodata);
 }
 
-
+/*
+ * Removes the data related to the given object.
+ */
 void GLCompiler::dropObjectInfo(LDObject* object)
 {
 	if (m_objectInfo.contains(object))
 	{
+		// If we have data relating to this object, remove it. The VBOs have changed now and need to be merged.
 		m_objectInfo.remove(object);
 		needMerge();
 	}
 }
 
+/*
+ * Makes the compiler forget about the given object completely.
+ */
 void GLCompiler::forgetObject(LDObject* object)
 {
 	dropObjectInfo(object);
-	m_staged.remove(object);
+	unstage(object);
 }
 
-
-void GLCompiler::compileObject (LDObject* obj)
+/*
+ * Compiles a single object.
+ */
+void GLCompiler::compileObject(LDObject* object)
 {
-	if (obj == nullptr)
+	if (object == nullptr)
 		return;
 
-	ObjectVBOInfo info;
-	info.isChanged = true;
-	dropObjectInfo (obj);
+	ObjectVboData info;
+	dropObjectInfo(object);
 
-	switch (obj->type())
+	switch (object->type())
 	{
 	// Note: We cannot split quads into triangles here, it would mess up the wireframe view.
 	// Quads must go into separate vbos.
@@ -302,33 +331,34 @@
 	case LDObjectType::EdgeLine:
 	case LDObjectType::ConditionalEdge:
 		{
-			LDPolygon* poly = obj->getPolygon();
-			poly->id = obj->id();
-			compilePolygon (*poly, obj, &info);
+			LDPolygon* poly = object->getPolygon();
+			poly->id = object->id();
+			compilePolygon (*poly, object, info);
 			delete poly;
 			break;
 		}
 
+	// TODO: try use interfaces to remove these special treatments?
 	case LDObjectType::SubfileReference:
 		{
-			LDSubfileReference* ref = static_cast<LDSubfileReference*> (obj);
-			auto data = ref->inlinePolygons();
+			LDSubfileReference* subfileReference = static_cast<LDSubfileReference*>(object);
+			auto data = subfileReference->inlinePolygons();
 
 			for (LDPolygon& poly : data)
 			{
-				poly.id = obj->id();
-				compilePolygon (poly, obj, &info);
+				poly.id = object->id();
+				compilePolygon (poly, object, info);
 			}
 			break;
 		}
 
 	case LDObjectType::BezierCurve:
 		{
-			LDBezierCurve* curve = static_cast<LDBezierCurve*> (obj);
+			LDBezierCurve* curve = static_cast<LDBezierCurve*>(object);
 			for (LDPolygon& polygon : curve->rasterizePolygons(grid()->bezierCurveSegments()))
 			{
-				polygon.id = obj->id();
-				compilePolygon (polygon, obj, &info);
+				polygon.id = object->id();
+				compilePolygon (polygon, object, info);
 			}
 		}
 		break;
@@ -337,12 +367,14 @@
 		break;
 	}
 
-	m_objectInfo[obj] = info;
+	m_objectInfo[object] = info;
 	needMerge();
 }
 
-
-void GLCompiler::compilePolygon (LDPolygon& poly, LDObject* topobj, ObjectVBOInfo* objinfo)
+/*
+ * Inserts a single polygon into VBOs.
+ */
+void GLCompiler::compilePolygon(LDPolygon& poly, LDObject* polygonOwner, ObjectVboData& objectInfo)
 {
 	VboClass surface;
 	int vertexCount;
@@ -371,8 +403,8 @@
 	for (VboSubclass complement : iterateEnum<VboSubclass>())
 	{
 		const int vbonum = vboNumber (surface, complement);
-		QVector<GLfloat>& vbodata = objinfo->data[vbonum];
-		const QColor color = getColorForPolygon (poly, topobj, complement);
+		QVector<GLfloat>& vbodata = objectInfo.data[vbonum];
+		const QColor color = getColorForPolygon (poly, polygonOwner, complement);
 
 		for (int vert = 0; vert < vertexCount; ++vert)
 		{
@@ -400,13 +432,6 @@
 	}
 }
 
-
-void GLCompiler::setRenderer (GLRenderer* renderer)
-{
-	m_renderer = renderer;
-}
-
-
 int GLCompiler::vboNumber (VboClass surface, VboSubclass complement)
 {
 	return (static_cast<int>(surface) * EnumLimits<VboSubclass>::Count) + static_cast<int>(complement);
@@ -424,7 +449,9 @@
 	return m_vboSizes[vbonum];
 }
 
-
+/*
+ * Recompiles the entire model.
+ */
 void GLCompiler::recompile()
 {
 	for (LDObject* object : m_renderer->model()->objects())
--- a/src/glcompiler.h	Thu Feb 23 23:36:59 2017 +0200
+++ b/src/glcompiler.h	Sat Feb 25 14:30:10 2017 +0200
@@ -23,44 +23,43 @@
 #include <QMap>
 #include <QSet>
 
-// =============================================================================
-//
+/*
+ * Compiles LDObjects into polygons for the GLRenderer to draw.
+ */
 class GLCompiler : public QObject, public HierarchyElement, protected QOpenGLFunctions
 {
 	Q_OBJECT
 
 public:
-	struct ObjectVBOInfo
-	{
-		QVector<GLfloat>	data[NumVbos];
-		bool				isChanged;
-	};
-
 	GLCompiler (GLRenderer* renderer);
 	~GLCompiler();
-	QColor getColorForPolygon (LDPolygon& poly, LDObject* topobj, VboSubclass complement);
-	QColor indexColorForID (int id) const;
+
 	void initialize();
-	void needMerge();
-	void prepareVBO (int vbonum, const Model* model);
-	void setRenderer (GLRenderer* compiler);
-	void stageForCompilation (LDObject* obj);
-	void unstage (LDObject* obj);
+	void prepareVBO (int vbonum);
 	GLuint vbo (int vbonum) const;
 	int vboSize (int vbonum) const;
 
 	static int vboNumber (VboClass surface, VboSubclass complement);
 
 private:
+	struct ObjectVboData
+	{
+		QVector<GLfloat> data[NumVbos];
+	};
+
 	void compileStaged();
-	void compilePolygon (LDPolygon& poly, LDObject* topobj, ObjectVBOInfo* objinfo);
+	void compilePolygon (LDPolygon& poly, LDObject* polygonOwner, ObjectVboData& objectInfo);
 	Q_SLOT void compileObject (LDObject* obj);
+	QColor getColorForPolygon (LDPolygon& poly, LDObject* topobj, VboSubclass complement);
+	QColor indexColorForID (qint32 id) const;
+	void needMerge();
 	Q_SLOT void recompile();
 	void dropObjectInfo (LDObject* obj);
 	Q_SLOT void forgetObject(LDObject* object);
+	void stageForCompilation (LDObject* obj);
+	void unstage (LDObject* obj);
 
-private:
-	QMap<LDObject*, ObjectVBOInfo>	m_objectInfo;
+	QMap<LDObject*, ObjectVboData>	m_objectInfo;
 	QSet<LDObject*> m_staged; // Objects that need to be compiled
 	GLuint m_vbo[NumVbos];
 	bool m_vboChanged[NumVbos] = {true};
@@ -68,5 +67,5 @@
 	GLRenderer* m_renderer;
 };
 
-#define CHECK_GL_ERROR() { CheckGLErrorImpl (this, __FILE__, __LINE__); }
-void CheckGLErrorImpl (HierarchyElement* element, const char* file, int line);
+#define CHECK_GL_ERROR() { checkGLError(this, __FILE__, __LINE__); }
+void checkGLError (HierarchyElement* element, QString file, int line);
--- a/src/glrenderer.cpp	Thu Feb 23 23:36:59 2017 +0200
+++ b/src/glrenderer.cpp	Sat Feb 25 14:30:10 2017 +0200
@@ -427,15 +427,15 @@
 			if (m_config->randomColors())
 				colors = VboSubclass::RandomColors;
 			else
-				colors = VboSubclass::NormalColors;
+				colors = VboSubclass::RegularColors;
 
 			drawVbos (VboClass::Triangles, colors);
 			drawVbos (VboClass::Quads, colors);
 		}
 
-		drawVbos (VboClass::Lines, VboSubclass::NormalColors);
+		drawVbos (VboClass::Lines, VboSubclass::RegularColors);
 		glEnable (GL_LINE_STIPPLE);
-		drawVbos (VboClass::ConditionalLines, VboSubclass::NormalColors);
+		drawVbos (VboClass::ConditionalLines, VboSubclass::RegularColors);
 		glDisable (GL_LINE_STIPPLE);
 
 		if (m_config->drawAxes())
@@ -501,9 +501,9 @@
 	int surfaceVboNumber = m_compiler->vboNumber(surface, VboSubclass::Surfaces);
 	int colorVboNumber = m_compiler->vboNumber(surface, colors);
 	int normalVboNumber = m_compiler->vboNumber(surface, VboSubclass::Normals);
-	m_compiler->prepareVBO(surfaceVboNumber, m_model);
-	m_compiler->prepareVBO(colorVboNumber, m_model);
-	m_compiler->prepareVBO(normalVboNumber, m_model);
+	m_compiler->prepareVBO(surfaceVboNumber);
+	m_compiler->prepareVBO(colorVboNumber);
+	m_compiler->prepareVBO(normalVboNumber);
 	GLuint surfaceVbo = m_compiler->vbo(surfaceVboNumber);
 	GLuint colorVbo = m_compiler->vbo(colorVboNumber);
 	GLuint normalVbo = m_compiler->vbo(normalVboNumber);

mercurial