cc: Spiral iterator fix for negative center.

cc: Spiral iterator fix for negative center.

When the center is off the tiling, then the unclamped calculations
are incorrect. That is, instead of values being rounded down in
integer divides, they are instead rounded toward zero.

This patch reworks the way the around coordinates are calculated.
Instead of relying on unclamped version of existing functions,
we instead explicitly check whether the src coord is within the
tiling total size.

R=enne

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=255670

[vmpstr, 2014-03-06 17:58:30]

Please take a look.

[enne (OOO), 2014-03-06 18:18:51]

A spiral iterator test for this is fine in addition, but can you add more simple
tests that just have TilingData and call these functions?

[vmpstr, 2014-03-06 19:22:51]

On 2014/03/06 18:18:51, enne wrote:
> A spiral iterator test for this is fine in addition, but can you add more
simple
> tests that just have TilingData and call these functions?

Added. Please take a look.

[enne (OOO), 2014-03-06 20:13:49]

I apologize for not taking a closer look before, but I feel like this unclamped
function itself is a little awkward.  Tile coordinates outside the tiling data
bounds are ill-defined because of border texels.  So, it's unclear to me what
values this should be returning in a general sense for different negative
numbers.

It looks like what you really want is "is this coordinate outside the bounds, if
so return a negative number", and that seems like something the caller of the
unclamped function could do (and then call the normal version if the coordinates
are reasonable) rather than having this unclamped function that returns weird
values that happen to work for your use case because you clamp any negative
number to -1 eventually.

[vmpstr, 2014-03-06 21:42:28]

On 2014/03/06 20:13:49, enne wrote:
> I apologize for not taking a closer look before, but I feel like this
unclamped
> function itself is a little awkward.  Tile coordinates outside the tiling data
> bounds are ill-defined because of border texels.  So, it's unclear to me what
> values this should be returning in a general sense for different negative
> numbers.
> 
> It looks like what you really want is "is this coordinate outside the bounds,
if
> so return a negative number", and that seems like something the caller of the
> unclamped function could do (and then call the normal version if the
coordinates
> are reasonable) rather than having this unclamped function that returns weird
> values that happen to work for your use case because you clamp any negative
> number to -1 eventually.

PTAL.

[enne (OOO), 2014-03-06 23:07:35]

lgtm, thanks.  Sorry for the churn in my suggestions.

[vmpstr, 2014-03-06 23:27:59]

The CQ bit was checked by vmpstr@chromium.org

[commit-bot: I haz the power, 2014-03-06 23:34:34]

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

[commit-bot: I haz the power, 2014-03-07 06:19:31]

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

[commit-bot: I haz the power, 2014-03-07 06:19:31]

Retried try job too often on mac_rel for step(s) telemetry_perf_unittests,
telemetry_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[vmpstr, 2014-03-07 17:25:06]

The CQ bit was checked by vmpstr@chromium.org

[commit-bot: I haz the power, 2014-03-07 17:28:17]

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

[commit-bot: I haz the power, 2014-03-07 18:39:48]

Change committed as 255670