1354946 - Fix passing Matrix4x4 across the FFI boundary

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, enhancement, P3)

Description

[Kartikaya Gupta (email:[e-mail])]

Right now the FFI signature for wr_push_stacking_context takes a float* on the C++ side and accepts a LayoutTransform on the Rust side. However, LayoutTransform is not repr(C) which means there is no guarantee that it's layout is the same as a float array. This is potential bug, and something we need to fix for the binding generator to work properly.

I'm working on a patch for this right now. It adds a WrMatrix type that stores a float[16] and otherwise behaves much like the WrRect type.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Attachment: Patch

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Green try push including this patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e0b9d1573a96eb4e5a60a05cf95c46085d5f0cde

Comment 3

[Ryan Hunt [:rhunt] (on leave until early May)]

Comment on attachment 8856298 [details] [diff] [review]
Patch

Review of attachment 8856298 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/webrender_bindings/WebRenderAPI.cpp
@@ +534,5 @@
>  {
> +  WrMatrix transform;
> +  static_assert(sizeof(aTransform.components) == sizeof(transform.values),
> +      "Matrix components size mismatch!");
> +  memcpy(transform.values, aTransform.components, sizeof(transform.values));

Can we have a wr::ToWrMatrix function for this?

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1354946 - Add an explicit WrMatrix type for passing 4x4 matrices across the FFI boundary.

Right now we just cast from a float* on the C++ side to a LayoutTransform on
the Rust side, except the LayoutTransform isn't a repr(C) storage type, so
there's no guarantee that it will actually have the same layout. Adding an
explicit type and conversion code ensure that we are able to pass the matrix
across the boundary safely.

Review commit: https://reviewboard.mozilla.org/r/128650/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/128650/

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Updated patch to have a ToWrMatrix function. This try push has the updated patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f73793aad289b511b84c5a57832b909b519fe0b7

Comment 6

[Ryan Hunt [:rhunt] (on leave until early May)]

Comment on attachment 8856736 [details]
Bug 1354946 - Add an explicit WrMatrix type for passing 4x4 matrices across the FFI boundary.

Thanks!

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/projects/graphics/rev/1bfa30d18fef
Add an explicit WrMatrix type for passing 4x4 matrices across the FFI boundary. r=rhunt

Comment 8

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/mozilla-central/rev/1bfa30d18fef