Fix #106425: Mantaflow guiding with domains is broken #117067
No reviewers
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset System
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Asset Browser Project
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
5 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#117067
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "lichtwerk/blender:106425_b"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Caused by
e968b4197b
/67e23b4b29
Since culprit commits, we are not running
MANTA::initGuiding
/fluid_alloc_guiding
if guiding is meant to be pulled from otherdomains (it does work with effectors though). This is because atm. only
the
FLUID_DOMAIN_ACTIVE_GUIDE
flag setsmUsingGuiding
to true(activated in
update_obstacleflags
, so only for effectors).So to fix this, we have to ensure that
MANTA::initGuiding
runs (alsoif guiding is pulled from domains), but also make sure we dont run into
the crashes from T102257.
It seems that the real reason we were getting crashes in T102257 is that
67e23b4b29
made it so that resetting the cache (including a call tofluid_modifier_reset_ex
) influid_modifier_processDomain
happensafter
FluidDomainSettings.active_fields
have been updated (thishappens in
update_flowsflags
&update_obstacleflags
).But
fluid_modifier_reset_ex
also resetsFluidDomainSettings.active_fields
. If we then update pointers later influid_modifier_processDomain
, we never get anything in the guidingrelated pointers for the new
mCurrentID
(specificallymPhiGuideIn
will be a nullptr). This is the real reason
initGuiding()
runs asecond time (it does once for
fluid_modifier_init
then again as aconsequence of the call to
manta_guiding
).This patch proposes to move the resetting process from
67e23b4b29
above the refreshing of the
FluidDomainSettings.active_fields
,this way these field stay intact, we do get the first run of
initGuiding()
fromfluid_modifier_init
, manta pointers get updatedwith intact fields afterwards (so we then get a valid
mPhiGuideIn
),which then prevents the second run from
manta_guiding
to actually callthe python script again.
The fix from
e968b4197b
is then not needed and reverted in this patch.This should be good for LTS I think.
Wouldnt have spent time on this if this was in @sebbas hands, but since I noticed this was caused/reviewed by still active devs, I gave it a shot [and it is basically a pretty bad regression I think].
@Sergey @ZedDB , you reviewed https://archive.blender.org/developer/D15210, https://archive.blender.org/developer/D16416 apparently did not have review at all
I need to re-study how this Guides thing works.
If I understand correctly, it seems like there are two types of Guides.
Apparently,
e968b4197b
broke the second type of "Guide", because even withfds->flags
having the flagFLUID_DOMAIN_USE_GUIDE
, the script that callsMANTA::initGuiding
orfluid_alloc_guiding
was not being generated.This caused the python error presented in #106425 and the consequent failure of the Guide.
So the first solution then is to revert
e968b4197b
to check theFLUID_DOMAIN_USE_GUIDE
flag again.But this brought back the #102257 crash, caused by "Guide" type Effectors.
The real problem in #102257 was that the functions
update_flowsflags
,update_obstacleflags
,fluid_modifier_reset_ex
andfluid_modifier_init
were being called in the wrong order.fluid_modifier_reset_ex
needs to be called beforeupdate_flowsflags
,update_obstacleflags
andfluid_modifier_init
asFluidDomainSettings.active_fields
is reset influid_modifier_reset_ex
.So the correct order is
fluid_modifier_reset_ex
,update_flowsflags
,update_obstacleflags
andfluid_modifier_init
, right?This prevent
initGuiding()
from being called twice in #102257 becausemanta_guiding
(called influid_modifier_init
) first checks whetherFluidDomainSettings.active_fields
are valid?If that's the case, the solution seems ok.
One last question that might be worth considering (I don't remember the details) but why is the
FLUID_DOMAIN_ACTIVE_GUIDE
flag no longer checked?Typing this on the mobile, but:
This is my understanding, yes
from the PR description:
So it is the fact we first dont have a valid
mPhiGuideIn
in the first run, on the second run we have.Because that was only the workaround from
e968b4197b
which we dont actually need / can revert?My last question was unclear, it was more of a code question to better understand what is happening (since I don't remember the details). In other words the question is: Why is
FLUID_DOMAIN_ACTIVE_GUIDE
not necessary when we useFLUID_DOMAIN_USE_GUIDE
? However, it doesn't seem like something important to be investigated.There are other details in the code that leave me lost like: What is this
mPhiGuideIn
? How is it invalidated?Either way, it's certainly more correct now :) (Nice work)
(Maybe it's worth adding a comment or an assert just to avoid this error? It appears to be fragile code).
(From my side it is approved).
Sounds promising, is it likely to be in Blender v4.0 and/or v4.1? or just v3.6 LTS!?
Should be in 4.1 and LTS, 4.0 is not getting another update
FLUID_DOMAIN_ACTIVE_GUIDE
is only activated inupdate_obstacleflags
(so only for effectors).If we always check that, guiding by domains will be ignored.
MANTA::updatePointers
be glad to try it out over the coming days - Thanks to Philipp and everyone involved!
FYI: this wont be in todays LTS updates (so only in 4.1 for now), should be in next months LTS releases (if no issues come up with it in 4.1)
i will definitely check it out shortly (4.1) and report anything! thanks all happy new year to you
I can confirm the Domain guide is working and seems stable!
I have noticed that domain does not show the guided smoke in viewport solid mode when created from the fluid domain, it does show up in rendered mode with principled volume added though.
still more testing to do, but that's the only things i have found so far. Fluid -> Smoke
Thx for testing!
Cant repro in my simple test, just to make sure we are all on the same page: could you create a report for this (including the .blend that you used)?
Hello,
The style of these two pictures displayed in rendering mode
I think it's correct to display it like smoke
@lichen : same for you: if you believe you have found a bug/issue, please open a bugreport for it, feel free to ping me there.
It is not clear if these issues are bugs and/or related to this pull request at all...