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

[dev-2.0] p5.Matrix and p5.Vector docs update #7637

Merged
merged 35 commits into from
Apr 8, 2025

Conversation

holomorfo
Copy link

@holomorfo holomorfo commented Mar 16, 2025

Resolves #[Add issue number here]

Changes:
Add documentation for p5.Matix

Screenshots of the change:

Screenshot 2025-03-24 at 11 11 24 PM

PR Checklist

@holomorfo holomorfo changed the title (Draft) Matrix docs update [dev-2.0] p5.Matrix docs update Mar 17, 2025
@holomorfo holomorfo marked this pull request as ready for review March 17, 2025 04:31
@holomorfo holomorfo changed the title [dev-2.0] p5.Matrix docs update [dev-2.0] p5.Matrix and p5.Vector docs update Mar 25, 2025
* const result = matrix.multiplyPoint(vector);
*
* // p5.js script example
* <div><code>
Copy link
Contributor

Choose a reason for hiding this comment

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

For ones like this that don't show anything on canvas, we can do <div class="norender"> to avoid having an empty canvas.

Copy link
Author

Choose a reason for hiding this comment

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

Hi Dave, I have updated the examples whit the norender to the scripts that need it, thanks.

* @param {Array} [mat4] column-major array literal of our 4×4 matrix
* @example
* // Creating a 3x3 matrix from an array using column major arrangement
* const matrix = new Matrix([1, 2, 3, 4, 5, 6, 7, 8, 9]);
Copy link
Contributor

Choose a reason for hiding this comment

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

For the sake of readability, maybe we should only show the p5 versions for now unless we can also show how to import these? What do you think @ksen0 and @limzykenneth?

Copy link
Member

Choose a reason for hiding this comment

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

For consistency sake, reference probably should only use the p5.Matrix (and other classes) version of the contructor. I don't think this will change for the reference in the future even when separate importing is documented just to keep things simple.

Copy link
Author

Choose a reason for hiding this comment

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

I agree, I have updated it now for both p5.Matrix and p5.Vector, thanks

@holomorfo
Copy link
Author

HI @davepagurek and @limzykenneth I have updated the PR with the changes, can you please review?

* rotate our Matrix around an axis by the given angle.
* @param {Number} a The angle of rotation in radians
* @param {p5.Vector|Number[]} axis the axis(es) to rotate around
* Rotate the Matrix around a specified axis by a given angle.
Copy link
Contributor

Choose a reason for hiding this comment

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

why we don't have any sketch related to rotate4X4() function? If possible can we add them?

Copy link
Author

Choose a reason for hiding this comment

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

I had missed to add it, I have updated now, thanks :)

@perminder-17
Copy link
Contributor

Hi @holomorfo @davepagurek , in the example of translate() when we write this sketch

function setup() {

  const matrix = new p5.Matrix(4); // Create a 4x4 identity matrix
  console.log("Original Matrix:", matrix.matrix);

  // Translate the matrix by a 3D vector
  matrix.translate([10, 20, 30]);
  console.log("After 3D Translation (10, 20, 30):", matrix.matrix);

  // Translate the matrix by a 2D vector
  matrix.translate([5, 15]);
  console.log("After 2D Translation (5, 15):", matrix.matrix);
}

what should I get in the console for After 3D Translation (10, 20, 30) and After 2D translation (5,15). I am actually getting the same matrix as the original one? Is there's something wrong internally? or maybe I am getting confused?

@davepagurek
Copy link
Contributor

@perminder-17 I think for performance reasons, when you log an object to the console, it doesn't make a deep copy of the current state of the object, it just holds on to a reference to it. Since at both logs, it's logging the same object, it means the console will be showing the same thing.

Maybe we need to be making a copy of the state of the object to log, e.g.: console.log(matrix.mat4.slice()). That way, we're making a copy of mat4 at its current state. Then when it gets modified later, the copy that we logged is unaffected.

@perminder-17
Copy link
Contributor

Hi @holomorfo I have reviewed the docs, feel free to take them. Actually I have read all the functions you documented and it looks really awesome. I haven't really studied much about matrix after my school but after looking at the example and docs it really seems simple, and easy to understand. So thanks for your work on this. Examples and docs are really nice, simple and concise.

@holomorfo
Copy link
Author

Hi @holomorfo I have reviewed the docs, feel free to take them. Actually I have read all the functions you documented and it looks really awesome. I haven't really studied much about matrix after my school but after looking at the example and docs it really seems simple, and easy to understand. So thanks for your work on this. Examples and docs are really nice, simple and concise.

Hi @perminder-17 these are great! I'll get to work on them, thank you for the notes and detailed review :)

@holomorfo
Copy link
Author

Hi team, I have updated all the comments and this PR is ready for merging, can you please check @davepagurek @limzykenneth
CC: @perminder-17 @GregStanton

@ksen0 ksen0 merged commit 11c18cb into processing:dev-2.0 Apr 8, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants