cc: Stop converting Rect to QuadF to map to an enclosed rect.

cc: Stop converting Rect to QuadF to map to an enclosed rect.

Occlusion tracker converts Rects to QuadF, then does MapClippedQuad,
then BoundingBox(), then ToEnclosedRect(). Instead, add methods to
MathUtil that mirror the {Map,Project}EnclosingClippedRect
methods but for an enclosed rect instead, that work only when the
transform preserves 2d axis alignment.

In the common case, this avoids converting the integers to floats just
to convert back to ints using the costly safe conversion methods.

R=enne

Committed: https://crrev.com/c8357ff51ba8458cc04848fa4ec4452d2660b160
Cr-Commit-Position: refs/heads/master@{#291785}

[danakj, 2014-08-21 02:06:32]

I could probably make the MathUtil code cleaner/less copying or otherwise
somehow a nicer more clear API? But I was wondering what you'd think of this.

[enne (OOO), 2014-08-21 17:54:05]

https://codereview.chromium.org/495873002/diff/20001/cc/base/math_util.cc
File cc/base/math_util.cc (right):

https://codereview.chromium.org/495873002/diff/20001/cc/base/math_util.cc#new...
cc/base/math_util.cc:266: return
gfx::ToEnclosedRect(projected_quad.BoundingBox());
This code was here before, but is this actually right? You're taking enclosed,
but "bounding box" is inherently an *enclosing* operation?

https://codereview.chromium.org/495873002/diff/20001/cc/base/math_util.h
File cc/base/math_util.h (right):

https://codereview.chromium.org/495873002/diff/20001/cc/base/math_util.h#newc...
cc/base/math_util.h:124: static gfx::Rect MapEnclosedNonClippedRect(const
gfx::Transform& transform,
Is it worth having unit tests for these functions? Or verifying that they do the
same as the clipped versions when their inputs are not clipped?

[danakj, 2014-08-21 17:58:08]

https://codereview.chromium.org/495873002/diff/20001/cc/base/math_util.cc
File cc/base/math_util.cc (right):

https://codereview.chromium.org/495873002/diff/20001/cc/base/math_util.cc#new...
cc/base/math_util.cc:266: return
gfx::ToEnclosedRect(projected_quad.BoundingBox());
On 2014/08/21 17:54:05, enne wrote:
> This code was here before, but is this actually right? You're taking enclosed,
> but "bounding box" is inherently an *enclosing* operation?

Ya, good point. I had this thought about everything being broken last night but
then I wrote a test and it didn't fail and I noticed, we only map/project and
intend to get an enclosed thing when Preserves2dAxisAlignment is true. So we
should be able to do something much less general in this method and DCHECK on
the transform.

https://codereview.chromium.org/495873002/diff/20001/cc/base/math_util.h
File cc/base/math_util.h (right):

https://codereview.chromium.org/495873002/diff/20001/cc/base/math_util.h#newc...
cc/base/math_util.h:124: static gfx::Rect MapEnclosedNonClippedRect(const
gfx::Transform& transform,
On 2014/08/21 17:54:05, enne wrote:
> Is it worth having unit tests for these functions? Or verifying that they do
the
> same as the clipped versions when their inputs are not clipped?

Ya I will write tests but I am not happy with the API as is. I think your point
about bounding box is great.

[danakj, 2014-08-25 19:13:36]

Patchset #2 (id:40001) has been deleted

[danakj, 2014-08-25 19:13:59]

PTAL I did something simpler, with reduced math all around, and added tests.

[enne (OOO), 2014-08-25 19:58:56]

lgtm

https://codereview.chromium.org/495873002/diff/60001/cc/base/math_util.cc
File cc/base/math_util.cc (right):

https://codereview.chromium.org/495873002/diff/60001/cc/base/math_util.cc#new...
cc/base/math_util.cc:190: gfx::Rect MathUtil::MapEnclosedNonClippedRect(const
gfx::Transform& transform,
Do you think this function name have the 2d axis alignment assertion embedded in
it too?

[danakj, 2014-08-25 20:30:46]

Used MapEnclosedRectWith2dAxisAlignedTransform as per chat-based decision.

[danakj, 2014-08-25 20:30:49]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2014-08-25 20:31:24]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/danakj@chromium.org/495873002/80001

[commit-bot: I haz the power, 2014-08-25 21:34:09]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[commit-bot: I haz the power, 2014-08-25 22:37:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-25 22:37:04]

Try jobs failed on following builders:
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[danakj, 2014-08-25 22:45:21]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2014-08-25 22:48:55]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/danakj@chromium.org/495873002/80001

[commit-bot: I haz the power, 2014-08-25 23:37:34]

Committed patchset #3 (80001) as 52211584ea6cc6f573ebbc51d7b925ace1e6adb5

[commit-bot: I haz the power, 2014-09-10 02:38:28]

Patchset 3 (id:??) landed as
https://crrev.com/c8357ff51ba8458cc04848fa4ec4452d2660b160
Cr-Commit-Position: refs/heads/master@{#291785}