Fix #107956: Simulation reset on fps change #108004

Merged

YimingWu merged 5 commits from ChengduLittleA/blender:fix-107956 into main

2023-05-31 10:06:05 +02:00

YimingWu commented

2023-05-17 10:56:17 +02:00

Member

Resets simulation cache when frame rate changes, so we don't get
inconsistent accumulations when frame rate is changed in the middle of
the simulation.

Resets simulation cache when frame rate changes, so we don't get inconsistent accumulations when frame rate is changed in the middle of the simulation.

YimingWu requested review from Jacques Lucke 2023-05-17 10:57:02 +02:00

YimingWu added the

Nodes & Physics

labels 2023-05-17 10:57:14 +02:00

YimingWu added this to the Nodes & Physics project 2023-05-17 10:57:24 +02:00

YimingWu changed title from ~~Fix #107956: Simulation reset on fps change.~~ to Fix #107956: Simulation reset on fps change

2023-05-17 10:59:07 +02:00

Iliya Katushenock commented

2023-05-17 13:14:28 +02:00

Member

Does it make sense to make the cache update part of the scene frame rate update callback?

Does it make sense to make the cache update part of the scene frame rate update callback?

YimingWu commented

2023-05-17 13:19:22 +02:00

Author

Member

Humm... That case we need to iterate every single object's modifier to clear the cache. It makes sense but I kinda prefer an on-demand kind of approach so if you change the value back it doesn't affect anything else that's not being evaluated.

Humm... That case we need to iterate every single object's modifier to clear the cache. It makes sense but I kinda prefer an on-demand kind of approach so if you change the value back it doesn't affect anything else that's not being evaluated.

Jacques Lucke requested changes 2023-05-23 18:43:34 +02:00

source/blender/blenkernel/BKE_simulation_state.hh Outdated

						
				@ -162,6 +162,8 @@ class ModifierSimulationCache {

				  CacheState cache_state_ = CacheState::Valid;

				  bool failed_finding_bake_ = false;

				  float delta_seconds_ = 0.0f;

Jacques Lucke commented

2023-05-23 18:43:23 +02:00

Member

That does not seem quite right, not sure why delta_seconds_ should be stored. It kind of conflicts with the future possibility to have adaptive subframes. Would likely be better to just store the fps here. Even better might to somehow hook the reset up to rna_Scene_fps_update but that can also be done later.

That does not seem quite right, not sure why `delta_seconds_` should be stored. It kind of conflicts with the future possibility to have adaptive subframes. Would likely be better to just store the fps here. Even better might to somehow hook the reset up to `rna_Scene_fps_update` but that can also be done later.

ChengduLittleA marked this conversation as resolved

YimingWu force-pushed fix-107956 from 673a9f8a42 to a702b908ea

2023-05-24 05:00:38 +02:00

Compare

Jacques Lucke requested changes 2023-05-24 08:39:53 +02:00

source/blender/modifiers/intern/MOD_nodes.cc Outdated

						
				@ -1187,16 +1201,12 @@ static void prepare_simulation_states_for_evaluation(const NodesModifierData &nm

				          }

				        }

				        else if (sim_states.prev != nullptr && sim_states.next == nullptr) {

				          const float max_delta_frames = 1.0f;

Jacques Lucke commented

2023-05-24 08:38:40 +02:00

Member

Is there any reason to move all this code?
Just put the fps check into the /* Reset cached data if necessary. */ section above.

Is there any reason to move all this code? Just put the fps check into the `/* Reset cached data if necessary. */` section above.

ChengduLittleA marked this conversation as resolved

source/blender/modifiers/intern/MOD_nodes.cc Outdated

						
				@ -1177,0 +1183,4 @@

				      const float delta_frames = std::min(max_delta_frames, scene_delta_frames);

				      const float delta_seconds = delta_frames / FPS;

				      const float old_fps = nmd_orig.simulation_cache->last_fps_;

				      if (old_fps && (!compare_ff(FPS, old_fps, 1e-8))) {

Jacques Lucke commented

2023-05-24 08:39:50 +02:00

Member

using 1e-8 when comparing floats is probably not very useful, because that's lower than the precision. An exact check would probably work here, but can also just use 1e-6 instead.

using `1e-8` when comparing floats is probably not very useful, because that's lower than the precision. An exact check would probably work here, but can also just use `1e-6` instead.

ChengduLittleA marked this conversation as resolved

YimingWu force-pushed fix-107956 from 48be8f32a0 to 7c3e83461e

2023-05-25 04:00:48 +02:00

Compare

YimingWu force-pushed fix-107956 from 7c3e83461e to 0cb730039c

2023-05-25 04:02:28 +02:00

Compare

YimingWu force-pushed fix-107956 from 0cb730039c to 529a240038

2023-05-25 04:07:41 +02:00

Compare

Jacques Lucke requested changes 2023-05-25 10:57:13 +02:00

Jacques Lucke left a comment

Member

Sorry for the unnecessarily long back and forth. Just noticed that we should probably just invalidate the cache when the fps changes. The reset then happens automatically when going back to frame one.

Also, why are you always force-pushing to your branch? It would make checking your changes easier if you would just make normal commits. You don't have to rebase your branch all the time.

Sorry for the unnecessarily long back and forth. Just noticed that we should probably just invalidate the cache when the fps changes. The reset then happens automatically when going back to frame one. Also, why are you always force-pushing to your branch? It would make checking your changes easier if you would just make normal commits. You don't have to rebase your branch all the time.

YimingWu commented

2023-05-25 11:42:59 +02:00

Author

Member

Hi @JacquesLucke , I rebase the branch so I compile faster because when switching from and to main there will be fewer file changes.

I'll figure out a way to make it into frame rate callback. :D

Hi @JacquesLucke , I rebase the branch so I compile faster because when switching from and to main there will be fewer file changes. I'll figure out a way to make it into frame rate callback. :D

YimingWu force-pushed fix-107956 from 529a240038 to 0510ab8960

2023-05-30 15:18:26 +02:00

Compare

Jacques Lucke approved these changes 2023-05-31 09:55:01 +02:00

source/blender/blenkernel/intern/simulation.cc Outdated

						
				@ -184,0 +194,4 @@

				        continue;

				      }

				      NodesModifierData *nmd = (NodesModifierData *)md;

				      NodesModifierData *nmd_orig = reinterpret_cast<NodesModifierData *>(

Jacques Lucke commented

2023-05-31 09:52:43 +02:00

Member

Please try it, but I'd assume that you can work on nmd directly instead of using nmd_orig.

Please try it, but I'd assume that you can work on `nmd` directly instead of using `nmd_orig`.

source/blender/makesrna/intern/rna_scene.c Outdated

						
				@ -922,6 +923,11 @@ static void rna_Scene_camera_update(Main *bmain, Scene *UNUSED(scene_unused), Po

				  DEG_relations_tag_update(bmain);

				}

				static void simulation_reset_cache(Scene *scene)

Jacques Lucke commented

2023-05-31 09:54:51 +02:00

Member

No need for this extra indirection, just call BKE_simulation_reset_scene directly below.

No need for this extra indirection, just call `BKE_simulation_reset_scene` directly below.

ChengduLittleA marked this conversation as resolved

YimingWu added 1 commit 2023-05-31 10:04:44 +02:00

4b1304bf3c Cleanup, use `nmd` directly.

YimingWu merged commit 397663a7cb into main

2023-05-31 10:06:05 +02:00

YimingWu deleted branch fix-107956

2023-05-31 10:06:06 +02:00

YimingWu referenced this issue from a commit

2023-05-31 10:06:08 +02:00

Fix #107956: Simulation reset on fps change

YimingWu referenced this pull request

2023-07-16 10:18:35 +02:00

Delta Time not used in Rigid Body Physics and strange behavior #110160

Sign in to join this conversation.

No reviewers

No Label

Animation & Rigging

Asset Browser Project Overview

Automated Testing

Blender Asset Bundle

Dependency Graph

Development Management

EEVEE & Viewport

Images & Movies

Motion Tracking

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Render Pipeline

Sculpt, Paint & Texture

Video Sequencer

Virtual Reality

Blender 2.8 Project

Milestone 1: Basic, Local Asset Browser

Good First Issue

Animation & Rigging

Development Management

EEVEE & Viewport

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Sculpt, Paint & Texture

Needs Info from Developers

Needs Information from User

No Milestone

No project

No Assignees

3 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#108004

No description provided.