Fixed in r51759. Thanks for the report! :) On 30/10/2012 08:30, Sergey Sharybin wrote: > 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 >> > >
_______________________________________________ Bf-committers mailing list [email protected] http://lists.blender.org/mailman/listinfo/bf-committers
