Details

Reviewers

Brecht Van Lommel (brecht)
Sergey Sharybin (sergey)
Nils Thuerey (n_t)
Georg Kohl (GeorgKohl)

Maniphest Tasks

T59995: Mantaflow Review

Commits

rCdd54bf816d45: Mantaflow [Part 6]: Updates in /blender/source
rBd27ccf990c2b: Mantaflow [Part 6]: Updates in /blender/source
rB9d40d0ad2e03: Mantaflow: Smaller update for draw engine code
rB305bedc9d11f: Merge branch 'internUpdate' into sourceUpdate
rBe4cd623fefd4: Merge branch 'internUpdate' into sourceUpdate
rBf3fa0fb9b175: Mantaflow: Updates in /blender/source

Summary

A collection of smaller changes that are required in the /blender/source files. A lot of them are also due to variable renaming.

Diff Detail

Repository: rB Blender

Event Timeline

Sebastián Barschkis (sebbas) created this revision.Oct 29 2018, 5:29 PM

Martin Felke (scorpion81) added a subscriber: Martin Felke (scorpion81).Nov 25 2018, 5:34 PM

Jérôme Matthey (JeromeM) added a subscriber: Jérôme Matthey (JeromeM).Dec 28 2018, 11:29 PM

Bastien Montagne (mont29) added a task: T59995: Mantaflow Review.Dec 30 2018, 10:07 PM

Update manta files after merge with master.

Harbormaster completed remote builds in B3296: Diff 14647.Apr 6 2019, 2:22 PM

Harbormaster completed remote builds in B3296: Diff 14647.

In general have the feeling naming is a bit fuzzy again, some defines/enums/operators are using MANTA prefix,(which is a good thing), but some others still use FLUID (which is too vague and generic), and modifiers are still even using Smoke in their names…

Also, I understand that new Manta shares most of its data structure with old smoke modifier, but still, not sure it's a good idea to reuse that one (and modifier type etc. as well)… @Campbell Barton (campbellbarton) do you think we could use new DNA renaming system here?

source/blender/blenkernel/intern/pointcache.c
1738–1744	Juts remove it then (also same below).
source/blender/makesdna/DNA_particle_types.h
434	That one should probably be removed?
source/blender/python/intern/bpy_app_build_options.c
29–30	should be removed?
source/blender/python/intern/bpy_interface.c
205–208	This looks like a fix? Should be committed to master directly, then…

Regarding naming I disagree. We should not use the "mantaflow" name in the user interface or operator names much. For users that's an implementation detail, ideally we just have one fluid and smoke simulation system. Internally it's fine.

Naming concerns are for code names, not UI ones (am totally fine with generic Fluid one in UI, also there we still have some sneaking smoke too). In code, I believe keeping the underlying lib in names makes sense, it helps in case we have several systems enabled at the same time, and also when migrating from one to another (since it makes things more clear in patches etc.). This is roughly a C equivalent to the namespace concept in C++, imho that is quiet desired.

Modifiers id_name is a bit of a gray area here, but it's theoretically code info (user is supposed to see their labels, not their id_name, usually). So I’d still rather have the manta info in those.

Even for operators, I prefer the UI and operator names to match whenever possible. Just to make it easier for scripting, logs, and setting up keymaps where these names do get exposed to non-technical users.

Updated diff with latest changes from fluid-mantaflow branch

Harbormaster completed remote builds in B4648: Diff 17628.Aug 28 2019, 8:27 PM

Updated diff with latest changes from fluid-mantaflow branch

Harbormaster completed remote builds in B5839: Diff 19944.Nov 28 2019, 6:20 PM

I agree with @Brecht Van Lommel (brecht) here. Seems the naming was not addressed here?

source/blender/editors/interface/interface_templates.c
1822–1829	Store `((ParticleSystemModifierData *)md)->psys->part->type` in a temporary variable. No need to copy-paste it in all the lines.
source/blender/makesdna/DNA_particle_types.h
435–440	Use anonymous enum, allowing to access values from `gdb`.
442–450	`PARTICLE_TYPE_FOO`, not `POO`.

This revision now requires changes to proceed.Dec 10 2019, 3:02 PM

Sergey Sharybin (sergey) mentioned this in D3858: Mantaflow [Part 9]: Updated rna smoke code.Dec 10 2019, 3:28 PM

Sebastián Barschkis (sebbas) mentioned this in D3854: Mantaflow [Part 5]: Update other /intern packages.Dec 10 2019, 9:23 PM

See 881b90b9229f for changes.

Sergey Sharybin (sergey) accepted this revision.Dec 11 2019, 2:48 PM

This revision is now accepted and ready to land.Dec 11 2019, 2:48 PM

Updated diff with latest changes from fluid-mantaflow branch

Harbormaster completed remote builds in B5996: Diff 20257.Dec 13 2019, 12:07 AM

Couldn't we move away from manta name for filenames and types?

eModifierType_Manta -> eModifierType_Fluid
BKE_manta.h -> BKE_fluid.h
DNA_manta_types.h -> DNA_fluid_types.h
WITH_MOD_MANTA -> WITH_MOD_FLUID

Campbell Barton (campbellbarton) added inline comments.Dec 13 2019, 11:47 AM

source/blender/blenkernel/intern/modifier.c
159	Why is this needed? (should be commented).

Updated diff with latest changes from fluid-mantaflow branch

Harbormaster completed remote builds in B6023: Diff 20308.Dec 16 2019, 4:08 PM

@Campbell Barton (campbellbarton) Yes, renaming is a good idea. I also got rid of the null check in the updated patch.

Closed by commit rBd27ccf990c2b: Mantaflow [Part 6]: Updates in /blender/source (authored by Sebastián Barschkis (sebbas)). · Explain WhyDec 16 2019, 4:37 PM

This revision was automatically updated to reflect the committed changes.

Sebastián Barschkis (sebbas) added a commit: rBd27ccf990c2b: Mantaflow [Part 6]: Updates in /blender/source.

Nathan Letwory (jesterking) added a commit: rCdd54bf816d45: Mantaflow [Part 6]: Updates in /blender/source.Jan 13 2020, 11:30 PM

Mantaflow [Part 6]: Updates in /blender/sourceClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 20321

extern/CMakeLists.txt

extern/mantaflow/CMakeLists.txt

intern/CMakeLists.txt

intern/cycles/blender/blender_mesh.cpp

intern/cycles/blender/blender_session.cpp

intern/cycles/blender/blender_util.h

release/datafiles/blender_icons16/icon16_mod_fluid.dat

release/datafiles/blender_icons16/icon16_mod_smoke.dat

release/datafiles/blender_icons32/icon32_mod_fluid.dat

release/datafiles/blender_icons32/icon32_mod_smoke.dat

release/datafiles/locale

source/blender/CMakeLists.txt

source/blender/alembic/intern/abc_exporter.cc

source/blender/blenkernel/BKE_fluid.h

source/blender/blenkernel/BKE_fluidsim.h

source/blender/blenkernel/BKE_pointcache.h

source/blender/blenkernel/BKE_smoke.h

source/blender/blenkernel/CMakeLists.txt

source/blender/blenkernel/intern/bpath.c

source/blender/blenkernel/intern/dynamicpaint.c

source/blender/blenkernel/intern/effect.c

source/blender/blenkernel/intern/fluidsim.c

source/blender/blenkernel/intern/modifier.c

source/blender/blenkernel/intern/object.c

source/blender/blenkernel/intern/particle.c

source/blender/blenkernel/intern/pointcache.c

source/blender/blenkernel/intern/smoke.c

source/blender/blenloader/intern/readfile.c

source/blender/blenloader/intern/versioning_250.c

source/blender/blenloader/intern/versioning_260.c

source/blender/blenloader/intern/versioning_270.c

source/blender/blenloader/intern/versioning_legacy.c

source/blender/blenloader/intern/writefile.c

source/blender/depsgraph/intern/depsgraph_physics.cc

source/blender/draw/engines/eevee/eevee_volumes.c

source/blender/draw/engines/overlay/overlay_extra.c

source/blender/draw/engines/overlay/shaders/volume_velocity_vert.glsl

source/blender/draw/engines/workbench/workbench_deferred.c

source/blender/draw/engines/workbench/workbench_forward.c

source/blender/draw/engines/workbench/workbench_volume.c

source/blender/editors/datafiles/CMakeLists.txt

source/blender/editors/include/UI_icons.h

source/blender/editors/interface/interface_templates.c

source/blender/editors/physics/CMakeLists.txt

source/blender/editors/physics/particle_object.c

source/blender/editors/physics/physics_intern.h

source/blender/editors/physics/physics_ops.c

source/blender/editors/space_buttons/buttons_context.c

source/blender/editors/space_outliner/outliner_draw.c

source/blender/editors/space_view3d/CMakeLists.txt

source/blender/gpencil_modifiers/CMakeLists.txt

source/blender/gpu/CMakeLists.txt

source/blender/gpu/GPU_draw.h

source/blender/gpu/intern/gpu_draw.c

source/blender/makesdna/DNA_modifier_types.h

source/blender/makesdna/DNA_object_types.h

source/blender/makesdna/DNA_particle_types.h

source/blender/makesdna/DNA_smoke_types.h

source/blender/makesdna/intern/makesdna.c

source/blender/makesrna/RNA_access.h

source/blender/makesrna/intern/CMakeLists.txt

source/blender/makesrna/intern/makesrna.c

source/blender/makesrna/intern/rna_fluidsim.c

source/blender/makesrna/intern/rna_gpencil_modifier.c

source/blender/makesrna/intern/rna_internal.h

source/blender/makesrna/intern/rna_modifier.c

source/blender/makesrna/intern/rna_object_force.c

source/blender/makesrna/intern/rna_particle.c

source/blender/makesrna/intern/rna_smoke.c

source/blender/modifiers/CMakeLists.txt

source/blender/modifiers/MOD_modifiertypes.h

source/blender/modifiers/intern/MOD_fluid.c

source/blender/modifiers/intern/MOD_fluidsim.c

source/blender/modifiers/intern/MOD_fluidsim_util.h

Mantaflow [Part 6]: Updates in /blender/source
ClosedPublic
Actions

Revision Contents
Changeset List