<div dir="ltr">Actually, that isn't my code, but existing Blender code. <div><br></div><div>I have introduced a lot of conditionals myself, but unlike the big object drawing paths, I think it would be premature to refactor these points into their own functions. So, I made them easy to find by always switching on the same variable. There are already several cases where I could imagine replacing the condition with a function that is used more than once.</div>
</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Sat, Sep 21, 2013 at 9:15 PM, David Jeske <span dir="ltr"><<a href="mailto:davidj@gmail.com" target="_blank">davidj@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="im">On Sat, Sep 21, 2013 at 1:08 AM, Jason Wilkins <span dir="ltr"><<a href="mailto:jason.a.wilkins@gmail.com" target="_blank">jason.a.wilkins@gmail.com</a>></span> wrote:<br>
</div><div class="gmail_extra"><div class="gmail_quote"><div class="im"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr">
<div>I see fixing the situation as taking one step at a time while making sure things don't get terribly broken in the meantime.<br></div></div></blockquote><div><br></div></div><div>Absolutely, and I agree with this approach.</div>
<div><br></div><div><div class="im"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">As a result, Blender can now even be run on top of Direct3D, so I think I have succeeded for the most part.</div>
</blockquote><div><br></div></div><div>I think running on ANGLE is fantastic. There is a reason Google decided to make ANGLE instead of running on-top of OpenGL on windows. (without judgement) Direct3D drivers are much more reliable and better supported on windows. IMO, their shader translation also allows them to better control GLSL syntax and feature support across platforms for webgl. </div>
<div><br></div><div>With 94%+ of Steam clients reporting DX10+ GPUs, and 98.6% reporting DX9/SM2 GPUs, we're really darn close to a day where ANGLE could be the reasonable baseline for 3d. ( ANGLE = GLES2.0 ~= DX9/SM2 ~= GL2.1/GLSL120 ) I hope the future brings us an ANGLE2 which supports geometry shaders and tessellation using the same implementation strategy, sadly it probably won't happen until those features make it into GLES and WebGL. </div>
</div><div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">My goal was to create a middleware library that can be used to replace deprecated functionality in such a way that the rest of Blender does not have to be completely rewritten *all at once*</blockquote>
<div><br></div></div><div>Let me rephrase my comment. This is not intended to be critical, just to offer a perspective and possibility for you to consider in your great work! </div><div><br></div><div>I'm also not your code-reviewer, so this is just my personal opinion, offered humbly in case it is useful to you.</div>
<div><br></div><div>I think there is an alternate way to incorporate your code which uses the existing abstraction layers, rather than if-conditionals. </div><div><br></div><div>For example, let's consider this code in ccdmderivedmesh.c....</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">static void cdDM_drawVerts(DerivedMesh *dm)<br>
{<br> CDDerivedMesh *cddm = (CDDerivedMesh *) dm;<br> MVert *mv = cddm->mvert;<br> int i;<br></blockquote><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
if (GPU_buffer_legacy(dm)) {<br> gpuImmediateFormat_V3();<br> gpuBegin(GL_POINTS);<br> for (i = 0; i < dm->numVertData; i++, mv++) {<br> gpuVertex3fv(mv->co);<br></blockquote><div> } <br>
</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"> gpuEnd();<br> gpuImmediateUnformat(); <br>
} else { /* use OpenGL VBOs or Vertex Arrays instead for better, faster rendering */<br> GPU_vertex_setup(dm);<br> GPU_commit_aspect();<br> if (!GPU_buffer_legacy(dm)) {<br> if (dm->drawObject->tot_triangle_point) {<br>
glDrawArrays(GL_POINTS, 0, dm->drawObject->tot_triangle_point);<br> } else {<br> glDrawArrays(GL_POINTS, 0, dm->drawObject->tot_loose_point);<br> }<br> GPU_buffer_unbind();<br>
</blockquote><div> } </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">}</blockquote></div><div class="gmail_quote">
<br><div>Another possibility to do this, is to use the function pointer abstraction to stop the intermingled IF-casing, like this:</div><div><br></div><div>static void cdDM_GL1_drawVerts(DerivedMesh *dm) <br></div>{<br><br>
CDDerivedMesh *cddm = (CDDerivedMesh *) dm;<br> MVert *mv = cddm->mvert;<br> int i;<br><br> gpuImmediateFormat_V3();<br> gpuBegin(GL_POINTS);<br> for (i = 0; i < dm->numVertData; i++, mv++) {<br>
gpuVertex3fv(mv->co);</div>
<div class="gmail_quote"> }<br> gpuEnd();<br> gpuImmediateUnformat();<br>}<br><br>static void cdDM_GL3_drawVerts(DerivedMesh *dm)<br>{</div><div class="gmail_quote"> /* use OpenGL VBOs or Vertex Arrays instead for better, faster rendering */<br>
GPU_vertex_setup(dm);<br> GPU_commit_aspect(); <br> if (dm->drawObject->tot_triangle_point) {<br> glDrawArrays(GL_POINTS, 0, dm->drawObject->tot_triangle_point);</div><div class="gmail_quote">
} else {<br> glDrawArrays(GL_POINTS, 0, dm->drawObject->tot_loose_point);</div><div class="gmail_quote"> }<br> GPU_buffer_unbind();<br>}<br><br>The simplest way to choose between these two implementations is to make a single conditional in ccDM_create(..) which fills in the function pointers differently based on the graphics card. </div>
<div class="gmail_quote"><br></div><div class="gmail_quote">If runtime switchable backends is desired, I would probably pull the repetitive function pointers out of DerivedMesh and replace them with a single pointer to a DerivedMeshImpl struct which would serve as a global singleton vtable, somewhat like the C++ ABI's vtable.</div>
<div class="gmail_quote"><br></div><div class="gmail_quote">Just food for thought...</div><div class="gmail_quote"><br></div><div class="gmail_quote">BTW - what is the extra "if (!GPU_buffer_legacy(dm))" for in the above code?</div>
<div class="gmail_quote"><br></div></div></div>
<br>_______________________________________________<br>
Soc-2013-dev mailing list<br>
<a href="mailto:Soc-2013-dev@blender.org">Soc-2013-dev@blender.org</a><br>
<a href="http://lists.blender.org/mailman/listinfo/soc-2013-dev" target="_blank">http://lists.blender.org/mailman/listinfo/soc-2013-dev</a><br>
<br></blockquote></div><br></div>