[Soc-2013-dev] Weekly Report 13-14 Viewport FX

[David Jeske]

On Sat, Sep 21, 2013 at 1:08 AM, Jason Wilkins <jason.a.wilkins at gmail.com>wrote:

> I see fixing the situation as taking one step at a time while making sure
> things don't get terribly broken in the meantime.
>

Absolutely, and I agree with this approach.

As a result, Blender can now even be run on top of Direct3D, so I think I
> have succeeded for the most part.
>

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.

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.

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*


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!

I'm also not your code-reviewer, so this is just my personal opinion,
offered humbly in case it is useful to you.

I think there is an alternate way to incorporate your code which uses the
existing abstraction layers, rather than if-conditionals.

For example, let's consider this code in ccdmderivedmesh.c....

static void cdDM_drawVerts(DerivedMesh *dm)
> {
>    CDDerivedMesh *cddm = (CDDerivedMesh *) dm;
>    MVert *mv = cddm->mvert;
>    int i;
>


>    if (GPU_buffer_legacy(dm)) {
>       gpuImmediateFormat_V3();
>       gpuBegin(GL_POINTS);
>       for (i = 0; i < dm->numVertData; i++, mv++) {
>          gpuVertex3fv(mv->co);
>
          }

>       gpuEnd();
>       gpuImmediateUnformat();
>    } else {  /* use OpenGL VBOs or Vertex Arrays instead for better,
> faster rendering */
>       GPU_vertex_setup(dm);
>       GPU_commit_aspect();
>       if (!GPU_buffer_legacy(dm)) {
>          if (dm->drawObject->tot_triangle_point) {
>            glDrawArrays(GL_POINTS, 0, dm->drawObject->tot_triangle_point);
>          } else {
>            glDrawArrays(GL_POINTS, 0, dm->drawObject->tot_loose_point);
>         }
>         GPU_buffer_unbind();
>
       }

> }


Another possibility to do this, is to use the function pointer abstraction
to stop the intermingled IF-casing, like this:

static void cdDM_GL1_drawVerts(DerivedMesh *dm)
{

    CDDerivedMesh *cddm = (CDDerivedMesh *) dm;
    MVert *mv = cddm->mvert;
    int i;

   gpuImmediateFormat_V3();
   gpuBegin(GL_POINTS);
   for (i = 0; i < dm->numVertData; i++, mv++) {
      gpuVertex3fv(mv->co);
    }
   gpuEnd();
   gpuImmediateUnformat();
}

static void cdDM_GL3_drawVerts(DerivedMesh *dm)
{
     /* use OpenGL VBOs or Vertex Arrays instead for better, faster
rendering */
     GPU_vertex_setup(dm);
     GPU_commit_aspect();
     if (dm->drawObject->tot_triangle_point) {
         glDrawArrays(GL_POINTS, 0, dm->drawObject->tot_triangle_point);
     } else {
         glDrawArrays(GL_POINTS, 0, dm->drawObject->tot_loose_point);
     }
     GPU_buffer_unbind();
}

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.

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.

Just food for thought...

BTW - what is the extra "if (!GPU_buffer_legacy(dm))" for in the above code?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.blender.org/pipermail/soc-2013-dev/attachments/20130921/59b8abe2/attachment.htm