Provide x- and y-velocities in drawInternal() Confetto method

[magneticflux-]

These are not calculated because you already use twice-integrated equations of motion based on the current time, acceleration, and initial conditions, but they could be similar fields to currentX and currentY. I'd be happy to submit a PR implementing the feature!

As a side note, the comments in the computeDistance function talk about linear acceleration, when you are clearly dealing with constant acceleration. It would be possible to implement linear acceleration, but I don't see any use cases for it.

As a side note to the side note, If you were to go down that path, you could use generalized mathematical functions from Apache Math to take derivatives, etc.

[jinatonic]

Sorry for the late reply! What would be the use case for knowing velocity inside drawInternal? The idea is that drawInternal will draw the confetto at that moment in time so velocity/acceleration values are not necessary there.

[magneticflux-]

@jinatonic The use case that I had in mind was for implementing a motion-blur effect that depended on the velocity. I approximated it by remembering the previous x and y values and calculating a displacement, but having the true velocity would let me be one time-step ahead and more precise. Other uses could include a color-based speed indicator, but my main one was for stretching rain particles based on speed to improve the apparent smoothness of the animation.

[jinatonic]

I see your point. I personally think that it doesn't quite belong in the library as it's not very applicable to the general use-case. I would trade ease of use and understandability over customizability in this case as I don't want to pollute the arguments of drawInternal with these various states that might be confusing to others.

I think this is probably best done via a fork of the library so you can customize it to your needs. I appreciate the your work in the PR though!

[magneticflux-]

@jinatonic Would you be open to me attempting this again in a cleaner and more performant fashion? I worked around it before, but a new downstream issue https://github.com/MatteoBattilana/WeatherView/issues/18 would be solved by having this information in a clean way.

[jinatonic]

Can you explain the issue a little bit more in detail? I'm not quite sure the problem you are describing with motion blur effect. Also what are your proposed changes at a high level before you dive into implementing it?

[magneticflux-]

Well, in the downstream motion blur issue, the motion blur was dependent on the distance the particle had covered since the last draw call. This worked fine mostly, but had issues when the time between draw calls wasn't "typical", as could happen during lag or when manually calling the draw method.

My proposed change was just to store currentVelocityX alongside currentX in the base Confetto class, and then pass them into the draw methods. Since Java doesn't have the ability to return multiple values from a method simultaneously, I rewrote the computeDistance method to take in a reusable 2-element float array to store the results of x and velocityX.

The change in method signature would mean it wouldn't be backwards-compatible so the version number would have to be bumped, but existing users wouldn't need to change anything in their method body.

[jinatonic]

I see. I still maintain my stance from before in that this feels like a niche use-case that might be better done via a fork of the project, it makes the public API more complicated for specific use cases.

[magneticflux-]

Okay, here's my last suggestion: What about simply making some protected float currentVelocityX; fields and keeping the method signature of the draw and drawInternal methods ? That would mean no compatibility issues, only internal modifications, and only users that actively seek out the feature would find it so there would be no detriment to the immediate ease of use.

[jinatonic]

yeah I'm ok with keeping current velocities as state variables in Confetto. Do you want to submit PR, or I can potentially do it later this week.

[magneticflux-]

Fantastic! I'll submit a PR soon with just those changes.