Complete fix for [#33002] Wrong vertex color.
authorBastien Montagne <montagne29@wanadoo.fr>
Mon, 29 Oct 2012 16:26:18 +0000 (16:26 +0000)
committerBastien Montagne <montagne29@wanadoo.fr>
Mon, 29 Oct 2012 16:26:18 +0000 (16:26 +0000)
Appart from the color glitch, there was several problems with vpaint:
* "fast_update" mode was never on, because of wrong testing code;
* drawing refresh during stroke in "fast_update" (i.e. no dm rebuild) mode was broken in VBO mode, because updated (tess data) mcol wasn't moved to colors GPUBuffer.

Solved the later point by adding a new DM_DIRTY_MCOL_UPDATE_DRAW flag to DerivedMesh dirty var, which is set each time vpaint stroke directly update me->mcol, and forces GPU_color_setup() to refresh the gpu's colors buffer.

Also got rid of the uggly GPU_color3_upload(), which basically did the same thing, but with an additional intermediate buffer !

source/blender/blenkernel/BKE_DerivedMesh.h
source/blender/blenkernel/intern/cdderivedmesh.c
source/blender/editors/sculpt_paint/paint_vertex.c
source/blender/gpu/GPU_buffers.h
source/blender/gpu/intern/gpu_buffers.c

index fa9223ba6e2c4f1f1ae17ebbca395046dd447506..738d9101e8e7bd6374bd0c1d97cf762697552d33 100644 (file)
@@ -148,6 +148,11 @@ typedef enum DMDrawFlag {
 typedef enum DMDirtyFlag {
        /* dm has valid tessellated faces, but tessellated CDDATA need to be updated. */
        DM_DIRTY_TESS_CDLAYERS = 1 << 0,
+       /* One of the MCOL layers have been updated, force updating of GPUDrawObject's colors buffer.
+        * This is necessary with modern, VBO draw code, as e.g. in vpaint mode me->mcol may be updated
+        * without actually rebuilding dm (hence by defautl keeping same GPUDrawObject, and same colors
+        * buffer, which prevents update during a stroke!). */
+       DM_DIRTY_MCOL_UPDATE_DRAW = 1 << 1,
 } DMDirtyFlag;
 
 typedef struct DerivedMesh DerivedMesh;
index 1f02ad1ea5a87412b7f1faec9cf1a54816f33a70..1415d18b62c5bcdeac4897b1d3b2d20498a31f02 100644 (file)
@@ -617,7 +617,7 @@ static void cdDM_drawFacesTex_common(DerivedMesh *dm,
        MCol *realcol = dm->getTessFaceDataArray(dm, CD_TEXTURE_MCOL);
        float *nors = dm->getTessFaceDataArray(dm, CD_NORMAL);
        MTFace *tf = DM_get_tessface_data_layer(dm, CD_MTFACE);
-       int i, j, orig, *index = DM_get_tessface_data_layer(dm, CD_ORIGINDEX);
+       int i, orig, *index = DM_get_tessface_data_layer(dm, CD_ORIGINDEX);
        int startFace = 0 /*, lastFlag = 0xdeadbeef */ /* UNUSED */;
        MCol *mcol = dm->getTessFaceDataArray(dm, CD_PREVIEW_MCOL);
        if (!mcol)
@@ -717,23 +717,6 @@ static void cdDM_drawFacesTex_common(DerivedMesh *dm,
                        
                        if (col != 0)
 #endif
-                       {
-                               unsigned char *colors = MEM_mallocN(dm->getNumTessFaces(dm) * 4 * 3 * sizeof(unsigned char), "cdDM_drawFacesTex_common");
-                               for (i = 0; i < dm->getNumTessFaces(dm); i++) {
-                                       for (j = 0; j < 4; j++) {
-                                               /* bgr -> rgb is intentional (and stupid), but how its stored internally */
-                                               colors[i * 12 + j * 3] = col[i * 4 + j].b;
-                                               colors[i * 12 + j * 3 + 1] = col[i * 4 + j].g;
-                                               colors[i * 12 + j * 3 + 2] = col[i * 4 + j].r;
-                                       }
-                               }
-                               GPU_color3_upload(dm, colors);
-                               MEM_freeN(colors);
-                               if (realcol)
-                                       dm->drawObject->colType = CD_TEXTURE_MCOL;
-                               else if (mcol)
-                                       dm->drawObject->colType = CD_MCOL;
-                       }
                        GPU_color_setup(dm);
                }
 
@@ -908,8 +891,9 @@ static void cdDM_drawMappedFaces(DerivedMesh *dm,
                int prevstart = 0;
                GPU_vertex_setup(dm);
                GPU_normal_setup(dm);
-               if (useColors && mc)
+               if (useColors && mc) {
                        GPU_color_setup(dm);
+               }
                if (!GPU_buffer_legacy(dm)) {
                        int tottri = dm->drawObject->tot_triangle_point / 3;
                        glShadeModel(GL_SMOOTH);
index 1fdbdd10d6dddc7417fb4c5f714c20439fdebcab..0846dc6d89ea0b5ce776aa4dbde645b3a2a6e885 100644 (file)
@@ -79,6 +79,7 @@
 #include "WM_api.h"
 #include "WM_types.h"
 
+#include "GPU_buffers.h"
 
 #include "ED_armature.h"
 #include "ED_mesh.h"
@@ -106,18 +107,40 @@ static int vertex_paint_use_fast_update_check(Object *ob)
 /* if the polygons from the mesh and the 'derivedFinal' match
  * we can assume that no modifiers are applied and that its worth adding tessellated faces
  * so 'vertex_paint_use_fast_update_check()' returns TRUE */
-static int vertex_paint_use_tessface_check(Object *ob)
+static int vertex_paint_use_tessface_check(Object *ob, Mesh *me)
 {
        DerivedMesh *dm = ob->derivedFinal;
 
-       if (dm) {
-               Mesh *me = BKE_mesh_from_object(ob);
-               return (me->mpoly == CustomData_get_layer(&dm->faceData, CD_MPOLY));
+       if (me && dm) {
+               return (me->mpoly == CustomData_get_layer(&dm->polyData, CD_MPOLY));
        }
 
        return FALSE;
 }
 
+static void update_tessface_data(Object *ob, Mesh *me)
+{
+       if (vertex_paint_use_tessface_check(ob, me)) {
+               /* assume if these exist, that they are up to date & valid */
+               if (!me->mcol || !me->mface) {
+                       /* should always be true */
+                       /* XXX Why this clearing? tessface_calc will reset it anyway! */
+/*                     if (me->mcol) {*/
+/*                             memset(me->mcol, 255, 4 * sizeof(MCol) * me->totface);*/
+/*                     }*/
+
+                       /* create tessfaces because they will be used for drawing & fast updates */
+                       BKE_mesh_tessface_calc(me); /* does own call to update pointers */
+               }
+       }
+       else {
+               if (me->totface) {
+                       /* this wont be used, theres no need to keep it */
+                       BKE_mesh_tessface_clear(me);
+               }
+       }
+
+}
 /* polling - retrieve whether cursor should be set or operator should be done */
 
 /* Returns true if vertex paint mode is active */
@@ -331,24 +354,7 @@ static void make_vertexcol(Object *ob)  /* single ob */
                mesh_update_customdata_pointers(me, TRUE);
        }
 
-       if (vertex_paint_use_tessface_check(ob)) {
-               /* assume if these exist, that they are up to date & valid */
-               if (!me->mcol || !me->mface) {
-                       /* should always be true */
-                       if (me->mcol) {
-                               memset(me->mcol, 255, 4 * sizeof(MCol) * me->totface);
-                       }
-
-                       /* create tessfaces because they will be used for drawing & fast updates */
-                       BKE_mesh_tessface_calc(me); /* does own call to update pointers */
-               }
-       }
-       else {
-               if (me->totface) {
-                       /* this wont be used, theres no need to keep it */
-                       BKE_mesh_tessface_clear(me);
-               }
-       }
+       update_tessface_data(ob, me);
 
        //if (shade)
        //      shadeMeshMCol(scene, ob, me);
@@ -2600,7 +2606,12 @@ static int vpaint_stroke_test_start(bContext *C, struct wmOperator *op, const fl
                make_vertexcol(ob);
        if (me->mloopcol == NULL)
                return OPERATOR_CANCELLED;
-       
+
+       /* Update tessface data if needed
+        * Added here too because e.g. switching to/from edit mode would remove tessface data,
+        * yet "fast_update" could still be used! */
+       update_tessface_data(ob, me);
+
        /* make mode data storage */
        vpd = MEM_callocN(sizeof(struct VPaintData), "VPaintData");
        paint_stroke_set_mode_data(stroke, vpd);
@@ -2616,9 +2627,11 @@ static int vpaint_stroke_test_start(bContext *C, struct wmOperator *op, const fl
        if (vertex_paint_use_fast_update_check(ob)) {
                vpaint_build_poly_facemap(vpd, me);
                vpd->use_fast_update = TRUE;
+/*             printf("Fast update!\n");*/
        }
        else {
                vpd->use_fast_update = FALSE;
+/*             printf("No fast update!\n");*/
        }
 
        /* for filtering */
@@ -2699,11 +2712,18 @@ static void vpaint_paint_poly(VPaint *vp, VPaintData *vpd, Object *ob,
                        ml = me->mloop + mpoly->loopstart;
                        mlc = me->mloopcol + mpoly->loopstart;
                        for (j = 0; j < mpoly->totloop; j++, ml++, mlc++) {
-                               if      (ml->v == mf->v1)            { MESH_MLOOPCOL_TO_MCOL(mlc, mc + 0); }
-                               else if (ml->v == mf->v2)            { MESH_MLOOPCOL_TO_MCOL(mlc, mc + 1); }
-                               else if (ml->v == mf->v3)            { MESH_MLOOPCOL_TO_MCOL(mlc, mc + 2); }
-                               else if (mf->v4 && ml->v == mf->v4)  { MESH_MLOOPCOL_TO_MCOL(mlc, mc + 3); }
-
+                               if (ml->v == mf->v1) {
+                                       MESH_MLOOPCOL_TO_MCOL(mlc, mc + 0);
+                               }
+                               else if (ml->v == mf->v2) {
+                                       MESH_MLOOPCOL_TO_MCOL(mlc, mc + 1);
+                               }
+                               else if (ml->v == mf->v3) {
+                                       MESH_MLOOPCOL_TO_MCOL(mlc, mc + 2);
+                               }
+                               else if (mf->v4 && ml->v == mf->v4) {
+                                       MESH_MLOOPCOL_TO_MCOL(mlc, mc + 3);
+                               }
                        }
                }
        }
@@ -2784,6 +2804,10 @@ static void vpaint_stroke_update_step(bContext *C, struct PaintStroke *stroke, P
                 * avoid this if we can! */
                DAG_id_tag_update(ob->data, 0);
        }
+       else if (!GPU_buffer_legacy(ob->derivedFinal)) {
+               /* If using new VBO drawing, mark mcol as dirty to force colors gpu buffer refresh! */
+               ob->derivedFinal->dirty |= DM_DIRTY_MCOL_UPDATE_DRAW;
+       }
 }
 
 static void vpaint_stroke_done(const bContext *C, struct PaintStroke *stroke)
index de83a717bce5fbe2403c0cd9dc8e6ac5bc8f2a56..f9183afd3efa58ec5dfbd005f97cc04a0523d1d6 100644 (file)
@@ -141,8 +141,6 @@ void *GPU_buffer_lock( GPUBuffer *buffer );
 void *GPU_buffer_lock_stream( GPUBuffer *buffer );
 void GPU_buffer_unlock( GPUBuffer *buffer );
 
-/* upload three unsigned chars, representing RGB colors, for each vertex. Resets dm->drawObject->colType to -1 */
-void GPU_color3_upload(struct DerivedMesh *dm, unsigned char *data );
 /* switch color rendering on=1/off=0 */
 void GPU_color_switch( int mode );
 
index 3f04de91900b77be5415abe3dc501c5527a5f73c..98a25ebe7696549b4ae2efd9c0876ad09d66e278 100644 (file)
@@ -708,34 +708,6 @@ static void GPU_buffer_copy_uv(DerivedMesh *dm, float *varray, int *index, int *
        }
 }
 
-
-static void GPU_buffer_copy_color3(DerivedMesh *dm, float *varray_, int *index, int *mat_orig_to_new, void *user)
-{
-       int i, totface;
-       char *varray = (char *)varray_;
-       char *mcol = (char *)user;
-       MFace *f = dm->getTessFaceArray(dm);
-
-       totface = dm->getNumTessFaces(dm);
-       for (i = 0; i < totface; i++, f++) {
-               int start = index[mat_orig_to_new[f->mat_nr]];
-
-               /* v1 v2 v3 */
-               copy_v3_v3_char(&varray[start], &mcol[i * 12]);
-               copy_v3_v3_char(&varray[start + 3], &mcol[i * 12 + 3]);
-               copy_v3_v3_char(&varray[start + 6], &mcol[i * 12 + 6]);
-               index[mat_orig_to_new[f->mat_nr]] += 9;
-
-               if (f->v4) {
-                       /* v3 v4 v1 */
-                       copy_v3_v3_char(&varray[start + 9], &mcol[i * 12 + 6]);
-                       copy_v3_v3_char(&varray[start + 12], &mcol[i * 12 + 9]);
-                       copy_v3_v3_char(&varray[start + 15], &mcol[i * 12]);
-                       index[mat_orig_to_new[f->mat_nr]] += 9;
-               }
-       }
-}
-
 static void copy_mcol_uc3(unsigned char *v, unsigned char *col)
 {
        v[0] = col[3];
@@ -944,7 +916,7 @@ static GPUBuffer *gpu_buffer_setup_type(DerivedMesh *dm, GPUBufferType type)
 static GPUBuffer *gpu_buffer_setup_common(DerivedMesh *dm, GPUBufferType type)
 {
        GPUBuffer **buf;
-       
+
        if (!dm->drawObject)
                dm->drawObject = GPU_drawobject_new(dm);
 
@@ -1008,6 +980,14 @@ void GPU_uv_setup(DerivedMesh *dm)
 
 void GPU_color_setup(DerivedMesh *dm)
 {
+       /* In paint mode, dm may stay the same during stroke, however we still want to update colors! */
+       if ((dm->dirty & DM_DIRTY_MCOL_UPDATE_DRAW) && dm->drawObject) {
+               GPUBuffer **buf = gpu_drawobject_buffer_from_type(dm->drawObject, GPU_BUFFER_COLOR);
+               GPU_buffer_free(*buf);
+               *buf = NULL;
+       }
+       dm->dirty &= ~DM_DIRTY_MCOL_UPDATE_DRAW;
+
        if (!gpu_buffer_setup_common(dm, GPU_BUFFER_COLOR))
                return;
 
@@ -1168,20 +1148,6 @@ void GPU_buffer_unbind(void)
                glBindBufferARB(GL_ARRAY_BUFFER_ARB, 0);
 }
 
-/* confusion: code in cdderivedmesh calls both GPU_color_setup and
- * GPU_color3_upload; both of these set the `colors' buffer, so seems
- * like it will just needlessly overwrite? --nicholas */
-void GPU_color3_upload(DerivedMesh *dm, unsigned char *data)
-{
-       if (dm->drawObject == 0)
-               dm->drawObject = GPU_drawobject_new(dm);
-       GPU_buffer_free(dm->drawObject->colors);
-
-       dm->drawObject->colors = gpu_buffer_setup(dm, dm->drawObject, 3,
-                                                 sizeof(char) * 3 * dm->drawObject->tot_triangle_point,
-                                                 GL_ARRAY_BUFFER_ARB, data, GPU_buffer_copy_color3);
-}
-
 void GPU_color_switch(int mode)
 {
        if (mode) {