{"id":19,"date":"2014-06-03T18:08:14","date_gmt":"2014-06-03T18:08:14","guid":{"rendered":"https:\/\/blogs.scummvm.org\/subr3v\/?p=19"},"modified":"2022-05-23T18:09:21","modified_gmt":"2022-05-23T18:09:21","slug":"refactoring-part-2","status":"publish","type":"post","link":"https:\/\/blogs.scummvm.org\/subr3v\/2014\/06\/03\/refactoring-part-2\/","title":{"rendered":"Refactoring part 2"},"content":{"rendered":"

As planned, I am still working on refactoring the maths code as I’ve encountered some problems on the road.<\/p>\n

The renderer is now working but there are some minor lighting issues that I am trying to address, however the code is now much cleaner and more readable than before and to show you this I am going to paste some snippets of before and after refactoring scenarios:<\/p>\n

Before refactoring –<\/p>\n

float\u00a0*m;\r\nV4\u00a0*n;\r\n \r\nif\u00a0(c->lighting_enabled)\u00a0{\r\n \/\/\u00a0eye\u00a0coordinates\u00a0needed\u00a0for\u00a0lighting\r\n \r\n m\u00a0=\u00a0&c->matrix_stack_ptr[0]->m[0][0];\r\n v->ec.X\u00a0=\u00a0(v->coord.X\u00a0*\u00a0m[0]\u00a0+\u00a0v->coord.Y\u00a0*\u00a0m[1]\u00a0+\r\n    v->coord.Z\u00a0*\u00a0m[2]\u00a0+\u00a0m[3]);\r\n v->ec.Y\u00a0=\u00a0(v->coord.X\u00a0*\u00a0m[4]\u00a0+\u00a0v->coord.Y\u00a0*\u00a0m[5]\u00a0+\r\n    v->coord.Z\u00a0*\u00a0m[6]\u00a0+\u00a0m[7]);\r\n v->ec.Z\u00a0=\u00a0(v->coord.X\u00a0*\u00a0m[8]\u00a0+\u00a0v->coord.Y\u00a0*\u00a0m[9]\u00a0+\r\n    v->coord.Z\u00a0*\u00a0m[10]\u00a0+\u00a0m[11]);\r\n v->ec.W\u00a0=\u00a0(v->coord.X\u00a0*\u00a0m[12]\u00a0+\u00a0v->coord.Y\u00a0*\u00a0m[13]\u00a0+\r\n    v->coord.Z\u00a0*\u00a0m[14]\u00a0+\u00a0m[15]);\r\n \r\n \/\/\u00a0projection\u00a0coordinates\r\n m\u00a0=\u00a0&c->matrix_stack_ptr[1]->m[0][0];\r\n v->pc.X\u00a0=\u00a0(v->ec.X\u00a0*\u00a0m[0]\u00a0+\u00a0v->ec.Y\u00a0*\u00a0m[1]\u00a0+\u00a0v->ec.Z\u00a0*\u00a0m[2]\u00a0+\u00a0v->ec.W\u00a0*\u00a0m[3]);\r\n v->pc.Y\u00a0=\u00a0(v->ec.X\u00a0*\u00a0m[4]\u00a0+\u00a0v->ec.Y\u00a0*\u00a0m[5]\u00a0+\u00a0v->ec.Z\u00a0*\u00a0m[6]\u00a0+\u00a0v->ec.W\u00a0*\u00a0m[7]);\r\n v->pc.Z\u00a0=\u00a0(v->ec.X\u00a0*\u00a0m[8]\u00a0+\u00a0v->ec.Y\u00a0*\u00a0m[9]\u00a0+\u00a0v->ec.Z\u00a0*\u00a0m[10]\u00a0+\u00a0v->ec.W\u00a0*\u00a0m[11]);\r\n v->pc.W\u00a0=\u00a0(v->ec.X\u00a0*\u00a0m[12]\u00a0+\u00a0v->ec.Y\u00a0*\u00a0m[13]\u00a0+\u00a0v->ec.Z\u00a0*\u00a0m[14]\u00a0+\u00a0v->ec.W\u00a0*\u00a0m[15]);\r\n \r\n m\u00a0=\u00a0&c->matrix_model_view_inv.m[0][0];\r\n n\u00a0=\u00a0&c->current_normal;\r\n \r\n v->normal.X\u00a0=\u00a0(n->X\u00a0*\u00a0m[0]\u00a0+\u00a0n->Y\u00a0*\u00a0m[1]\u00a0+\u00a0n->Z\u00a0*\u00a0m[2]);\r\n v->normal.Y\u00a0=\u00a0(n->X\u00a0*\u00a0m[4]\u00a0+\u00a0n->Y\u00a0*\u00a0m[5]\u00a0+\u00a0n->Z\u00a0*\u00a0m[6]);\r\n v->normal.Z\u00a0=\u00a0(n->X\u00a0*\u00a0m[8]\u00a0+\u00a0n->Y\u00a0*\u00a0m[9]\u00a0+\u00a0n->Z\u00a0*\u00a0m[10]);\r\n \r\n if\u00a0(c->normalize_enabled)\u00a0{\r\n  gl_V3_Norm(&v->normal);\r\n }\r\n}\u00a0else\u00a0{\r\n \/\/\u00a0no\u00a0eye\u00a0coordinates\u00a0needed,\u00a0no\u00a0normal\r\n \/\/\u00a0NOTE:\u00a0W\u00a0=\u00a01\u00a0is\u00a0assumed\r\n m\u00a0=\u00a0&c->matrix_model_projection.m[0][0];\r\n \r\n v->pc.X\u00a0=\u00a0(v->coord.X\u00a0*\u00a0m[0]\u00a0+\u00a0v->coord.Y\u00a0*\u00a0m[1]\u00a0+\u00a0v->coord.Z\u00a0*\u00a0m[2]\u00a0+\u00a0m[3]);\r\n v->pc.Y\u00a0=\u00a0(v->coord.X\u00a0*\u00a0m[4]\u00a0+\u00a0v->coord.Y\u00a0*\u00a0m[5]\u00a0+\u00a0v->coord.Z\u00a0*\u00a0m[6]\u00a0+\u00a0m[7]);\r\n v->pc.Z\u00a0=\u00a0(v->coord.X\u00a0*\u00a0m[8]\u00a0+\u00a0v->coord.Y\u00a0*\u00a0m[9]\u00a0+\u00a0v->coord.Z\u00a0*\u00a0m[10]\u00a0+\u00a0m[11]);\r\n if\u00a0(c->matrix_model_projection_no_w_transform)\u00a0{\r\n  v->pc.W\u00a0=\u00a0m[15];\r\n }\u00a0else\u00a0{\r\n  v->pc.W\u00a0=\u00a0(v->coord.X\u00a0*\u00a0m[12]\u00a0+\u00a0v->coord.Y\u00a0*\u00a0m[13]\u00a0+\u00a0v->coord.Z\u00a0*\u00a0m[14]\u00a0+\u00a0m[15]);\r\n }\r\n}\r\n \r\nv->clip_code\u00a0=\u00a0gl_clipcode(v->pc.X,\u00a0v->pc.Y,\u00a0v->pc.Z,\u00a0v->pc.W);<\/pre>\n
After Refactoring –<\/p>\n
Matrix4\u00a0*m;\r\nVector4\u00a0*n;\r\n \r\nif\u00a0(c->lighting_enabled)\u00a0{\r\n \/\/\u00a0eye\u00a0coordinates\u00a0needed\u00a0for\u00a0lighting\r\n \r\n m\u00a0=\u00a0c->matrix_stack_ptr[0];\r\n v->ec\u00a0=\u00a0m->transform3x4(v->coord);\r\n \r\n \/\/\u00a0projection\u00a0coordinates\r\n m\u00a0=\u00a0c->matrix_stack_ptr[1];\r\n v->pc\u00a0=\u00a0m->transform(v->ec);\r\n \r\n m\u00a0=\u00a0&c->matrix_model_view_inv;\r\n n\u00a0=\u00a0&c->current_normal;\r\n \r\n v->normal\u00a0=\u00a0m->transform3x3(n->toVector3());\r\n \r\n if\u00a0(c->normalize_enabled)\u00a0{\r\n  v->normal.normalize();\r\n }\r\n}\u00a0else\u00a0{\r\n \/\/\u00a0no\u00a0eye\u00a0coordinates\u00a0needed,\u00a0no\u00a0normal\r\n \/\/\u00a0NOTE:\u00a0W\u00a0=\u00a01\u00a0is\u00a0assumed\r\n m\u00a0=\u00a0&c->matrix_model_projection;\r\n \r\n v->pc\u00a0=\u00a0m->transform3x4(v->coord);\r\n if\u00a0(c->matrix_model_projection_no_w_transform)\u00a0{\r\n  v->pc.setW(m->get(3,3));\u00a0\r\n }\r\n}\r\n \r\nv->clip_code\u00a0=\u00a0gl_clipcode(v->pc.getX(),\u00a0v->pc.getY(),\u00a0v->pc.getZ(),\u00a0v->pc.getW());<\/pre>\n
As you can see the code is more readable and the operations performed are clearly stated.
\nWhile trying to fix some issues that arised during the refactoring I also stumbled upon the git command “stash”: this command lets you store the changes in your code in a place and then apply them afterwards, I used this system to keep my changes while I was switching betweeen branches to execute the old and new version of the code while fixing all the issues I found, so I highly reccommend to read more about it and learn how to use it!<\/p>\n","protected":false},"excerpt":{"rendered":"
As planned, I am still working on refactoring the maths code as I’ve encountered some problems on the road. The renderer is now working but there are some minor lighting issues that I am trying to address, however the code is now much cleaner and more readable than before and to show you this I […]<\/p>\n","protected":false},"author":1,"featured_media":0,"comment_status":"closed","ping_status":"closed","sticky":false,"template":"","format":"standard","meta":{"footnotes":""},"categories":[1],"tags":[],"class_list":["post-19","post","type-post","status-publish","format-standard","hentry","category-uncategorized"],"_links":{"self":[{"href":"https:\/\/blogs.scummvm.org\/subr3v\/wp-json\/wp\/v2\/posts\/19","targetHints":{"allow":["GET"]}}],"collection":[{"href":"https:\/\/blogs.scummvm.org\/subr3v\/wp-json\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/blogs.scummvm.org\/subr3v\/wp-json\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/blogs.scummvm.org\/subr3v\/wp-json\/wp\/v2\/users\/1"}],"replies":[{"embeddable":true,"href":"https:\/\/blogs.scummvm.org\/subr3v\/wp-json\/wp\/v2\/comments?post=19"}],"version-history":[{"count":2,"href":"https:\/\/blogs.scummvm.org\/subr3v\/wp-json\/wp\/v2\/posts\/19\/revisions"}],"predecessor-version":[{"id":21,"href":"https:\/\/blogs.scummvm.org\/subr3v\/wp-json\/wp\/v2\/posts\/19\/revisions\/21"}],"wp:attachment":[{"href":"https:\/\/blogs.scummvm.org\/subr3v\/wp-json\/wp\/v2\/media?parent=19"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/blogs.scummvm.org\/subr3v\/wp-json\/wp\/v2\/categories?post=19"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/blogs.scummvm.org\/subr3v\/wp-json\/wp\/v2\/tags?post=19"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}