Bug 120528

Summary: [BlackBerry] Improved unproject algorithm
Product: WebKit Reporter: Arvid Nilsson <anilsson>
Component: WebKit BlackBerryAssignee: Arvid Nilsson <anilsson>
Status: RESOLVED INVALID    
Severity: Normal CC: andersca, anilsson, jkjiang, jpetsovits, rwlbuis
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 120362    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch none

Description Arvid Nilsson 2013-08-30 08:07:49 PDT
Instead of doing perspective-correct texturing to unproject, map from
view (x,y) coordinates to view (x,y,z) coordinates by finding the
corresponding z coordinate in the plane of the layer. Then map from view coordinates to (normalized) model coordinates using the inverse model-view matrix.

Now this gets slightly awkward because the unproject method accepts
normalized device coordinates as input, so we have to move to view
space. In the future, we should update the code to mainly work with
view coordinates and only transform to normalized device coordinates at
the very end (e.g. in the shader).

For now though, this means we need to store yet another matrix (which
will hopefully make m_drawTransform obsolete eventually) and we need
to invert what we can of the (orthographic) projection matrix.

However, it's all worth it because the new unprojection code is much
faster and more numerically stable. And the w's aren't needed any more.
Comment 1 Arvid Nilsson 2013-08-30 08:16:57 PDT
Created attachment 210112 [details]
Patch
Comment 2 Jacky Jiang 2013-08-30 08:30:29 PDT
Comment on attachment 210112 [details]
Patch

Looks good to me, r+ internally.
Comment 3 Rob Buis 2013-08-30 10:03:52 PDT
Comment on attachment 210112 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=210112&action=review

Looks good.

> Source/WebCore/ChangeLog:7
> +        Internally reviewed by NOBODY (OOPS!).

This  line will probably not work, at least not with cq+, it should work when pushing by hand.
Comment 4 Arvid Nilsson 2013-09-02 00:05:29 PDT
Comment on attachment 210112 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=210112&action=review

> Source/WebCore/platform/graphics/blackberry/LayerCompositingThread.cpp:135
> +    m_modelViewTransform = projectionMatrix;

Oops, I recreated this patch from memory for the review, but got this bit wrong - it's the model-view bit we want, not the projection bit! Should be

    m_modelViewTransform = matrix;

> Source/WebCore/platform/graphics/blackberry/LayerUtilities.h:251
> +    AffineTransform inverseProjectionMatrix(projectionMatrix.m11(), projectionMatrix.m21(), projectionMatrix.m21(), projectionMatrix.m22(), projectionMatrix.m41(), projectionMatrix.m42());

Typo, I wrote m21 twice... Should be AffineTransform inverseProjectionMatrix(projectionMatrix.m11(), projectionMatrix.m21(), projectionMatrix.m12(), projectionMatrix.m22(), projectionMatrix.m41(), projectionMatrix.m42());
Comment 5 Arvid Nilsson 2013-09-02 00:16:08 PDT
Created attachment 210266 [details]
Patch
Comment 6 Arvid Nilsson 2013-09-02 00:16:39 PDT
(In reply to comment #5)
> Created an attachment (id=210266) [details]
> Patch

Fixed typos, sorry about those
Comment 7 Arvid Nilsson 2013-09-02 00:17:17 PDT
This is a followup to 120362