[BlackBerry] Don't use reinterpret_cast to convert to TransformationMatrix

• Component: WebKit BlackBerry
• Status: RESOLVED INVALID

[Jakob Petsovits]

We're currently using reinterpret_cast in WebPageCompositor::render() to convert from a BlackBerry::Platform::TransformationMatrix to a WebCore::TransformationMatrix. This is especially bad since the BlackBerry::Platform one uses column-major storage and the WebCore one stores its values in row-major layout. It basically only worked because so far we're using an extremely limited subset of potential transformation matrices.

Fixed by the patch below.

[Jakob Petsovits]

Created attachment 202435 [details]
Patch

[Arvid Nilsson]

While the reinterpret_cast may be ugly, I don't agree that WebCore::TransformationMatrix is row major - I think the commit message is misleading. TransformationMatrix order was swapped by Chromium devs quite a while ago.

[Arvid Nilsson]

I also note that the range of transformation matrices used by CSS3 Transforms is not at all limited, and converts successfully to BB::P::TransformationMatrix every frame. It's then passed directly to OpenGL down in platform, and OpenGL expects column major matrices (according to some accounts, not 100% sure about GL ES2 actually)...

[Jakob Petsovits]

(In reply to comment #2)
> While the reinterpret_cast may be ugly, I don't agree that WebCore::TransformationMatrix is row major - I think the commit message is misleading. TransformationMatrix order was swapped by Chromium devs quite a while ago.

My math is not great, but let me get the facts together here.

(1) http://en.wikipedia.org/wiki/Row-major_order row-major order means that an array of
1 2 3
4 5 6
is laid out in memory as { { 1, 2, 3 }, { 4, 5, 6 } }. That means the element with value 4 is located at m_matrix[1][0].

(2) According to the handy graphic at http://en.wikipedia.org/wiki/Matrix_%28mathematics%29 and its caption, a matrix descriptor a(2,1) represents the element in the second row, first column of the matrix.

(3) Wolfram Alpha, http://en.wikipedia.org/wiki/Translation_%28geometry%29 as well as your favourite math textbook state that translation elements are on the right edge of a 4x4 matrix, starting with x from the top. That means x translation is at a(1,4), y is at a(2,4) and z at a(3,4).

(4) For the x and y translation elements, aliased as methods e() and f(), WebCore::TransformationMatrix returns m_matrix[3][0] and m_matrix[3][1]. In a row-major matrix, these values would correspond to a(4,1) and a(4,2). However according to (3) this is not the case, and therefore WebCore::TransformationMatrix is column-major. (i.e. you're right)

(5) WebCore::TransformationMatrix::m14() returns m_matrix[0][3], which means WebCore's matrix accessor methods are an outright misnomer, or confusing at best.

(6) The BlackBerry::Platform::TransformationMatrix constructor assigns its arguments in the same order and with the same names as WebCore::TransformationMatrix, but then assigns them to the mathematically correct position instead. As determined above, mathematically correct means codewise incorrect. Consequently, I'll go off to change BlackBerry::Platform instead and will hopefully figure out what to do about the mIJ() accessor naming.

It seems this patch won't be necessary, then. Let me confirm and close later if this is the case.

[Arvid Nilsson]

I'm probably guilty of the BB::P::TransformationMatrix constructor being wrong - because of all the reinterpret_cast going on, it was probably dead code until now...
.