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

Unecessary dependancy in draw.js + matrix #68

Open
Incubatio opened this issue Feb 17, 2013 · 1 comment
Open

Unecessary dependancy in draw.js + matrix #68

Incubatio opened this issue Feb 17, 2013 · 1 comment

Comments

@Incubatio
Copy link

The following code in draw.js

// line 653 of gamejs.js
var m = matrix.translate(matrix.identity(), rDest.left, rDest.top);
m = matrix.multiply(m, src._matrix);
this.context.transform(m[0], m[1], m[2], m[3], m[4], m[5]);

add an unecessary dependancy in code, and could be replaced by:

this.context.translate(rDest.left, rDest.top);

Every example seem to still work, but some unit test (not all of them) of rotation and scale seem to break, maybe some refactoring will be needed with transform.js.

Talking about matrix, if such module exists it should not be so coupled to the transformation matrix (add and multiply ,method only work with one dimension matrix with a definite number of arguments which just break reusability of the module), but moved where specific transform are made.

@oberhamsi
Copy link
Member

regarding draw.js: it could probably be simplified to what you say but not on this.context but on the src context.. I would have to try. This was tricky code and some of the corner cases only show themselves when you do rotation & translation in one go (and there's probably no unit tests for these)

it's true that the matrix module only works for 2d transformations.. but then again that's what most gamejs games do :) maybe we could name it transform matrix or similar. or at least point that out in the docs.

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

No branches or pull requests

2 participants