Non-Mutating Vectors #731
Non-Mutating Vectors #731
Conversation
- Removes temporary vectors. All vectors are permanent now. The temporary constructors still exist for compatibility, but they create permanent vectors. Also removes lovr.math.drain. - All vector methods (except :set) return new vectors instead of mutating. This makes some methods vestigial/weird, like :add/sub/mul/div and matrix "initializers" like :identity/:fov/:perspective/:ortho/:lookAt/:target/:reflect.
|
First, thank you again for providing this excellent code base. I've identified three primary use cases for LOVR, each with its own specific needs: Rapid Prototyping and Fun Experiments: The API should prioritize user-friendliness and accessibility. The goal is to enable quick and easy experimentation without the need for deep technical understanding. Community Contributions and Packages: This aspect involves creating libraries that simplify or empower the first use case. Scriptable Rendering Pipeline for Advanced Projects: In scenarios like mine, where the core simulation would be handled in C/C++, the primary requirement would be to interface with math types without overhead. Since I am very new to LOVR, I cannot really estimate the impact of the change. I understand making a decision about the proposed pull request involves Is cost of change low because framework user activity is low? What mainly drives your decision making on where the framework is heading? |
|
Hey, thanks for the thoughts. I think the assessment about the use cases is accurate. LÖVR aims to be:
As far as deciding on how to make a breaking change, it's really difficult! Since #169 was opened a few months after vectors got added, there has been consistent feedback from people that LÖVR's vector API is confusing because the methods mutate instead of returning new vectors. Separately, people seem to trip up on the 2 different types of vectors: permanent vs. temporary. Temporary vectors also use a pretty nasty hack involving the global lightuserdata metatable that I've never been super proud of. So there have been a lot of people asking for this change, and good reasons to do it. On the other hand, some people liked vectors the way they were and felt that the mutating API, temporary vector pool, etc. were important for performance. Sure, you have to copy vectors explicitly when you need a new one, but this control lets you really cut down on the number of objects you create and reduces GC impact. This branch was an attempt to experiment with switching to a non-mutating, easier API like people have wanted and A) try it out to see how it "feels" to use and B) measure performance. Initially there were some pretty big performance regressions, but these were solved (without adding back temporary vectors). Regarding performance, my conclusions from recent testing are:
Based on this, I'm not sure that the temporary vector system and mutating API was hitting the right tradeoff! It made things more confusing for everyone and although the API reduced the amount of garbage created (when used well), in real projects this wasn't impacting performance all that much and the user friendliness of the API was more of a bottleneck. During the development of the initial vector API, I was a little bit too cautious about garbage collection. And finally, regarding the breaking change part: Although LÖVR isn't at 1.0 yet, and there isn't a huge number of people using it, LÖVR does try to avoid making changes like this. In recent versions features have been deprecated instead of removed, and I plan on doing this moving forward whenever it's sane to do. We are still arguably recovering from the graphics rework in v0.16.0 when LÖVR switched to Vulkan, and making everyone rewrite their vector code is kinda just salt in the wounds. I wouldn't say the cost of making this change is low, at all. However, people have also pointed out that making this change earlier is better than waiting until later (when there's even more code to port), and it does fix something that people have been criticizing for a few years now. |
|
This probably won't be merged soon because #737 brought up more questions about vectors that need to be resolved. |
TLDR; all vectors are permanent, vector methods no longer mutate, it's a big breaking change under consideration.
This pull request resolves #169 by doing the following:
lovr.math.vec3andlovr.math.newVec3) both exist for backwards compat, but they both do the same thing.:setmethods, which continue to assign the vector's components using various inputs.add/sub/mul/divmethods became redundant with metamethod operators, and so they were removed.Mat4:orthographic/perspective/fov/lookAt/targetare a bit weird now. Those should probably instead belovr.math.orthographicorMat4.orthographicinstead of taking an unused Mat4 object as the first argument.lovr.math.pushandlovr.math.pop. Vectors created inside the scope will be invalidated once the scope is popped, allowing them to be reused across loop iterations, frames, etc.:setmethod can be used inside a temporary scope to persist values, since the usuala = a + bapproach wouldn't work because it assigns a temporary vector toa.During the development of this branch, a version using FFI was attempted but abandoned because it was about 4x slower than using Lua's C API, for non-benchmark projects.
This is a massive breaking change that will basically break all existing code! There is a large cost to merging this and it is still under consideration. Feedback is welcome.