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

shift popMatrix, pushMatrix, printMatrix to legacy.js #3592

Merged
merged 1 commit into from
Mar 11, 2019
Merged

shift popMatrix, pushMatrix, printMatrix to legacy.js #3592

merged 1 commit into from
Mar 11, 2019

Conversation

Ajayneethikannan
Copy link
Contributor

Fixes: #3589

The changes made in this PR are :

  • shift pushMatrix, popMatrix, printMatrix methods to legacy.js
  • write a note about how to implement tracking of matrix for printMatrix method

I request the maintainers to review this PR,
Thanks !

@grege2
Copy link

grege2 commented Mar 11, 2019

Hi @aj_ryuk, thanks for this PR. I agree it's good to remove unneeded calls, but personally I would recommend not consigning printMatrix() to the dustbin. I think p5.js should actually fully implement printMatrix() and getMatrix(). They really are important tools to debug what's happening with your ModelView matrix. I've been using Iris GL and OpenGL for 30 years and you do need to have "introspection" of the internal state of your GL. If you're doing something complicated and stacking and backtracking transforms, like a "generative art" tree (ie. a physical plant !) you can easily make a mistake. Plus you can make a mistake with the simplest code, rotating something around the screen. A getMatrix() or at least printMatrix() call can help you debug. If you know that your matrix should be like these: Wikipedia rotate matrix and it's not, it's a valuable debugging clue.

Looking forward to other opinions.

Best, GE.

@Ajayneethikannan
Copy link
Contributor Author

Thank you very much for providing your thoughts on this issue @grege2 😃 Currently, as per discussions in #243 , the pushMatrix and popMatrix have not been implemented due to errors and performance issue of maintaining the transformation matrix by ourself, and consequently printMatrix.
I agree that it would be very beneficial for debugging to implement printMatrix, and in this PR, I am only updating the message to say how to implement it in an ad hoc way, by the users.
In case it is to be implemented , I am all up for it !
Would love to hear others opinions on this .
Thanks !

@lmccart lmccart merged commit 074afd0 into processing:master Mar 11, 2019
@grege2
Copy link

grege2 commented Mar 11, 2019

Hi @aj_ryuk. Thanks for the extra explanation. I read through #243. I can see that p5.js doesn't keep track of the matrix, at least not in the traditional way that an OpenGL implementation would, it's held by the Canvas, and it's hard to implement a getMatrix(). Ok, getMatrix()and printMatrix() will have to wait.

Ciao !

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.

3 participants