Fix handling of projective transforms #199
Conversation
When transforming points with projective matrices, we must divide the resulting vector by w, in order to get the normalized result. This commit fixes graphene_matrix_transform_point, graphene_matrix_transform_point3d and graphene_matrix_transform_bounds. I suspect that some of the remaining transform functions need similar fixes, but they are not used in GTK, so I couldn't verify that.
src/graphene-matrix.c
Outdated
| res->z = graphene_simd4f_get_z (vec3); | ||
| res->x = graphene_simd4f_get_x (vec3) / graphene_simd4f_get_w (vec3); | ||
| res->y = graphene_simd4f_get_y (vec3) / graphene_simd4f_get_w (vec3); | ||
| res->z = graphene_simd4f_get_y (vec3) / graphene_simd4f_get_w (vec3); |
Fix a copy-paste error Pointed out by Timm Bäder.
|
Here is a gtk-side fix for now: https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/2514 |
| res->z = graphene_simd4f_get_z (vec3); | ||
| res->x = graphene_simd4f_get_x (vec3) / graphene_simd4f_get_w (vec3); | ||
| res->y = graphene_simd4f_get_y (vec3) / graphene_simd4f_get_w (vec3); | ||
| res->z = graphene_simd4f_get_z (vec3) / graphene_simd4f_get_w (vec3); | ||
| } |
There was a problem hiding this comment.
graphene_matrix_unproject_point3d() does:
inv_w = 1.f / graphene_simd4f_get_w (v);
v = graphene_simd4f_mul (v, graphene_simd4f_splat (inv_w));
Which seems more efficient than this
There was a problem hiding this comment.
good point. Also interesting that graphene_matrix_unproject_point3d takes two matrices - I get the sense that we are not fully understanding the graphene api and are not using it properly in gtk
There was a problem hiding this comment.
Unprojection requires the viewport, and has a split modelview/projection matrices because most users of such an API will have a separate modelview and projection matrices already.
I guess the issue, here, is that Graphene adheres to a different set of conventions. The documentation should clarify the use cases.
There was a problem hiding this comment.
In 3d apps, the scene graph typically converts objects into camera space, which is a 3d space with the cameras location at origin, looking down -z axis. Then the camera projection matrix is applied on top of that to convert to 2d space. The exact transform there depends on the camera (i.e. perspective or orthonormal, field of vision, etc). When rendering an object we combine all these matrixes into one (object -> world -> camera -> screen) which the GPU applies to each object vector.
However, for unproject we want to go backwards into world space (not object space), so we invert the camera->screen and apply the camera->world matrix on top of that. These are taken as two separate args because those are the two matrixes that the client usually stores, and we can't use the combined matrix because that is from object space (and only available during rendering).
For gtk+, I don't really think we need unprojection, because we don't set up a complicated projection anyway. It just maps the z=0 plane one-to-one. I guess that means the exact z perspective behaviour is just what happens to fall out of the math... Maybe worth looking at.
There was a problem hiding this comment.
We are handling 4x4 matrices here that operate on points in homogeneous coordinates, to go from homogeneous coordinates to points with 3 coordinates (or 2, as the transform_bounds api does) in general requires a division by w, otherwise the results are just wrong.
Given that this api is in use, we should probably just document the fact that passing a matrix with perspective component to transform_bounds and transform_points is going to yield unexpected results.
For GTK, I have added the functions we needed here: https://gitlab.gnome.org/GNOME/gtk/-/blob/master/gsk/gsktransform.c#L2169
|
Given things like https://gitlab.gnome.org/GNOME/gtk/-/commit/30ab8b3ecac15e22436f6c38c1149e54f42f0d50 it seems existing code relies on the current behaviour. So, is it safe to change this? |
When transforming points with projective matrices,
we must divide the resulting vector by w, in order
to get the normalized result.
This commit fixes graphene_matrix_transform_point,
graphene_matrix_transform_point3d and
graphene_matrix_transform_bounds. I suspect that
some of the remaining transform functions need
similar fixes, but they are not used in GTK, so
I couldn't verify that.
Before:
After:
Notice how the bounding box for the projectively transformed button is much too small before, and just right after.