Bug 120528 - [BlackBerry] Improved unproject algorithm
Summary: [BlackBerry] Improved unproject algorithm
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit BlackBerry (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Arvid Nilsson
URL:
Keywords:
Depends on: 120362
Blocks:
  Show dependency treegraph
 
Reported: 2013-08-30 08:07 PDT by Arvid Nilsson
Modified: 2014-01-09 21:04 PST (History)
5 users (show)

See Also:


Attachments
Patch (11.09 KB, patch)
2013-08-30 08:16 PDT, Arvid Nilsson
no flags Details | Formatted Diff | Diff
Patch (11.07 KB, patch)
2013-09-02 00:16 PDT, Arvid Nilsson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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