Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new methods to AABB and Rect2 classes #81096

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mateuseap
Copy link
Contributor

@mateuseap mateuseap commented Aug 28, 2023

What I did

  • Added the following methods to AABB and Rect2 classes: lerp(), cubic_interpolate() and cubic_interpolate_in_time();
  • Modified the source code of following methods of Animation class: _cubic_interpolate_in_time() and interpolate_variant().

Closes: #80627

@AThousandShips
Copy link
Member

In the future please be aware of when people have already said they wanted to solve an issue like this case, first ask if they are still working on it, otherwise you might cost someone a lot of lost time when you grab the issue

@AThousandShips
Copy link
Member

This doesn't solve the linked issue unless you also integrate these methods with the animation

@mateuseap
Copy link
Contributor Author

In the future please be aware of when people have already said they wanted to solve an issue like this case, first ask if they are still working on it, otherwise you might cost someone a lot of lost time when you grab the issue

Alright, I'll pay more attention next time. For sure isn't nice to waste someone's time!

@mateuseap
Copy link
Contributor Author

This doesn't solve the linked issue unless you also integrate these methods with the animation

@AThousandShips can you please give me more info of how can I do it?

@AThousandShips
Copy link
Member

I'll try find the relevant code later today

@mateuseap
Copy link
Contributor Author

Thanks, really appreciate it! I'll come back to this later as well, I'll keep looking further into the code trying to understand what else is needed to implement/modify.

@AThousandShips
Copy link
Member

AThousandShips commented Aug 29, 2023

I'd suggest looking at scene/resources/animation.cpp, look at lines:

  • 2357
  • 2396
  • 5664

Also might be good to look into adding the same to AABB for completeness

@TokageItLab
Copy link
Member

TokageItLab commented Aug 29, 2023

The missing function is not the only problem, but the substance of the problem is the need to force a positive value with abs() at the end of the interpolation function.

@AThousandShips AThousandShips requested a review from a team August 31, 2023 17:02
@mateuseap
Copy link
Contributor Author

@AThousandShips @TokageItLab I've implemented the following methods for AABB and Rect2 classes: lerp(), cubic_interpolate() and cubic_interpolate_in_time(). Also I started using these new methods in the source code of the following Animation class methods: _cubic_interpolate_in_time() and interpolate_variant().

@mateuseap mateuseap changed the title Add lerp() and cubic_interpolate() methods to Rect2 class Add lerp() and cubic_interpolate() methods to Rect2 and AABB classes Aug 31, 2023
@mateuseap mateuseap changed the title Add lerp() and cubic_interpolate() methods to Rect2 and AABB classes Add new methods to Rect2 and AABB classes Aug 31, 2023
core/math/aabb.h Outdated Show resolved Hide resolved
@mateuseap mateuseap changed the title Add new methods to Rect2 and AABB classes Add new methods to AABB and Rect2 classes Sep 12, 2023
@mateuseap mateuseap force-pushed the feat/rect2 branch 3 times, most recently from 04d1f21 to 7b3a10c Compare September 12, 2023 11:08
@AThousandShips
Copy link
Member

Please use clang-format locally instead of depending on the CI status to check your code style

@mateuseap
Copy link
Contributor Author

@TokageItLab can you take a look in this PR? I've made the requested changes

core/math/aabb.h Outdated Show resolved Hide resolved
core/math/aabb.h Outdated Show resolved Hide resolved
core/math/aabb.h Outdated Show resolved Hide resolved
core/math/rect2.h Outdated Show resolved Hide resolved
core/math/rect2.h Outdated Show resolved Hide resolved
core/math/rect2.h Outdated Show resolved Hide resolved
- Add lerp(), cubic_interpolate() and cubic_interpolate_in_time() methods to AABB and Rect2 classes.
- Modify the animation.cpp file.
Add new methods to AABB and Rect2 classes

- Add lerp(), cubic_interpolate() and cubic_interpolate_in_time() methods to AABB and Rect2 classes.
- Modify the animation.cpp file.
@mateuseap
Copy link
Contributor Author

@AThousandShips I've modified what you've pointed out. I've also solved the conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants