Fix blendToIdentity case in PerspectiveTransformOperation::blend

Fix blendToIdentity case in PerspectiveTransformOperation::blend

Perspective identity matrix correspond to a perspective value of Infinity.

Also, since perspective projection matrix uses the inverse of d, it might make sense to blend the inverse of the to and the from perspective values (and use the inverse of this blend value).

BUG=319567

[Julien - ping for review, 2013-11-21 02:08:08]

Adding some potential reviewers for this change.

[Julien - ping for review, 2013-12-05 01:18:45]

I think the new formula is fine as we want <length> to grow unbounded
(perspective: none is equivalent to an infinite <length>). However we should
simplify it and fix the potential division by 0.

https://codereview.chromium.org/73493002/diff/1/Source/platform/transforms/Pe...
File Source/platform/transforms/PerspectiveTransformOperation.cpp (right):

https://codereview.chromium.org/73493002/diff/1/Source/platform/transforms/Pe...
Source/platform/transforms/PerspectiveTransformOperation.cpp:43: p = 1.0 /
WebCore::blend(1.0 / p, 0.0, progress);
This formula is overly complicated and it took me a while to understand what was
going on. It can be simplified to:

p = p / (1 - progress)

This uncovers an issue in the formula: if progress == 1, we are going to do a
division by 0.

[Julien - ping for review, 2014-01-10 13:41:40]

lgtm, could we add a test for the progress == 1 case (the old division by 0)?

https://codereview.chromium.org/73493002/diff/50001/Source/platform/transform...
File Source/platform/transforms/PerspectiveTransformOperation.cpp (right):

https://codereview.chromium.org/73493002/diff/50001/Source/platform/transform...
Source/platform/transforms/PerspectiveTransformOperation.cpp:43: if (progress ==
1) {
We don't put curly braces around single line if's.

[a.renevier, 2014-01-10 18:21:21]

I was not successful in writing a test where progress == 1. If I try to get the
values at 2s, the PerspectiveTransformOperation::blend is not called

[Julien - ping for review, 2014-01-15 01:09:59]

lgtm, too bad for the test.

[commit-bot: I haz the power, 2014-01-15 01:19:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/a.renevier@samsung.com/73493002/100001

[dstockwell, 2014-01-15 10:24:12]

On 2014/01/15 01:09:59, Julien Chaffraix - PST wrote:
> too bad for the test.

Note that there are other interpolation tests in
LayoutTests/animations/interpolation/perspective-interpolation.html

[commit-bot: I haz the power, 2014-01-15 13:57:11]

Retried try job too often on blink_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=blink_pres...

[Julien - ping for review, 2014-01-15 17:28:34]

On 2014/01/15 10:24:12, dstockwell wrote:
> On 2014/01/15 01:09:59, Julien Chaffraix - PST wrote:
> > too bad for the test.
> 
> Note that there are other interpolation tests in
> LayoutTests/animations/interpolation/perspective-interpolation.html

Fair enough, the fact is that I wasn't totally sure the progress == 1 case could
be reachable. Based on Arno's comment, it seems that it isn't but I still much
prefer to code defensively.

[commit-bot: I haz the power, 2014-01-17 18:58:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/a.renevier@samsung.com/73493002/100001

[commit-bot: I haz the power, 2014-01-17 19:19:31]

Retried try job too often on blink_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=blink_pres...

[a.renevier, 2014-01-17 19:37:38]

trying to reupload the same patch

[commit-bot: I haz the power, 2014-01-21 16:28:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/a.renevier@samsung.com/73493002/270001

[commit-bot: I haz the power, 2014-01-21 16:46:12]

Retried try job too often on blink_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=blink_pres...

[a.renevier, 2014-01-21 17:55:17]

On 2014/01/21 16:46:12, I haz the power (commit-bot) wrote:
> Retried try job too often on blink_presubmit for step(s) presubmit
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=blink_pres...

I have no idea what's going on.

[Julien - ping for review, 2014-01-21 18:09:59]

On 2014/01/21 17:55:17, a.renevier wrote:
> On 2014/01/21 16:46:12, I haz the power (commit-bot) wrote:
> > Retried try job too often on blink_presubmit for step(s) presubmit
> >
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=blink_pres...
> 
> I have no idea what's going on.

The patch doesn't apply anymore (you can click on the "blink_presubmit" trybot
to see what's wrong with it). Updating your checkout and rebaselining the change
is the way to go.

[commit-bot: I haz the power, 2014-01-21 20:58:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/a.renevier@samsung.com/73493002/370001

[commit-bot: I haz the power, 2014-01-21 21:19:21]

Retried try job too often on blink_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=blink_pres...

[a.renevier, 2014-07-02 20:58:55]

Seems like it's fixed now. At least, the testcase
https://bug-52700-attachments.webkit.org/attachment.cgi?id=79464 seems to work
fine now. Should we close the codereview and the bug?

[Julien - ping for review, 2014-07-03 00:18:37]

On 2014/07/02 at 20:58:55, a.renevier wrote:
> Seems like it's fixed now. At least, the testcase
https://bug-52700-attachments.webkit.org/attachment.cgi?id=79464 seems to work
fine now. Should we close the codereview and the bug?

I would make sure we land the test case at least (if it's not covered by the
existing test obviously). That would prevent us from regressing it.

[Julien - ping for review, 2014-07-08 15:09:19]

We need to update the description as we are not fixing anything, just landing a
test case now. You have to do it through the web-interface as git cl doesn't
send local git commit message updates :-(

https://codereview.chromium.org/73493002/diff/430001/LayoutTests/animations/3...
File LayoutTests/animations/3d/transform-perspective.html (right):

https://codereview.chromium.org/73493002/diff/430001/LayoutTests/animations/3...
LayoutTests/animations/3d/transform-perspective.html:35: [0.5, "box2",
"webkitTransform", [1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, -0.0005, 0, 0, 0, 1],
0.002],
It seems really weird to me that the current test case is wrong and that we can
modify the expected result without changing any code.

[Julien - ping for review, 2014-07-08 15:10:48]

btw, I added some try jobs to check the change and they are not able to apply
the patch (the file doesn't seem to exist anymore)

[a.renevier, 2015-04-02 00:14:28]

https://codereview.chromium.org/73493002/diff/430001/LayoutTests/animations/3...
File LayoutTests/animations/3d/transform-perspective.html (right):

https://codereview.chromium.org/73493002/diff/430001/LayoutTests/animations/3...
LayoutTests/animations/3d/transform-perspective.html:35: [0.5, "box2",
"webkitTransform", [1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, -0.0005, 0, 0, 0, 1],
0.002],
On 2014/07/08 15:09:19, Julien Chaffraix - PST wrote:
> It seems really weird to me that the current test case is wrong and that we
can
> modify the expected result without changing any code.

First, sorry for the really late reply. It seems like the testcase is indeed
wrong. We are going from perspective(1000px) to perspective: none; That is
equivalent to perspective: infinity.

With the formula p = p / (1 - progress), that means that at 0.5, we want the
equivalent of perspective(2000px), which would give us a value of 1/2000 in the
transform matrix. This test would also 

The current expected value is equivalent to perspective(500px)

[jamesr, 2015-04-02 00:15:06]

jamesr@chromium.org changed reviewers:
- jamesr@chromium.org

[Stephen Chennney, 2015-04-02 14:36:31]

schenney@chromium.org changed reviewers:
+ jbroman@chromium.org

[Stephen Chennney, 2015-04-02 14:37:18]

+jbroman, who's been recently trapped in the land of  perspective transforms.

[jbroman, 2015-04-02 14:52:49]

On 2015/04/02 14:37:18, Stephen Chenney wrote:
> +jbroman, who's been recently trapped in the land of  perspective transforms.

The testcase on the wkbug still seems broken to me on ToT (try moving your mouse
in and out repeatedly really quickly).