Details

Reviewers

Campbell Barton (campbellbarton)
Clément Foucault (fclem)

Commits

rBS88c88c461026: Fix T51210: Draw Manager: Support for Metaball Drawing
rB88c88c461026: Fix T51210: Draw Manager: Support for Metaball Drawing

Summary

An attempt to solve the task T51210

The DispList indices may represent triangles or quads. Think of a better solution for tessellate?;

The function exposed in rna_meta_api.c was done only to match the curves, but may not be necessary.

Feel free to edit :)

Diff Detail

Event Timeline

Germano Cavalcante (mano-wii) created this revision.Nov 9 2017, 7:51 PM

Feedback after a quick test:

Selection is not working well: it always select the "main one". If I select via the outliner it works [as you mentioned]
If I move the "main one", the others are not updated until I move one of the other metaballs
It only works with Clay

Also:

We need an "engine" for edit mode as well, for drawing feedback.
ASAN crash when closing Blender: P557
Need the "circle" around the balls to be drawn as part of the object mode engine (which is what you should use to allow selecting of different metaballs).

Fix crash when closing Blender;

Draw the circles/elipses of the metaballs (allows selection in object mode);
Separate the DispList loop of type DL_INDEX4 from DL_SURF (the degenerate faces test becomes more localized);
Correct edit mode engine for metaballs.

Fixed crash when you open a file with metaballs;
Support for metaballs in edit mode;

Finished exclusive shader for metaballs;

comments;

fix shader compilation;

Germano Cavalcante (mano-wii) edited the summary of this revision. (Show Details)Nov 13 2017, 4:19 PM

Cleanup edit_metaball_mode.c;
Optimize shader by calculating attributes in C;

cleanup and style;

In general LGTM, some minor comments.

source/blender/blenkernel/BKE_mball.h
80	Name STmat is a bit odd, what is an stmat? - could be more verbose & descriptive
source/blender/blenkernel/intern/mball.c
565–575	could use copy_v3v3 here
source/blender/draw/intern/draw_cache_impl_displist.c
158	Better change this comment into a BLI_assert
source/blender/makesdna/DNA_meta_types.h
64	picky, rather not use capitals in DNA names, since this is runtime data it could be more clear that `stiffness_rad` is only for display. eg: `disp_scale_xform`, `disp_stiffness_rad`

Dalai Felinto (dfelinto) added inline comments.Nov 16 2017, 3:21 PM

source/blender/makesdna/DNA_meta_types.h
64	What with the short names even? Why not use stiffness_radius, transformation_matrix, ...? (also typo: `Tranlation` →`Translation`)

Just quickly scrolled over code and noticed some picky details. Mainly for readability which I care about a lot :)
I agree on not using shortened variable names, except in cases where abbreviation is common even outside of Blender.

source/blender/blenkernel/intern/mball.c
551	`const` should be used for passing arrays that should not be modified. Remember, they are basically non-`const` pointers otherwise.
source/blender/draw/intern/draw_cache_impl_metaball.c
70–72	Dead code :)
112–116	Just change the `calloc` above to be a `malloc` and move this out of the `else`. If cache is `NULL`, the `calloc` does the actual initialization right now (`memset` to 0), which is not obvious on first sight. Also makes sense if in future a `MetaBallBatchCache` member is added that should not be initialized to 0.
source/blender/draw/modes/edit_metaball_mode.c
157	Clean code 101: Instead of adding such comment you should add a new function. E.g. `EDIT_METABALL_cache_populate_radius_visualization`. These functions can also set the `static const` color variables themselves and limit their scope that way. Oh, and would also prefer the loop to be a `for` loop instead, so that you can see how we iterate exactly on first sight :) Also extra `NULL` check above isn't needed then.

Closed by commit rB88c88c461026: Fix T51210: Draw Manager: Support for Metaball Drawing (authored by Germano Cavalcante (mano-wii)). · Explain WhyNov 16 2017, 6:12 PM

This revision was automatically updated to reflect the committed changes.

Germano Cavalcante (mano-wii) mentioned this in rB88c88c461026: Fix T51210: Draw Manager: Support for Metaball Drawing.

Germano Cavalcante (mano-wii) marked 9 inline comments as done.Nov 16 2017, 6:13 PM

Draw Manager: Support for Metaball Drawing
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 9544

source/blender/blenkernel/BKE_mball.h

source/blender/blenkernel/intern/mball.c

source/blender/blenkernel/intern/object_update.c

source/blender/blenloader/intern/readfile.c

source/blender/draw/CMakeLists.txt

source/blender/draw/intern/draw_cache.h

source/blender/draw/intern/draw_cache.c

source/blender/draw/intern/draw_cache_impl.h

source/blender/draw/intern/draw_cache_impl_displist.c

source/blender/draw/intern/draw_cache_impl_metaball.c

source/blender/draw/intern/draw_common.h

source/blender/draw/intern/draw_common.c

source/blender/draw/intern/draw_manager.c

source/blender/draw/modes/edit_metaball_mode.c

source/blender/draw/modes/object_mode.c

source/blender/editors/space_view3d/space_view3d.c

source/blender/gpu/CMakeLists.txt

source/blender/gpu/GPU_shader.h

source/blender/gpu/intern/gpu_shader.c

source/blender/gpu/shaders/gpu_shader_instance_mball_helpers_vert.glsl

source/blender/makesdna/DNA_meta_types.h

source/blender/makesrna/intern/rna_meta_api.c

Draw Manager: Support for Metaball DrawingClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 9544

source/blender/blenkernel/BKE_mball.h

source/blender/blenkernel/intern/mball.c

source/blender/blenkernel/intern/object_update.c

source/blender/blenloader/intern/readfile.c

source/blender/draw/CMakeLists.txt

source/blender/draw/intern/draw_cache.h

source/blender/draw/intern/draw_cache.c

source/blender/draw/intern/draw_cache_impl.h

source/blender/draw/intern/draw_cache_impl_displist.c

source/blender/draw/intern/draw_cache_impl_metaball.c

source/blender/draw/intern/draw_common.h

source/blender/draw/intern/draw_common.c

source/blender/draw/intern/draw_manager.c

source/blender/draw/modes/edit_metaball_mode.c

source/blender/draw/modes/object_mode.c

source/blender/editors/space_view3d/space_view3d.c

source/blender/gpu/CMakeLists.txt

source/blender/gpu/GPU_shader.h

source/blender/gpu/intern/gpu_shader.c

source/blender/gpu/shaders/gpu_shader_instance_mball_helpers_vert.glsl

source/blender/makesdna/DNA_meta_types.h

source/blender/makesrna/intern/rna_meta_api.c

Draw Manager: Support for Metaball Drawing
ClosedPublic
Actions

Revision Contents
Changeset List