Skip to content

Added Bezier curve and showed it in UtilsDemo. #34

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

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

Ne1gh-RR
Copy link
Collaborator

@Ne1gh-RR Ne1gh-RR commented Jun 3, 2025

No description provided.

@Ne1gh-RR Ne1gh-RR requested review from Martenfur June 3, 2025 13:22

namespace Monofoxe.Engine.Drawing
{
public class BezierCurve
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be static

{
public class BezierCurve
{
public static void Draw(Vector2[] controlPoints, float interval = 0.01f)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add the autodoc comment describing what the method does and what the arguments are for.


for (var i = 0; i < points.Length - 1; i += 1)
{
LineShape.Draw(points[i], points[i + 1]);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is inefficient, you should be pushing vertices to the vertexbatch like LineShape.Draw() does.


namespace Monofoxe.Engine.Drawing
{
public class BezierCurve
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename to BezierCurveShape to keep consistency with other such classes.


#region Bezier curve stuff.
// A look up table for factorials. Capped to 16.
private static float[] Factorial = new float[]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a private field, the naming should be _factorial

float uu = u * u;
int N = controlPoints.Length - 1;

if (N > 16)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should also add a check for less than 3 points.

/// <summary>
/// Returns an array of points spaced with passed interval.
/// </summary>
public static Vector2[] BezierCurvePoints(Vector2[] controlPoints, float interval = 0.01f)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this method, it's not really needed.

/// </summary>
/// <param name="value">Should be in 0..1 range.</param>
public static Vector2 BezierCurve(Vector2 startPoint, Vector2 controlPoint, Vector2 endPoint, float value)
public static Vector2 BezierCurve(Vector2[] controlPoints, float value)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Return three-point BezierCurve, it can exist alongside the n-point bezier.

		public static Vector2 BezierCurve(float value, params Vector2[] points)
		{ 
// ...
		}

		public static Vector2 BezierCurve(float value, Vector2 startPoint, Vector2 controlPoint, Vector2 endPoint)
		{
// ...
		}

@@ -118,6 +118,18 @@ public UtilsDemo(Layer layer) : base(layer)
_stateMachine.AddState(TestStates.Blue, Blue);
_stateMachine.AddState(TestStates.Red, Red);
// State machine.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add three-point bezier demo and add a demo that calls GameMath.BezierCurve directly.

@@ -56,7 +56,7 @@ public class UtilsDemo : Entity

Sprite _fireSprite;


Vector2[] _bezierCurvePoints;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private Vector2[] _bezierCurvePoints;

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

Successfully merging this pull request may close these issues.

2 participants