[Bf-codereview] Blender Dynamic Paint (soc-2011-carrot) (issue 5189041)

g.ulairi at gmail.com g.ulairi at gmail.com
Tue Oct 11 14:13:12 CEST 2011


Pardon, didn't have enough time yet to make deeper review. Answered some
comments.


http://codereview.appspot.com/5189041/diff/1/source/blender/blenkernel/intern/dynamicpaint.c
File source/blender/blenkernel/intern/dynamicpaint.c (right):

http://codereview.appspot.com/5189041/diff/1/source/blender/blenkernel/intern/dynamicpaint.c#newcode74
source/blender/blenkernel/intern/dynamicpaint.c:74: #include
"intern/openexr/openexr_api.h"
It should be something like ibuf->ftype= OPENEXR and then IMB_saveiff
should save this ibuf as exr image.

http://codereview.appspot.com/5189041/diff/1/source/blender/blenkernel/intern/dynamicpaint.c#newcode78
source/blender/blenkernel/intern/dynamicpaint.c:78: #include
"intern/MOD_util.h"
Didn't use it before but sounds like it's custom data layer related. So
probably it should be in customdata.c

http://codereview.appspot.com/5189041/diff/1/source/blender/blenkernel/intern/dynamicpaint.c#newcode85
source/blender/blenkernel/intern/dynamicpaint.c:85: #include
"../render/intern/include/texture.h"
Yes, understand this. I asked Brecht to help you here.

http://codereview.appspot.com/5189041/diff/1/source/blender/blenkernel/intern/dynamicpaint.c#newcode499
source/blender/blenkernel/intern/dynamicpaint.c:499: ob->recalc =
oflags;
Problem that you aren't restoring all states, you're restoring just
flags. Probably just tagging object to be re-calculated after your work
is nicer -- all dependencies would be updated automatically due to
depsgraph.

http://codereview.appspot.com/5189041/diff/1/source/blender/blenkernel/intern/dynamicpaint.c#newcode659
source/blender/blenkernel/intern/dynamicpaint.c:659: #ifdef _OPENMP
It's not really hurts, just other places doesn't use this check. In
fact, think can be kept unchanged here.

http://codereview.appspot.com/5189041/diff/1/source/blender/blenkernel/intern/dynamicpaint.c#newcode990
source/blender/blenkernel/intern/dynamicpaint.c:990: if (scene) {
I see. And yes, comments for non-obvious cases helps a lot :)

http://codereview.appspot.com/5189041/


More information about the Bf-codereview mailing list