cc: Fix bugs found by fuzzer due to floating point imprecision.

cc: Fix bugs found by fuzzer due to floating point imprecision.

This patch changes two things:
- For scales that are within epsilon value of 1, we use 1 directly since
minute differences could cause changes when multiplied by large values
- Changed the index generation to consider the fact that using wanted
pixels might grab more tiles than needed.

BUG=707430
R=enne@chromium.org, danakj@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2816943004
Cr-Commit-Position: refs/heads/master@{#471342}
Committed: https://chromium.googlesource.com/chromium/src/+/7ff1f0e4e9ad3eef8f4a1e36c08b95237a1c4dcf

[vmpstr, 2017-04-13 21:36:43]

Description was changed from

==========
cc: Fix bugs found by fuzzer due to floating point imprecision.

This patch changes two things:
- For scales that are within epsilon value of 1, we use 1 directly since
minute differences could cause changes when multiplied by large values
- Changed the index generation to consider the fact that using wanted
pixels might grab more tiles than needed.

BUG=707430
R=enne@chromium.org, danakj@chromium.org
==========

to

==========
cc: Fix bugs found by fuzzer due to floating point imprecision.

This patch changes two things:
- For scales that are within epsilon value of 1, we use 1 directly since
minute differences could cause changes when multiplied by large values
- Changed the index generation to consider the fact that using wanted
pixels might grab more tiles than needed.

BUG=707430
R=enne@chromium.org, danakj@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[vmpstr, 2017-04-13 21:37:53]

Please take a look.

[enne (OOO), 2017-04-14 05:35:55]

enne@chromium.org changed reviewers:
+ trchen@chromium.org

[enne (OOO), 2017-04-14 05:35:56]

+trchen for opinions about tiling texel math

https://codereview.chromium.org/2816943004/diff/1/cc/tiles/picture_layer_tili...
File cc/tiles/picture_layer_tiling.cc (right):

https://codereview.chromium.org/2816943004/diff/1/cc/tiles/picture_layer_tili...
cc/tiles/picture_layer_tiling.cc:455: while
(!content_rect.Intersects(data.TexelExtent(right_, top_)) &&
Hmmm, do you think that FirstBorderTileXIndexFromSrcCoord should return the
correct value here, rather than having PictureLayerTiling do this work?

https://codereview.chromium.org/2816943004/diff/1/cc/tiles/picture_layer_tili...
cc/tiles/picture_layer_tiling.cc:562: #if DCHECK_IS_ON()
Do you need these extra #ifs?

[vmpstr, 2017-04-14 18:17:42]

https://codereview.chromium.org/2816943004/diff/1/cc/tiles/picture_layer_tili...
File cc/tiles/picture_layer_tiling.cc (right):

https://codereview.chromium.org/2816943004/diff/1/cc/tiles/picture_layer_tili...
cc/tiles/picture_layer_tiling.cc:455: while
(!content_rect.Intersects(data.TexelExtent(right_, top_)) &&
On 2017/04/14 05:35:56, enne wrote:
> Hmmm, do you think that FirstBorderTileXIndexFromSrcCoord should return the
> correct value here, rather than having PictureLayerTiling do this work?

The problem is that FirstBorderTileXIndexFromSrcCoord takes a float, and at that
point we've already accessed "right()" which returns a wrong value... I'm happy
to do this some other way if there are other options.

https://codereview.chromium.org/2816943004/diff/1/cc/tiles/picture_layer_tili...
cc/tiles/picture_layer_tiling.cc:562: #if DCHECK_IS_ON()
On 2017/04/14 05:35:56, enne wrote:
> Do you need these extra #ifs?

Not really, it's just everything is DCHECK in there.. I can remove.

[trchen, 2017-04-14 22:07:33]

https://codereview.chromium.org/2816943004/diff/1/cc/tiles/picture_layer_tili...
File cc/tiles/picture_layer_tiling.cc (right):

https://codereview.chromium.org/2816943004/diff/1/cc/tiles/picture_layer_tili...
cc/tiles/picture_layer_tiling.cc:36: }
I doubt this function works as intended...

By definition, epsilion is the difference between 1.f and the nearest machine
number, that means there is only one machine number within the range (1.f -
epsilion, 1.f + epsilon). That is, 1.f. In other words, this function is no-op
in theory.

C/C++ excessive precision rule complicates the issue. C++11 only requires
truncating to specified precision when binding value to a variable, but in
reality compilers may or may not respect the rule depends on compiler flags.

Nevertheless x86_64 targets now default to use SSE for FP implementation, which
doesn't carry excessive precision. Thus I still believe this function is no-op
(except for corner cases such as +-NaN, +-Inf).

https://codereview.chromium.org/2816943004/diff/1/cc/tiles/picture_layer_tili...
cc/tiles/picture_layer_tiling.cc:455: while
(!content_rect.Intersects(data.TexelExtent(right_, top_)) &&
On 2017/04/14 18:17:42, vmpstr wrote:
> On 2017/04/14 05:35:56, enne wrote:
> > Hmmm, do you think that FirstBorderTileXIndexFromSrcCoord should return the
> > correct value here, rather than having PictureLayerTiling do this work?
> 
> The problem is that FirstBorderTileXIndexFromSrcCoord takes a float, and at
that
> point we've already accessed "right()" which returns a wrong value... I'm
happy
> to do this some other way if there are other options.

Just double checked FirstBorderTileXIndexFromSrcCoord takes an int. Since
wanted_texels is integer rect, wanted_texels.right() must be accurate (unless it
overflowed). The accurate description of the problem would be wanted_texels
being inaccurate because ToEnclosingRect introduced errors.

I think it is possible to implement a version of ToEnclosingRect that is
accurate (and not too expensive!):

void EnclosingSegment(float x, float w, int& ox, int& ow)
__attribute__((optimize("no-associative-math")));
void EnclosingSegment(float x, float w, int& ox, int& ow) {
  float floorx = floorf(x);
  ox = static_cast<int>(floorx);
  float ceilw = ceilf(w);
  ow = static_cast<int>(ceilw);
  float delta = ceilf((x - floorx) + (w - ceilw));
  ow += static_cast<int>(delta);
}

[vmpstr, 2017-04-14 23:36:26]

https://codereview.chromium.org/2816943004/diff/1/cc/tiles/picture_layer_tili...
File cc/tiles/picture_layer_tiling.cc (right):

https://codereview.chromium.org/2816943004/diff/1/cc/tiles/picture_layer_tili...
cc/tiles/picture_layer_tiling.cc:36: }
On 2017/04/14 22:07:33, trchen wrote:
> I doubt this function works as intended...
> 
> By definition, epsilion is the difference between 1.f and the nearest machine
> number, that means there is only one machine number within the range (1.f -
> epsilion, 1.f + epsilon). That is, 1.f. In other words, this function is no-op
> in theory.

The definition of epsilon is the difference between 1.f and the next
representable machine number. We need to be careful when generalizing this
definition to all numbers representable by a float. As a trivial example, there
are certainly numbers that are between 0 and epsilon, since the exponent of the
float can be large negative. Specifically in this case, the reasoning that that
there are no numbers representable in (1 - eps, 1 + eps) other than 1 itself is
not true, since the definition of epsilon does not mention anything that needs
to be representable in the (1 - eps, 1) range. Consider this:

float one = 1.f - std::numeric_limits<float>::epsilon() * .5f;

fprintf (stderr, "epsilon: %.10f\n", std::numeric_limits<float>::epsilon());
fprintf (stderr, "1 - one: %.10f\n", 1.f - one);

This yields:
epsilon: 0.0000001192
1 - one: 0.0000000596

which means that this number "one" is representable by float and is between (1 -
eps) and 1.

Multiplying this by some arbitrarily large int (such as 1 << 30) yields:
number * one = 1073741760
number * SnapToOne(one) = 1073741824 (= 1 << 30)

> 
> C/C++ excessive precision rule complicates the issue. C++11 only requires
> truncating to specified precision when binding value to a variable, but in
> reality compilers may or may not respect the rule depends on compiler flags.
> 
> Nevertheless x86_64 targets now default to use SSE for FP implementation,
which
> doesn't carry excessive precision. Thus I still believe this function is no-op
> (except for corner cases such as +-NaN, +-Inf).
>

https://codereview.chromium.org/2816943004/diff/1/cc/tiles/picture_layer_tili...
cc/tiles/picture_layer_tiling.cc:455: while
(!content_rect.Intersects(data.TexelExtent(right_, top_)) &&
On 2017/04/14 22:07:33, trchen wrote:
> On 2017/04/14 18:17:42, vmpstr wrote:
> > On 2017/04/14 05:35:56, enne wrote:
> > > Hmmm, do you think that FirstBorderTileXIndexFromSrcCoord should return
the
> > > correct value here, rather than having PictureLayerTiling do this work?
> > 
> > The problem is that FirstBorderTileXIndexFromSrcCoord takes a float, and at
> that
> > point we've already accessed "right()" which returns a wrong value... I'm
> happy
> > to do this some other way if there are other options.
> 
> Just double checked FirstBorderTileXIndexFromSrcCoord takes an int. Since
> wanted_texels is integer rect, wanted_texels.right() must be accurate (unless
it
> overflowed). The accurate description of the problem would be wanted_texels
> being inaccurate because ToEnclosingRect introduced errors.

Yes, I meant the access to right/bottom in enclosing rect is what causes the
issue.

> 
> I think it is possible to implement a version of ToEnclosingRect that is
> accurate (and not too expensive!):
> 
> void EnclosingSegment(float x, float w, int& ox, int& ow)
> __attribute__((optimize("no-associative-math")));
> void EnclosingSegment(float x, float w, int& ox, int& ow) {
>   float floorx = floorf(x);
>   ox = static_cast<int>(floorx);
>   float ceilw = ceilf(w);
>   ow = static_cast<int>(ceilw);
>   float delta = ceilf((x - floorx) + (w - ceilw));
>   ow += static_cast<int>(delta);
> }

[trchen, 2017-04-14 23:53:43]

https://codereview.chromium.org/2816943004/diff/1/cc/tiles/picture_layer_tili...
File cc/tiles/picture_layer_tiling.cc (right):

https://codereview.chromium.org/2816943004/diff/1/cc/tiles/picture_layer_tili...
cc/tiles/picture_layer_tiling.cc:36: }
On 2017/04/14 23:36:26, vmpstr wrote:
> On 2017/04/14 22:07:33, trchen wrote:
> > I doubt this function works as intended...
> > 
> > By definition, epsilion is the difference between 1.f and the nearest
machine
> > number, that means there is only one machine number within the range (1.f -
> > epsilion, 1.f + epsilon). That is, 1.f. In other words, this function is
no-op
> > in theory.
> 
> The definition of epsilon is the difference between 1.f and the next
> representable machine number. We need to be careful when generalizing this
> definition to all numbers representable by a float. As a trivial example,
there
> are certainly numbers that are between 0 and epsilon, since the exponent of
the
> float can be large negative. Specifically in this case, the reasoning that
that
> there are no numbers representable in (1 - eps, 1 + eps) other than 1 itself
is
> not true, since the definition of epsilon does not mention anything that needs
> to be representable in the (1 - eps, 1) range. Consider this:
> 
> float one = 1.f - std::numeric_limits<float>::epsilon() * .5f;
> 
> fprintf (stderr, "epsilon: %.10f\n", std::numeric_limits<float>::epsilon());
> fprintf (stderr, "1 - one: %.10f\n", 1.f - one);
> 
> This yields:
> epsilon: 0.0000001192
> 1 - one: 0.0000000596
> 
> which means that this number "one" is representable by float and is between (1
-
> eps) and 1.
> 
> Multiplying this by some arbitrarily large int (such as 1 << 30) yields:
> number * one = 1073741760
> number * SnapToOne(one) = 1073741824 (= 1 << 30)

You're right. I forgot (1-epsilon) contains only 23 digits. But the only
exception is 1-2^-24. Your code would be equivalent to

constexpr float before_one = 1.f - std::numeric_limits<float>::epsilon() * 0.5f;
return (n == before_one) ? 1.f : n;

Why is the value special? Why is (1.f - epsilon) fine?

[vmpstr, 2017-04-15 00:31:25]

https://codereview.chromium.org/2816943004/diff/1/cc/tiles/picture_layer_tili...
File cc/tiles/picture_layer_tiling.cc (right):

https://codereview.chromium.org/2816943004/diff/1/cc/tiles/picture_layer_tili...
cc/tiles/picture_layer_tiling.cc:36: }
On 2017/04/14 23:53:43, trchen wrote:
> On 2017/04/14 23:36:26, vmpstr wrote:
> > On 2017/04/14 22:07:33, trchen wrote:
> > > I doubt this function works as intended...
> > > 
> > > By definition, epsilion is the difference between 1.f and the nearest
> machine
> > > number, that means there is only one machine number within the range (1.f
-
> > > epsilion, 1.f + epsilon). That is, 1.f. In other words, this function is
> no-op
> > > in theory.
> > 
> > The definition of epsilon is the difference between 1.f and the next
> > representable machine number. We need to be careful when generalizing this
> > definition to all numbers representable by a float. As a trivial example,
> there
> > are certainly numbers that are between 0 and epsilon, since the exponent of
> the
> > float can be large negative. Specifically in this case, the reasoning that
> that
> > there are no numbers representable in (1 - eps, 1 + eps) other than 1 itself
> is
> > not true, since the definition of epsilon does not mention anything that
needs
> > to be representable in the (1 - eps, 1) range. Consider this:
> > 
> > float one = 1.f - std::numeric_limits<float>::epsilon() * .5f;
> > 
> > fprintf (stderr, "epsilon: %.10f\n", std::numeric_limits<float>::epsilon());
> > fprintf (stderr, "1 - one: %.10f\n", 1.f - one);
> > 
> > This yields:
> > epsilon: 0.0000001192
> > 1 - one: 0.0000000596
> > 
> > which means that this number "one" is representable by float and is between
(1
> -
> > eps) and 1.
> > 
> > Multiplying this by some arbitrarily large int (such as 1 << 30) yields:
> > number * one = 1073741760
> > number * SnapToOne(one) = 1073741824 (= 1 << 30)
> 
> You're right. I forgot (1-epsilon) contains only 23 digits. But the only
> exception is 1-2^-24. Your code would be equivalent to
> 
> constexpr float before_one = 1.f - std::numeric_limits<float>::epsilon() *
0.5f;
> return (n == before_one) ? 1.f : n;
> 
> Why is the value special? Why is (1.f - epsilon) fine?

It just happens that when coverage scale is 7352.331055f and the raster
transform scale is 7352.331055f, then the math on lines 384-385 of the "before"
code makes the resulting scale be that. (That's the scale that fuzzer found).
I'm happy to use a larger epsilon.

Overall, I think we might have to rethink our whole approach to float scales in
combination with floating math and integer conversions when it comes to large
values, otherwise we'll keep running into situations like this.

[enne (OOO), 2017-04-18 20:50:02]

Discussed some with trchen in person.  lgtm to do this like this.  Thanks for
the unit test.

trchen is worried some about left and top maybe being wrong, and I suggested
that maybe he write a unit test that exposes that if that's true.  I also think
that maybe there's some more general way to fix this in ToEnclosingRect, but I
think I'd like to be motivated by a real world use case before we go down that
route.

Sorry for the review delay!

[trchen, 2017-04-18 21:13:46]

https://codereview.chromium.org/2816943004/diff/1/cc/tiles/picture_layer_tili...
File cc/tiles/picture_layer_tiling.cc (right):

https://codereview.chromium.org/2816943004/diff/1/cc/tiles/picture_layer_tili...
cc/tiles/picture_layer_tiling.cc:36: }
On 2017/04/15 00:31:24, vmpstr wrote:
> On 2017/04/14 23:53:43, trchen wrote:
> > On 2017/04/14 23:36:26, vmpstr wrote:
> > > On 2017/04/14 22:07:33, trchen wrote:
> > > > I doubt this function works as intended...
> > > > 
> > > > By definition, epsilion is the difference between 1.f and the nearest
> > machine
> > > > number, that means there is only one machine number within the range
(1.f
> -
> > > > epsilion, 1.f + epsilon). That is, 1.f. In other words, this function is
> > no-op
> > > > in theory.
> > > 
> > > The definition of epsilon is the difference between 1.f and the next
> > > representable machine number. We need to be careful when generalizing this
> > > definition to all numbers representable by a float. As a trivial example,
> > there
> > > are certainly numbers that are between 0 and epsilon, since the exponent
of
> > the
> > > float can be large negative. Specifically in this case, the reasoning that
> > that
> > > there are no numbers representable in (1 - eps, 1 + eps) other than 1
itself
> > is
> > > not true, since the definition of epsilon does not mention anything that
> needs
> > > to be representable in the (1 - eps, 1) range. Consider this:
> > > 
> > > float one = 1.f - std::numeric_limits<float>::epsilon() * .5f;
> > > 
> > > fprintf (stderr, "epsilon: %.10f\n",
std::numeric_limits<float>::epsilon());
> > > fprintf (stderr, "1 - one: %.10f\n", 1.f - one);
> > > 
> > > This yields:
> > > epsilon: 0.0000001192
> > > 1 - one: 0.0000000596
> > > 
> > > which means that this number "one" is representable by float and is
between
> (1
> > -
> > > eps) and 1.
> > > 
> > > Multiplying this by some arbitrarily large int (such as 1 << 30) yields:
> > > number * one = 1073741760
> > > number * SnapToOne(one) = 1073741824 (= 1 << 30)
> > 
> > You're right. I forgot (1-epsilon) contains only 23 digits. But the only
> > exception is 1-2^-24. Your code would be equivalent to
> > 
> > constexpr float before_one = 1.f - std::numeric_limits<float>::epsilon() *
> 0.5f;
> > return (n == before_one) ? 1.f : n;
> > 
> > Why is the value special? Why is (1.f - epsilon) fine?
> 
> It just happens that when coverage scale is 7352.331055f and the raster
> transform scale is 7352.331055f, then the math on lines 384-385 of the
"before"
> code makes the resulting scale be that. (That's the scale that fuzzer found).
> I'm happy to use a larger epsilon.
> 
> Overall, I think we might have to rethink our whole approach to float scales
in
> combination with floating math and integer conversions when it comes to large
> values, otherwise we'll keep running into situations like this.

Does this helps with the bug you're fixing? I seems to me it is mainly line
441~463 that adjusted the tile range to match coverage rect.

In general I feel snapping would make analysis harder, because error and
tolerance is a part of FP programming and snapping introduces one more error
source.

https://codereview.chromium.org/2816943004/diff/1/cc/tiles/picture_layer_tili...
cc/tiles/picture_layer_tiling.cc:455: while
(!content_rect.Intersects(data.TexelExtent(right_, top_)) &&
Just sync'd up with enne@ offline. We agreed that there is some unknown
performance implication with accurate ToEnclosingRect. Needs to do it with a
separate CL and see if performance is acceptable.

Also ToEnclosingRect is not the only error source here. For example, line
427~429 could all introduce errors, not just in width/height but also x/y. (We
may even need fix-up for left_ and top_ too. I'll need to think about this
more.)

IMO doing fix-up here is the right thing to do, but I think this could be more
numerical robust. The most important invariant is to have iter->geometry_rect()
to cover coverage_rect. As content space and coverage space mapping can
introduce error, it is possible that a content_rect full coverage doesn't map to
coverage_rect full coverage.

I think we can do this: Refactor line 499~537 to a helper function (computes
canonical geometry rect for tile(i, j)). Then doing the fix-up here in coverage
space instead.

[vmpstr, 2017-04-20 19:53:21]

Please take a look. I've changed the implementation a bit to do the comparison
in the right space. That is, operator++ becomes a loop that skips empty geometry
rects if we encounter them. This is equivalent to fixing them up in the ctor,
but more correct (since some off by one errors might creep up when we fix up in
the ctor)

[vmpstr, 2017-04-27 17:34:50]

ping

[trchen, 2017-04-27 18:18:14]

My apologies, didn't notice it. The new approach lgtm.

The loop looks a bit awkward though, but IMO still very readable and can't think
of any better. What do you think, enne@?

[enne (OOO), 2017-04-27 18:22:20]

lgtm

[vmpstr, 2017-04-27 18:58:53]

The CQ bit was checked by vmpstr@chromium.org

[commit-bot: I haz the power, 2017-04-27 18:59:22]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-27 19:34:19]

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

[commit-bot: I haz the power, 2017-04-27 19:34:20]

Try jobs failed on following builders:
  linux_trusty_blink_rel on master.tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_trusty_blink_rel/b...)

[vmpstr, 2017-04-28 16:44:37]

The CQ bit was checked by vmpstr@chromium.org

[commit-bot: I haz the power, 2017-04-28 16:45:16]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-28 18:47:42]

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

[commit-bot: I haz the power, 2017-04-28 18:47:43]

Try jobs failed on following builders:
  linux_trusty_blink_rel on master.tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_trusty_blink_rel/b...)

[vmpstr, 2017-05-01 17:57:50]

The CQ bit was checked by vmpstr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-01 17:58:23]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-01 19:44:10]

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

[commit-bot: I haz the power, 2017-05-01 19:44:11]

Dry run: This issue passed the CQ dry run.

[vmpstr, 2017-05-01 20:20:04]

I've had to rebaseline some tests. I believe this is due to the different
scaling in the ctor. Please take a look to see if this is acceptable.

[enne (OOO), 2017-05-01 20:22:29]

I think the rebaseline looks not worrisome, but I'm kind of surprised anything
would change in a device scale = 1 world.  Can you explain a little bit further?

[vmpstr, 2017-05-01 20:32:56]

On 2017/05/01 20:22:29, enne wrote:
> I think the rebaseline looks not worrisome, but I'm kind of surprised anything
> would change in a device scale = 1 world.  Can you explain a little bit
further?

Previously we did

X * (1.f / X); now, we're doing X / X, which is a slightly different float. But
I agree that for the scales used in the layout tests, this shouldn't make a
different float... I'm going to upload a patch without that part of the change
to confirm that that's the difference.

[vmpstr, 2017-05-08 20:54:24]

Sorry, this kind of dropped off. It seems that undoing the scale changes does
"fix" the layout tests. So, I'm going to undo the last ps and land the layout
changes as well.

[vmpstr, 2017-05-08 20:54:30]

Patchset #5 (id:80001) has been deleted

[vmpstr, 2017-05-08 20:54:42]

The CQ bit was checked by vmpstr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-08 20:55:38]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-09 01:21:33]

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

[commit-bot: I haz the power, 2017-05-09 01:21:36]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[vmpstr, 2017-05-09 16:48:45]

The CQ bit was checked by vmpstr@chromium.org

[vmpstr, 2017-05-09 16:48:45]

The patchset sent to the CQ was uploaded after l-g-t-m from trchen@chromium.org,
enne@chromium.org
Link to the patchset: https://codereview.chromium.org/2816943004/#ps60001
(title: "layouttests")

[vmpstr, 2017-05-09 16:48:48]

The CQ bit was unchecked by vmpstr@chromium.org

[vmpstr, 2017-05-09 21:50:20]

The CQ bit was checked by vmpstr@chromium.org

[commit-bot: I haz the power, 2017-05-09 21:51:36]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-09 23:54:00]

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

[commit-bot: I haz the power, 2017-05-09 23:54:01]

Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has
not started yet; builder either lacks capacity or does not exist (misspelled?))
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet;
builder either lacks capacity or does not exist (misspelled?))
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does
not exist (misspelled?))
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, build
has not started yet; builder either lacks capacity or does not exist
(misspelled?))
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
build has not started yet; builder either lacks capacity or does not exist
(misspelled?))
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does
not exist (misspelled?))
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does
not exist (misspelled?))
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does
not exist (misspelled?))
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
build has not started yet; builder either lacks capacity or does not exist
(misspelled?))

[vmpstr, 2017-05-11 20:36:52]

The CQ bit was checked by vmpstr@chromium.org

[commit-bot: I haz the power, 2017-05-11 20:38:10]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-11 22:21:58]

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

[commit-bot: I haz the power, 2017-05-11 22:22:00]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[vmpstr, 2017-05-12 16:00:40]

The CQ bit was checked by vmpstr@chromium.org

[commit-bot: I haz the power, 2017-05-12 16:01:09]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-12 17:00:32]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1494604840410000,
"parent_rev": "7794609a04bdf904491f8d9501f6cd5ecf39adde", "commit_rev":
"7ff1f0e4e9ad3eef8f4a1e36c08b95237a1c4dcf"}

[commit-bot: I haz the power, 2017-05-12 17:00:45]

Description was changed from

==========
cc: Fix bugs found by fuzzer due to floating point imprecision.

This patch changes two things:
- For scales that are within epsilon value of 1, we use 1 directly since
minute differences could cause changes when multiplied by large values
- Changed the index generation to consider the fact that using wanted
pixels might grab more tiles than needed.

BUG=707430
R=enne@chromium.org, danakj@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
cc: Fix bugs found by fuzzer due to floating point imprecision.

This patch changes two things:
- For scales that are within epsilon value of 1, we use 1 directly since
minute differences could cause changes when multiplied by large values
- Changed the index generation to consider the fact that using wanted
pixels might grab more tiles than needed.

BUG=707430
R=enne@chromium.org, danakj@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2816943004
Cr-Commit-Position: refs/heads/master@{#471342}
Committed:
https://chromium.googlesource.com/chromium/src/+/7ff1f0e4e9ad3eef8f4a1e36c08b...
==========

[commit-bot: I haz the power, 2017-05-12 17:00:46]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/7ff1f0e4e9ad3eef8f4a1e36c08b...