Skip to content

some more optimizations... - #97

Closed
djreiss wants to merge 5 commits into
mrdoob:masterfrom
djreiss:master
Closed

some more optimizations...#97
djreiss wants to merge 5 commits into
mrdoob:masterfrom
djreiss:master

Conversation

@djreiss

@djreiss djreiss commented Jan 20, 2011

Copy link
Copy Markdown
Contributor

Getting diminishing returns here but I thought I should push my updates, anyway. THis will probably be my last update for a while ;)

Main change: moved Matrix4.m33 to WebGLRenderer._m33 and WebGLRenderer2._m33 since it is only used there and only used for a single matrix in renderObject().

Also, moved Matrix4.flat to a global Matrix4._flat -- seemed to work.

I think some of this code will not be thread safe if we the library eventually starts using web workers, but I'm just following along the lines of what alteredq has already done ;)

* memory and speed optimizations in Matrix4.makeInvert3x3() (moved Matrix4.m33 to WebGLRenderer._m33 since it is
  only used there and only for a single Matrix4)
…atrix4.makeInvert3x3() call accordingly, as with WebGLRenderer
@alteredq

Copy link
Copy Markdown
Contributor

I'm not sure moving out of individual matrices things that are being returned (as opposed to things that are just temp variables for computation) wouldn't make problems somewhere sometime.

For the moment, yes, these are just used at some well defined places, so we know it would be ok.

Web workers I think have their own separate scope and communicate with main thread just via messages, so there it shouldn't make problems.

What could make problems is if in some use case returns from several matrices would be used at once. Could be solved by cloning return values, but I think this would be easy to forget about.

@djreiss

djreiss commented Jan 21, 2011

Copy link
Copy Markdown
Contributor Author

OK, that is fine, and perfectly understandable. Another alternative is to add an m33 parameter to Matrix4.flatten(), and if none is passed, then allocate the matrix4's invidiual m33 object. Let me know if that would be an acceptable alternative and I can implement it.

But, in the meantime, I can re-do my edits, keeping the m33 class member in Matrix4.
Or, feel free to just accept the first three commits, not the last two ;)

@mrdoob

mrdoob commented Jan 23, 2011

Copy link
Copy Markdown
Owner

Done :D

@alteredq

Copy link
Copy Markdown
Contributor

Oops, meanwhile I just did the same :S

Or, to be more precise, I did more conservative merge - I didn't took in move of matrices into WebGLRenderer but reapplied optimization from these commits.

npmcomponent pushed a commit to npmcomponent/timoxley-threejs that referenced this pull request Jan 6, 2014
3jsLive pushed a commit to 3jsLive/three.js that referenced this pull request Feb 21, 2023
This pull request was closed.
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