Immutable vectors

[jcornaz]

• ImmutableVector2
• ImmutableVector3

---

Hi,

Thank you very much for providing theses utilities for Kotlin :-)

I would like to propose to introduce some immutable classes for at least vector and colors. (could be called KVector2 or ImmutableVector2 for instance)

In order to allow easy usage with LibGDX they could either:

• inherit from their equivalent in LibGDX, deprecate all mutation method and throw if a mutation method is called
• provide adapter like ImmutableVector2.asVector2() and Vector2.toImmutableVector2()

Why?
Even in Java I find error-prone that classes like vector or color are mutable. It is for instance possible to change by mistake Vector2.ZERO, and it is a mistake really easy to do. But I think that's even more true in Kotlin where the language provide good support for immutability (val is the default, data class provides copy to easily get a new object, and immutable fields can be smart-casted).

Mutable vectors cannot be safely shared, and one has to do defensive copy each time it gets a vectors, which is cumbersome, unsafe, and inefficient.

In short:

• Even in Java it should have been immutable
• In Kotlin, immutable data classes are even more idiomatic

[czyzby]

I think LibGDX sacrifices safety for conciseness and performance - the difference might not be noticeable on desktop, but it does somewhat matter on mobile.

That being said, immutable vectors and colors would be a great addition and might prevent some non-obvious bugs - but since there is no Vector or Color interface that you could just implement, the conversion code interacting with existing LibGDX APIs could quickly become tedious. We might also have to reimplement all (or most) methods, or at least delegate them to the original, mutable objects.

@jcornaz Would you like to try implementing such immutable classes or is it a feature request?

[jcornaz]

I think LibGDX sacrifices safety for conciseness and performance - the difference might not be noticeable on desktop, but it does somewhat matter on mobile.

Yes, it does. However as you said it is less true on desktop. And I think it may even actually lead to worse performances, as it is dangerous to share the mutable vectors mutable different threads. Making performances improvement through multi-threading almost impossible.

Would you like to try implementing such immutable classes or is it a feature request?

This is a feature request. But I don't mind trying to implement it myself and propose a PR.

[czyzby]

I'd be happy to review the PR, thanks.

[jcornaz]

What would you think about the following design:

data class ImVector2(val x: Float = 0f, val y: Float = 0f) {
  val isZero get() = x == 0f && y == 0f

  // immutability allow some performance improvements as well, by making able to cache results:
  val len2 by lazy { (x * x) + (y * y) }
  val len by lazy { sqrt(len2) }

  // [...] Other relevant members

  companion object {
    val ZERO = ImVector2()
    val X = ImVector2(x = 1f)
    val Y = ImVector2(y = 1f)
  }
}

// Conversion from/to LibGDX vectors:
fun ImVector2.mutable(): Vector2 = Vector2(x, y)
fun Vector2.immutable(): ImVector2 = ImVector2(x, y)

// Operators as extension functions
operator fun ImVector2.plus(vector: ImVector2): ImVector2 =
  ImVector2(x = x + vector.x, y = y + vector.y)

fun ImVector2.lerp(target: ImVector2, alpha: Float): ImVector2 {
  val invAlpha = 1f - alpha
  return ImVector2(
    x = (x * invAlpha) + (target.x * alpha),
    y = (y * invAlpha) + (target.y * alpha)
  )
}

// [...] Other operators written as extension functions

I'am only talking about design. Of course I'll make more operators, and I'll do it for Vector3 as well. Colors may come in a separate PR.

[czyzby]

Not sure about the name of the class. We could either go with a verbose ImmutableVector2, or a less obvious KVector2. I'm leaning towards KVector2, what do you think?

Lazy fields might be too much of an overhead - instead of just 2 float fields, each vector object would hold a ton of float wrappers if we go that way. Most available vector operations just aren't heavy enough to justify the cache. Let's trust the user to save the computation results.

Some extra dynamic properties (like the length) are OK, but I think the API should have all of the original methods from Vector2 to avoid confusion and work as a drop-in replacement.

Kotlin built-in conversion methods usually start with to, so I think toImmutable and toMutable names would be preferable. Maybe we should also add a concise way to convert an immutable vector into Vector2 for compatibility - like invoke(): Vector2, which would allow to do this: val vector: Vector2 = myImmutableVector(). It isn't obvious, though, so I'm not quite sure about this.

[jcornaz]

Not sure about the name of the class. We could either go with a verbose ImmutableVector2, or a less obvious KVector2. I'm leaning towards KVector2, what do you think?

I was not sure either. I like KVectorN except it doesn't carry the immutability concept. However this may not be a problem as the mutable structure should have an explicit name, not the immutable one.
I'm fine with any naming here. I'll start with KVectorN and we'll chose the final naming when reviewing the PR.

Lazy fields might be too much of an overhead - instead of just 2 float fields, each vector object would hold a ton of float wrappers if we go that way. Most available vector operations just aren't heavy enough to justify the cache. Let's trust the user to save the computation results.

I don't want to add everything in lazy properties. Only the expensive ones, like len which has to compute a square-root. Other properties could either be computed at construction time or evaluated at each call. And for the properties to cach, I may measure actual performance benefit (with JMH) to make sure it is worth caching them.

Some extra dynamic properties (like the length) are OK, but I think the API should have all of the original methods from Vector2 to avoid confusion and work as a drop-in replacement.

Yes everything available from LibGDX vectors should be available. But is it ok if is it available as extension functions? Or would you put it in the class?

Kotlin built-in conversion methods usually start with to, so I think toImmutable and toMutable names would be preferable

Yes. But in this case it should be named toImmutableVectorN and toMutableVectorN shouldn't it? Then it would fit better in Kotlin conventions, but it would be more verbose. I'm ok with both solutions.

Maybe we should also add a concise way to convert an immutable vector into Vector2 for compatibility - like invoke(): Vector2, which would allow to do this: val vector: Vector2 = myImmutableVector(). It isn't obvious, though, so I'm not quite sure about this.

I think toMutableVector is enough. And actually that's why I thought about simply naming them mutable() and immutable() so it would be less verbose to do : val vector: Vector2 = myImmutableVector.mutable()

[czyzby]

Only the expensive ones, like len which has to compute a square-root.

Each lazy property is an extra object created for each vector, regardless of whether you use that particular expensive method or not. Caching in this case might be a premature optimization of a problem that won't affect most use cases. Feel free to write benchmarks, but I think the memory cost is just not worth it - IMHO, vectors should be as close to value classes as JVM (currently) allows.

But is it ok if is it available as extension functions? Or would you put it in the class?

Either way works. I think keeping them in the class might be more readable.

Yes. But in this case it should be named toImmutableVectorN and toMutableVectorN shouldn't it? Then it would fit better in Kotlin conventions, but it would be more verbose. I'm ok with both solutions.

I feel like toMutable is a good trade-off between conciseness and Kotlin conventions. Personally, when I look for conversion methods, I instinctively write to and wait for the auto-completion to do the job.

[jcornaz]

Actually, does it make sense to have immutable vectors without having immutable matrix and quaternions?

I'm starting to think that adding everything would be too much. And maybe this feature request is too much against the concept: "make LibGDX as Kotlin-friendly as possible without turning the API upside down"

Although I'd love to have immutable matrix, vectors and quaternions.

[czyzby]

Immutability is encouraged in Kotlin, as you've said. As long as using the immutable variants is optional and doesn't get in the way, I don't see why we wouldn't add them. I'd only suggest to use delegation for complex methods rather than try to reimplement every single Vector/Matrix operation. It's OK to create a mutable instance, perform the operation on it, and convert it into the immutable variant.

[czyzby]

Start with just the vectors, though. I try to write tests for every feature, and I imagine the vector operations alone would require quite a bit of tests. It's also easier to thoroughly review smaller PRs.