This works very well to resolve issues with FSR 1, 2.2, and TAA on Terrain3D. All three look good without any artifact issues. Motion vectors are sane. At least this is the case on my RTX 3070.
Thank you very much!

FSR 1 has no temporal components, it's purely spatial.

Given the now confusing naming, an option we could also consider longterm is adding an alias teleport() function instead of reset_physics_interpolation() (and perhaps deprecate the old function name but leave it available for compatibility).

teleport() was always my preference for terminology (and is used in smoothing addon).

Although that would be for separate proposal, as it has implications for docs etc.

Bear in mind also that this PR will need some rejigging after #104269, (or vice versa if this is merged first).

Given the now confusing naming, an option we could also consider longterm is adding an alias teleport() function instead of reset_physics_interpolation() (and perhaps deprecate the old function name but leave it available for compatibility).

I thought about that but decided it was out of scope of this PR since it would require me to propagate the teleport through the scene tree, something which right now reset_physics_interpolation() does for me.

I suppose an optional teleport parameter could be added to Node3D's exposed transform functions, similar to what this PR is doing internally right now.
Implementation wise the transform functions would just have to call reset_physics_interpolation() (maybe renamed to reset_interpolation()) after updating the transform if the new parameter is true.

teleport() was always my preference for terminology (and is used in smoothing addon).

Yeah, I also called the optional parameter "teleport" in this PR.

Bear in mind also that this PR will need some rejigging after #104269, (or vice versa if this is merged first).

Yeah. I considered building this PR on top of yours for a while, but in the end decided to stick to master. This is a significantly smaller change than your interpolation refactor, so I am hoping to get it merged quickly.

I thought about that but decided it was out of scope of this PR since it would require me to propagate the teleport through the scene tree, something which right now reset_physics_interpolation() does for me.

Agree is out of scope, but just to clarify my suggestion was to rename the existing reset_physics_interpolation() functionality to teleport(), rather than add in extra machinery.

I suppose an optional teleport parameter could be added to Node3D's exposed transform functions, similar to what this PR is doing internally right now.

We did consider this at the time, but a separate function was preferred in general (with the idea that it would rarely be used), and it had the benefit of being backward compatible. I tended to think this made sense from an efficiency point of view too, as set_transform() is used a lot, and there's no sense in sending an extra bool all the time if it isn't needed.

If you aren't rebasing on top of #104269, you can probably use the existing instance_reset_physics_interpolation() function, making a lot of the changes here unnecessary I think.

If this is the intention long term, I could remove the depreciation of instance_reset_physics_interpolation() in #104269 .. although I suspect it would be better to just create an equivalent server function instance_teleport(RID), something like that.

Actually it occurs to me there is one special scenario we may not have thought of (when used in conjunction with physics interpolation):

If a user has turned off physics interpolation for a branch, the resets may not be propagated currently. We may want to alter this so that the reset (for motion vectors) still gets sent to the server. This is something that can be fixed up after #104269 though.

To make this PR and Physics Interpolation work in Terrain3D, we are currently doing this:

	RS->instance_set_transform(mesh_array[instance], t, true); // added true for this PR
	RS->instance_reset_physics_interpolation(mesh_array[instance]);

They are both called right after we reposition the clipmap terrain. It would make sense to me to have one function call reset motion vectors for both PI and TAA/FSR. I think a name like reset_motion_vectors is clearer than teleport, but ultimately don't care as long as I have a clear, documented way to reset them, which has now been provided.

Although we don't always future proof, the problem with using specific names like reset_physics_interpolation and reset_motion_vectors is that it doesn't take account that typically several systems need to get reset on a teleport.

The user should ideally have one function to call that does it all.

Consider that e.g. spatial audio should also be interpolated eventually, just to give an example. Granted that's a different server, but this collision of functionality has already occurred within VisualServer, and it's a fair bet it may happen again.

Although we don't always future proof, the problem with using specific names like reset_physics_interpolation and reset_motion_vectors is that it doesn't take account that typically several systems need to get reset on a teleport.

If a user has turned off physics interpolation for a branch, the resets may not be propagated currently. We may want to alter this so that the reset (for motion vectors) still gets sent to the server. This is something that can be fixed up after #104269 though.

I am convinced that we will need a generic reset_interpolation at some point in the future. However, implementing that might be tricky. If this function is always called after the property that should be reset has already been changed I am no certain if we can guarantee that none of the other threads has already reacted to this change and interpolated it before receiving the reset command. Ahh, the joys of multithreading.

If you aren't rebasing on top of #104269, you can probably use the existing instance_reset_physics_interpolation() function, making a lot of the changes here unnecessary I think.

Honestly not really. Most of the changes here are the plumbing needed to expose the optional parameter, something I would have done anyways since being able to turn on teleportation while setting the transform is essential imo.

You are correct that if I had patched that function I could have left VisualInstance3D as it is, but since I already have the teleport parameter I might as well use it. Performance is the same and code readability is better when we are explicit about setting transform with teleport enabled.

However, just for completeness sake I will add teleportation to instance_reset_physics_interpolation(). It is not really needed since imo the proper way is to already enable teleportation when setting the transform, but as long as that method exists it should behave as users expect it to. Feel free to purge it in the future.

This PR was just discussed in the weekly rendering meeting. While the overall idea and design was approved, there was some argument about the exposed API.

As of right now, the PR exposes the following function:

RS::instance_set_transform_teleport(instance, global_transform)

However, the team pointed out that in the future we might wish to reset stuff other than the transform of mashes, and thus something along the lines of

RS::instance_set_transform(instance, global_transform)
RS::instance_reset_interpolation(instance)

might be more future proof.

However, we couldn't agree on what the new instance_reset_interpolation() function should actually be called. reset might imply that it completely resets an instance to it's initial state, while teleport means "to instantly move an object from one location in space to another", which imo is only an appropriate name as long as we are only resetting transforms.

I am open to suggestions for a better name. @lawnjelly do you have any suggestions?

FSR 1 will not produce MV issues because it doesn't use them (it's a purely Spatial Upscaler & Sharpener), so no need to spend your time testing FSR 1

I'm getting this error message now w/ FSR2 enabled.
Huh, that is interesting. I don't think I changed anything that should mess with that. Made some more minor changes, if you have some time I would appreciate it if you could try again, maybe I already fixed that.

If the issue still exists just report back and I will look into it some more once I am home. Looking at my PR on a phone I can't see how I could be causing this with the current version.

FSR 1 will not produce MV issues because it doesn't use them (it's a purely Spatial Upscaler & Sharpener), so no need to spend your time testing FSR 1

The quoted error was new and related to FSR. I was confirming that the error appeared only when FSR2 was enabled, and not in other modes: disabled, FSR1, etc. Nothing to do with MV in this case.

I don't think I changed anything that should mess with that.

I tested more and found:

• TAA & FSR2 are very bad w/o this PR
• They're solid with this PR
• The FSR2 error exists in master and 4.4.1 reported in FSR2 Crash and Error FFX_ERROR_INVALID_ARGUMENT #105509

Tested on my terrain implementation as well and all artifacts are gone, works like a charm!

Thanks!