Draw Manager: Support for Metaball Drawing
ClosedPublic

Authored by Germano Cavalcante (mano-wii) on Nov 9 2017, 7:51 PM.

Details

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

Repository
rB Blender

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).
Germano Cavalcante (mano-wii) edited the summary of this revision. (Show Details)
  • Fix crash when closing Blender;
Germano Cavalcante (mano-wii) edited the summary of this revision. (Show Details)
  • 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.
Germano Cavalcante (mano-wii) edited the summary of this revision. (Show Details)
  • Fixed crash when you open a file with metaballs;
  • Support for metaballs in edit mode;
Germano Cavalcante (mano-wii) edited the summary of this revision. (Show Details)
  • Finished exclusive shader for metaballs;
  • fix shader compilation;
Germano Cavalcante (mano-wii) retitled this revision from Draw Manager: Partial implementation of MetaBall drawing to Draw Manager: Support for Metaball Drawing.
Germano Cavalcante (mano-wii) edited the summary of this revision. (Show Details)
  • Cleanup edit_metaball_mode.c;
  • Optimize shader by calculating attributes in C;

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

source/blender/makesdna/DNA_meta_types.h
64

What with the short names even? Why not use stiffness_radius, transformation_matrix, ...?
(also typo: TranlationTranslation)

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.

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