Yellow-Dog-Man / Resonite-Issues

Issue repository for Resonite.
https://resonite.com
139 stars 2 forks source link

spinner and panner (and probably more) components react poorly to old sessions #1355

Closed art0007i closed 8 months ago

art0007i commented 8 months ago

Describe the bug?

in sessions which have been open for several days or longer some components such as spinners and panners will become choppy.

To Reproduce

join a session which has been open for a long time attach Spinner component observe the slot's rotation

Expected behavior

the spinner rotates smoothly

Screenshots

No response

Resonite Version Number

Beta 2024.2.12.1430

What Platforms does this occur on?

Windows

What headset if any do you use?

No response

Log Files

N/A

Additional Context

this can easily be fixed by doing calculations on double precision values and only downcast it to a float at the final step.

this doesn't solve the underlaying problem but it should be good enough as I have not seen a session open for long enough to notice precision issues on a double

Reporters

No response

shiftyscales commented 8 months ago

I don't think this is a bug- as you noted it's a precision issue caused by sessions that have been running for a prolonged period.

Typically headless owners will want to set their idleRestartInterval in their headless configuration file so the session periodically restarts/avoids these issues entirely.

BlueCyro commented 8 months ago

I'd say it's less of a bug and more of a side-effect of these components downcasting the world time into a float from a double. It's understandable because in most cases it'll look fine and it uses less resources to do a bunch of calculations on floats. However, I think it would be nice if these components had a sort of 'high-precision' mode.

In the meantime, you can use AuthorityTimeBase or the high-precision WorldTime nodes in protoflux to drive values that need longevity.

Frooxius commented 8 months ago

I think generally sessions shouldn't let be run that long and restart regularly.

Even if we add higher precision versions of those components, you'll run into precision issues in other cases and "fixing" or changing everything to doubles is really not suitable solution here, it'll become a bit of a "whack-a-mole problem". We solve this one - something else will pop-up elsewhere.

With some of the future plans too for optimizations and such, switching to doubles could have even more of an impact. Say we rework these components to use more parallelized and streamlined system for the computations, so you could have dozens of thousands of these - switch from float to double will start adding up a lot more there.

art0007i commented 8 months ago

I think generally sessions shouldn't let be run that long and restart regularly.

sounds reasonable, but is there any specific reason? is there any reason to restart regularly other than the downcasted world time losing precision?

Even if we add higher precision versions of those components, you'll run into precision issues in other cases and "fixing" or changing everything to doubles is really not suitable solution here, it'll become a bit of a "whack-a-mole problem". We solve this one - something else will pop-up elsewhere.

what do you mean by this exactly? that there are a ton of places which use WorldTime and instantly downcast it to a float? I've checked using a decompiler and found ~20 places which downcast worldTime to float like this. here's a list of all components which do this

Components FrooxEngine: - RadiantDash.get_TintColor -- could do modulo 1 on the time before casting it to float - TimeSineDriver.OnCommonUpdate -- could do the sin function on Double and then cast result to float - HandPoser.DebugCoordinateCompensation -- same as above - BillboardBrushTool.OnCommonUpdate -- same as above - BoxBrushTool.GetGlobalRotation -- same as above - BoxBrushTool.OnCommonUpdate -- same as above - QuadBrushTool.GetGlobalRotation -- same as above - QuadBrushTool.OnCommonUpdate -- same as above - GlueVisualizer.OnCommonUpdate -- same as above - ConvexHullBrushTool.OnCommonUpdate -- not sure exactly what this function does, but it does downcast the time early - InteractiveCamera.SimulateMovement -- downcasts early, but then passes the value into SimplexNoise and it seems that there isn't a double overload for simplex noise - Wiggler.get_Wiggle -- same as above - VRIKAvatar.UpdateAvatar -- same as above - EyeManager.OnCommonUpdate -- same as above - AppEnder.OnCommonUpdate -- probably not a problem in here, since this component only exists in the exiting world which usually does not last longer than a minute - Panner1,2,3,4D -- change "UnwrappedPosition" to double - Spinner.get_RawRotation -- use doubles before the modulo operator ProtoFlux: - WorldTimeHalfFloat.Compute -- could multiply in doubles then cast to float, but that's probably not necessary on protoflux since we have nodes to get time in a double anyway - WorldTimeTenthFloat.Compute -- same as above

With some of the future plans too for optimizations and such, switching to doubles could have even more of an impact. Say we rework these components to use more parallelized and streamlined system for the computations, so you could have dozens of thousands of these - switch from float to double will start adding up a lot more there.

I would hope that in the modern age we can use doubles without much performance concern. Most systems are 64-bit and have hardware acceleration for double precision operations.

In fact I found this resource which says that doing operation on doubles could even be faster than on floats somehow https://dev.to/maximsaplin/c-float-vs-double-performance-considerations-2o8k

TisFoolish commented 8 months ago

I think generally sessions shouldn't let be run that long and restart regularly.

But restarting daily increases downtime and on a platform where there are people trying to build small businesses having increased downtime is unoptimal. Right now the platform is small enough that there can be regular times during the day that a restart won't negatively effect anyone, but as the community grows this will eventually no longer hold true.

art0007i commented 8 months ago

why close this? I think I've made some valid points for adding this in my comment above.

knackrack615 commented 8 months ago

I second arti on this one, this issue is valid and should be re-opened. @shiftyscales