fix: Camera.viewToRay end position calculation

[kubax2000]

#400 fix

If z equals 1.0f viewToWorld returns [NaN, NaN, -Infinity].

[grassydragon]

Hi!
Are you sure this pull request fixes the referenced issue?

[ThomasGorisse]

You're right @grassydragon.
The issue might be coming from there:
https://github.com/SceneView/sceneview-android/blob/main/sceneview%2Fsrc%2Fmain%2Fjava%2Fio%2Fgithub%2Fsceneview%2Futils%2FCamera.kt#L253-L267
@grassydragon This code was coming from Sceneform but I can't figure out what is wrong. Could you have a quick look at it?

[grassydragon]

Could you have a quick look at it?

Yes, I'll take a look today in the evening.

[grassydragon]

In Sceneform there is a return then when w is close to 0 and z is set to 0 to in this case: https://github.com/SceneView/sceneform-android/blob/master/core/src/main/java/com/google/ar/sceneform/Camera.java#L386-L389

[ThomasGorisse]

That's also what I had in mind but do you think the NAN could come from there?

[ThomasGorisse]

Here is the fix I had in mind 😀
https://www.linkedin.com/posts/thomas-gorisse_android-studio-gemini-integration-is-simply-activity-7198315922854490112-2c3E?utm_source=share&utm_medium=member_android

[den59k]

Hi!
I faced the same problem - screen tap didn't work. I found out that it was due to incorrect operation of * operator from dev.romainguy.kotlin.math library.

To be sure, replace the line
https://github.com/SceneView/sceneview-android/blob/ffb2f40500c6d1807d33a028e0a50f9f6630a79f/sceneview/src/main/java/io/github/sceneview/utils/Camera.kt#L261

with

    val arr = inverse(projectionTransform * viewTransform).toFloatArray()
    val vec = clipSpacePosition.toFloatArray()
    val resultVec = FloatArray(4)
    Matrix.multiplyMV(resultVec, 0, arr, 0, vec, 0)
    val result = resultVec.toFloat4()

and get the correct answer

I hope this helps in finding the right solution

[grassydragon]

That's also what I had in mind but do you think the NAN could come from there?

@ThomasGorisse, to be honest, I don't understand the formula in the unproject method completely. I'll see how it's implemented in other libraries.

[ThomasGorisse]

Thanks a lot @den59k . That was very very tricky to find. Good job!!!
@grassydragon Don't you think it's only due to the usual row/column order inversion between kotlin-math and Filament?

[grassydragon]

Don't you think it's only due to the usual row/column order inversion between kotlin-math and Filament?

I'll try soon and tell the results. I checked yesterday that other libraries, for example, GLM, had the same formula for unprojecting a point so the problem isn't in the formula.

[den59k]

Yeah, it doesn't seem to be the formula, sorry. I found out after I started testing further.

But my click started working correctly after I replaced projectionTransform with cullingProjectionTransform

@ThomasGorisse I will describe the problem of why NaN occurs:

When we project a point onto projectionTransform, the w component is zero (the point goes to infinity). Then there is a condition that replaces xy by 0 in this case (I don't know why this is here, it looks like a dirty hack). And at the end the result is multiplied by 1/w, and in the Kotlin 0 * Infinity = NaN

[grassydragon]

Don't you think it's only due to the usual row/column order inversion between kotlin-math and Filament?

@ThomasGorisse, no, I've checked and this isn't the case here.

But my click started working correctly after I replaced projectionTransform with cullingProjectionTransform

Yes, @den59k is absolutely right. We can't use the projectionTransform since the Filament documentation says that this projection matrix has its far plane set to infinity.

I think the final code of the method should be as follows:

fun Camera.viewToWorld(viewPosition: Float2, z: Float = 1.0f): Position {
    // Normalize between -1 and 1.
    val clipSpacePosition = Float4(
        x = viewPosition.x * 2.0f - 1.0f,
        y = viewPosition.y * 2.0f - 1.0f,
        z = 2.0f * z - 1.0f,
        w = 1.0f
    )
    val result = inverse(cullingProjectionTransform * viewTransform) * clipSpacePosition

    if (MathHelper.almostEqualRelativeAndAbs(result.w, 0.0f)) {
        return Position()
    }

    return result.xyz / result.w
}

[ThomasGorisse]

Great work here!
I'll create the PR with a MathHelper.almostEqualRelativeAndAbs() replacement applied to Kotlin-math and check if everything is alright.
Thanks all.

[ThomasGorisse]

Fixed in 2.2.0 from 5cc1fb0