Hi, This commit ended up with such an artifacts for textured cube: http://www.pasteall.org/pic/show.php?id=39736
Test file: http://www.pasteall.org/blend/17180 On Mon, Oct 29, 2012 at 10:26 PM, Bastien Montagne <[email protected]>wrote: > Revision: 51737 > > http://projects.blender.org/scm/viewvc.php?view=rev&root=bf-blender&revision=51737 > Author: mont29 > Date: 2012-10-29 16:26:18 +0000 (Mon, 29 Oct 2012) > Log Message: > ----------- > Complete fix for [#33002] Wrong vertex color. > > 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?\194?\160! > > Modified Paths: > -------------- > trunk/blender/source/blender/blenkernel/BKE_DerivedMesh.h > trunk/blender/source/blender/blenkernel/intern/cdderivedmesh.c > trunk/blender/source/blender/editors/sculpt_paint/paint_vertex.c > trunk/blender/source/blender/gpu/GPU_buffers.h > trunk/blender/source/blender/gpu/intern/gpu_buffers.c > > Modified: trunk/blender/source/blender/blenkernel/BKE_DerivedMesh.h > =================================================================== > --- trunk/blender/source/blender/blenkernel/BKE_DerivedMesh.h 2012-10-29 > 16:07:16 UTC (rev 51736) > +++ trunk/blender/source/blender/blenkernel/BKE_DerivedMesh.h 2012-10-29 > 16:26:18 UTC (rev 51737) > @@ -148,6 +148,11 @@ > 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; > > Modified: trunk/blender/source/blender/blenkernel/intern/cdderivedmesh.c > =================================================================== > --- trunk/blender/source/blender/blenkernel/intern/cdderivedmesh.c > 2012-10-29 16:07:16 UTC (rev 51736) > +++ trunk/blender/source/blender/blenkernel/intern/cdderivedmesh.c > 2012-10-29 16:26:18 UTC (rev 51737) > @@ -617,7 +617,7 @@ > 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 @@ > > 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 @@ > 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); > > Modified: trunk/blender/source/blender/editors/sculpt_paint/paint_vertex.c > =================================================================== > --- trunk/blender/source/blender/editors/sculpt_paint/paint_vertex.c > 2012-10-29 16:07:16 UTC (rev 51736) > +++ trunk/blender/source/blender/editors/sculpt_paint/paint_vertex.c > 2012-10-29 16:26:18 UTC (rev 51737) > @@ -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 @@ > /* 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,25 +354,8 @@ > 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); > - } > + update_tessface_data(ob, me); > > - /* 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); > - } > - } > - > //if (shade) > // shadeMeshMCol(scene, ob, me); > //else > @@ -2600,7 +2606,12 @@ > 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 @@ > 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 @@ > 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 @@ > * 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) > > Modified: trunk/blender/source/blender/gpu/GPU_buffers.h > =================================================================== > --- trunk/blender/source/blender/gpu/GPU_buffers.h 2012-10-29 > 16:07:16 UTC (rev 51736) > +++ trunk/blender/source/blender/gpu/GPU_buffers.h 2012-10-29 > 16:26:18 UTC (rev 51737) > @@ -141,8 +141,6 @@ > 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 ); > > > Modified: trunk/blender/source/blender/gpu/intern/gpu_buffers.c > =================================================================== > --- trunk/blender/source/blender/gpu/intern/gpu_buffers.c 2012-10-29 > 16:07:16 UTC (rev 51736) > +++ trunk/blender/source/blender/gpu/intern/gpu_buffers.c 2012-10-29 > 16:26:18 UTC (rev 51737) > @@ -708,34 +708,6 @@ > } > } > > - > -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_common(DerivedMesh *dm, GPUBufferType > type) > { > GPUBuffer **buf; > - > + > if (!dm->drawObject) > dm->drawObject = GPU_drawobject_new(dm); > > @@ -1008,6 +980,14 @@ > > 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 @@ > 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); > > @@ Diff output truncated at 10240 characters. @@ > _______________________________________________ > Bf-blender-cvs mailing list > [email protected] > http://lists.blender.org/mailman/listinfo/bf-blender-cvs > -- With best regards, Sergey Sharybin _______________________________________________ Bf-committers mailing list [email protected] http://lists.blender.org/mailman/listinfo/bf-committers
