Revert of Don't expand empty rect during rect mapping

Revert of Don't expand empty rect during rect mapping (patchset #4 id:60001 of https://codereview.chromium.org/2416693002/ )

Reason for revert:
It changed
  transform->mapRect(enclosingIntRect(rect))
to
  transform->mapRect(FloatRect(enclosingIntRect(rect)))
causing missing enclosingIntRect() after the transform.

We may also need enclosingIntRect for empty rects because even empty
object can have visible filter depending on pixel snapping here.

BUG=669774
TEST=paint/invalidation/rotated-subpixel.html

Original issue's description:
> Don't expand empty rect during rect mapping
>
> For example, enclosingIntRect(LayoutRect(0.5, 0.5, 0, 0)) results
> IntRect(0, 0, 1, 1). This causes unnecessary non-empty visual rects
> mapped from empty LayoutRects.
>
> This CL is mainly to reduce the difference between the old path and
> slimmingPaintInvalidation with pixel snapping feature.
>
> Committed: https://crrev.com/67c516aaed2486295a98d7f0ff2235b921cd7933
> Cr-Commit-Position: refs/heads/master@{#424970}

TBR=chrishtr@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.

Committed: https://crrev.com/4774275d950261bf9ef99c55b326979298b5c558
Cr-Commit-Position: refs/heads/master@{#435677}

[Xianzhu, 2016-11-30 20:10:02]

Created Revert of Don't expand empty rect during rect mapping

[Xianzhu, 2016-11-30 20:17:46]

Patchset #2 (id:190001) has been deleted

[Xianzhu, 2016-11-30 20:18:21]

Description was changed from

==========
Revert of Don't expand empty rect during rect mapping (patchset #4 id:60001 of
https://codereview.chromium.org/2416693002/ )

Reason for revert:
It changed
  transform->mapRect(enclosingIntRect(rect))
to
  transform->mapRect(FloatRect(enclosingIntRect(rect)))
causing missing enclosingIntRect() after the transform.

We may also need enclosingIntRect for empty rects because even empty
object can have visible filter depending on pixel snapping here.

BUG=669774

Original issue's description:
> Don't expand empty rect during rect mapping
> 
> For example, enclosingIntRect(LayoutRect(0.5, 0.5, 0, 0)) results
> IntRect(0, 0, 1, 1). This causes unnecessary non-empty visual rects
> mapped from empty LayoutRects.
> 
> This CL is mainly to reduce the difference between the old path and
> slimmingPaintInvalidation with pixel snapping feature.
> 
> Committed: https://crrev.com/67c516aaed2486295a98d7f0ff2235b921cd7933
> Cr-Commit-Position: refs/heads/master@{#424970}

TBR=chrishtr@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
==========

to

==========
Revert of Don't expand empty rect during rect mapping (patchset #4 id:60001 of
https://codereview.chromium.org/2416693002/ )

Reason for revert:
It changed
  transform->mapRect(enclosingIntRect(rect))
to
  transform->mapRect(FloatRect(enclosingIntRect(rect)))
causing missing enclosingIntRect() after the transform.

We may also need enclosingIntRect for empty rects because even empty
object can have visible filter depending on pixel snapping here.

BUG=669774
TEST=paint/invalidation/rotated-subpixel.html

Original issue's description:
> Don't expand empty rect during rect mapping
> 
> For example, enclosingIntRect(LayoutRect(0.5, 0.5, 0, 0)) results
> IntRect(0, 0, 1, 1). This causes unnecessary non-empty visual rects
> mapped from empty LayoutRects.
> 
> This CL is mainly to reduce the difference between the old path and
> slimmingPaintInvalidation with pixel snapping feature.
> 
> Committed: https://crrev.com/67c516aaed2486295a98d7f0ff2235b921cd7933
> Cr-Commit-Position: refs/heads/master@{#424970}

TBR=chrishtr@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
==========

[Xianzhu, 2016-11-30 20:18:39]

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

[Xianzhu, 2016-11-30 20:19:02]

Ptal mainly the new test paint/invalidation/rotated-subpixel.html

[commit-bot: I haz the power, 2016-11-30 20:19:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[chrishtr, 2016-11-30 20:27:36]

lgtm

[Xianzhu, 2016-11-30 20:32:35]

The CQ bit was unchecked by wangxianzhu@chromium.org

[Xianzhu, 2016-11-30 20:32:39]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2016-11-30 20:33:08]

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, 2016-11-30 22:03:33]

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

[commit-bot: I haz the power, 2016-11-30 22:03:34]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Xianzhu, 2016-11-30 22:11:21]

The new test failed on Mac and Windows because of slight blended pixels around
the red box.

Chris, do you think we should expand the transformed rect by 1 more pixel to
cover the blended pixels?

[chrishtr, 2016-11-30 22:13:40]

On 2016/11/30 at 22:11:21, wangxianzhu wrote:
> The new test failed on Mac and Windows because of slight blended pixels around
the red box.
> 
> Chris, do you think we should expand the transformed rect by 1 more pixel to
cover the blended pixels?

We didn't used to have that, right? Did the CL you're reverting rebaseline these
tests somehow?

[Xianzhu, 2016-11-30 22:19:11]

On 2016/11/30 22:13:40, chrishtr wrote:
> On 2016/11/30 at 22:11:21, wangxianzhu wrote:
> > The new test failed on Mac and Windows because of slight blended pixels
around
> the red box.
> > 
> > Chris, do you think we should expand the transformed rect by 1 more pixel to
> cover the blended pixels?
> 
> We didn't used to have that, right? Did the CL you're reverting rebaseline
these
> tests somehow?

paint/invalidation/rotated-subpixel.html is a new test added in this CL. Even
enclosingIntRect before and after transform can't cover all changed pixels in
the test.

The other two failures need rebaseline.

[chrishtr, 2016-11-30 22:22:55]

On 2016/11/30 at 22:19:11, wangxianzhu wrote:
> On 2016/11/30 22:13:40, chrishtr wrote:
> > On 2016/11/30 at 22:11:21, wangxianzhu wrote:
> > > The new test failed on Mac and Windows because of slight blended pixels
around
> > the red box.
> > > 
> > > Chris, do you think we should expand the transformed rect by 1 more pixel
to
> > cover the blended pixels?
> > 
> > We didn't used to have that, right? Did the CL you're reverting rebaseline
these
> > tests somehow?
> 
> paint/invalidation/rotated-subpixel.html is a new test added in this CL. Even
enclosingIntRect before and after transform can't cover all changed pixels in
the test.
> 
> The other two failures need rebaseline.

I see, didn't look closely enough at where the test came from, sorry.

Do you know exactly why it doesn't contain all changed pixels? Is it composited
blending as far as you can tell?

[Xianzhu, 2016-11-30 22:36:28]

On 2016/11/30 22:22:55, chrishtr wrote:
> On 2016/11/30 at 22:19:11, wangxianzhu wrote:
> > On 2016/11/30 22:13:40, chrishtr wrote:
> > > On 2016/11/30 at 22:11:21, wangxianzhu wrote:
> > > > The new test failed on Mac and Windows because of slight blended pixels
> around
> > > the red box.
> > > > 
> > > > Chris, do you think we should expand the transformed rect by 1 more
pixel
> to
> > > cover the blended pixels?
> > > 
> > > We didn't used to have that, right? Did the CL you're reverting rebaseline
> these
> > > tests somehow?
> > 
> > paint/invalidation/rotated-subpixel.html is a new test added in this CL.
Even
> enclosingIntRect before and after transform can't cover all changed pixels in
> the test.
> > 
> > The other two failures need rebaseline.
> 
> I see, didn't look closely enough at where the test came from, sorry.
> 
> Do you know exactly why it doesn't contain all changed pixels? Is it
composited
> blending as far as you can tell?

The box is not composited. The extra pixels look like an anti-alias effect.

[chrishtr, 2016-11-30 22:41:27]

On 2016/11/30 at 22:36:28, wangxianzhu wrote:
> On 2016/11/30 22:22:55, chrishtr wrote:
> > On 2016/11/30 at 22:19:11, wangxianzhu wrote:
> > > On 2016/11/30 22:13:40, chrishtr wrote:
> > > > On 2016/11/30 at 22:11:21, wangxianzhu wrote:
> > > > > The new test failed on Mac and Windows because of slight blended
pixels
> > around
> > > > the red box.
> > > > > 
> > > > > Chris, do you think we should expand the transformed rect by 1 more
pixel
> > to
> > > > cover the blended pixels?
> > > > 
> > > > We didn't used to have that, right? Did the CL you're reverting
rebaseline
> > these
> > > > tests somehow?
> > > 
> > > paint/invalidation/rotated-subpixel.html is a new test added in this CL.
Even
> > enclosingIntRect before and after transform can't cover all changed pixels
in
> > the test.
> > > 
> > > The other two failures need rebaseline.
> > 
> > I see, didn't look closely enough at where the test came from, sorry.
> > 
> > Do you know exactly why it doesn't contain all changed pixels? Is it
composited
> > blending as far as you can tell?
> 
> The box is not composited. The extra pixels look like an anti-alias effect.

Right...

Let me ask Florin and get back to you.

[chrishtr, 2016-11-30 22:45:25]

chrishtr@chromium.org changed reviewers:
+ fmalita@chromium.org

[chrishtr, 2016-11-30 22:45:25]

Florin, can you take a look at the rebaseline failure for
paint/invalidation/rotated-subpixel.html and comment on why we seem to be
missing a pixel
even if using enclosingIntRect after the transform?

[Xianzhu, 2016-11-30 23:38:05]

The value of the extra pixels is #fefefe (#ffffff expected). As this is almost
invisible to human eyes, I would like to change the test to a normal pixel test
instead of a ref test and generate new baselines, mark the bug Fixed, and file a
new bug for the extra pixels. Wdyt?

[Xianzhu, 2016-11-30 23:47:29]

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

[commit-bot: I haz the power, 2016-11-30 23:47:56]

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, 2016-12-01 01:51:16]

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

[commit-bot: I haz the power, 2016-12-01 01:51:17]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Xianzhu, 2016-12-01 05:27:15]

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

[commit-bot: I haz the power, 2016-12-01 05:27:22]

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, 2016-12-01 07:03:13]

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

[commit-bot: I haz the power, 2016-12-01 07:03:14]

Dry run: This issue passed the CQ dry run.

[Xianzhu, 2016-12-01 18:13:31]

Chris, what do you think about landing the patch (with the new test as a pixel
test) and filing a bug about the #fefefe pixel overflow?

[chrishtr, 2016-12-01 19:07:50]

lgtm

Ok. In parallel, Florin is looking into the issue. Please file the bug against
him.

[Xianzhu, 2016-12-01 19:24:36]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2016-12-01 19:25: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, 2016-12-01 19:30:01]

CQ is committing da patch.

Bot data: {"patchset_id": 250001, "attempt_start_ts": 1480620276754470,
"parent_rev": "877b010a60fde4ab19b08a108be5242ba245c05a", "commit_rev":
"fcbf3ef3492eb0087c24ebf7b9b8ec230866713c"}

[commit-bot: I haz the power, 2016-12-01 19:30:36]

Description was changed from

==========
Revert of Don't expand empty rect during rect mapping (patchset #4 id:60001 of
https://codereview.chromium.org/2416693002/ )

Reason for revert:
It changed
  transform->mapRect(enclosingIntRect(rect))
to
  transform->mapRect(FloatRect(enclosingIntRect(rect)))
causing missing enclosingIntRect() after the transform.

We may also need enclosingIntRect for empty rects because even empty
object can have visible filter depending on pixel snapping here.

BUG=669774
TEST=paint/invalidation/rotated-subpixel.html

Original issue's description:
> Don't expand empty rect during rect mapping
> 
> For example, enclosingIntRect(LayoutRect(0.5, 0.5, 0, 0)) results
> IntRect(0, 0, 1, 1). This causes unnecessary non-empty visual rects
> mapped from empty LayoutRects.
> 
> This CL is mainly to reduce the difference between the old path and
> slimmingPaintInvalidation with pixel snapping feature.
> 
> Committed: https://crrev.com/67c516aaed2486295a98d7f0ff2235b921cd7933
> Cr-Commit-Position: refs/heads/master@{#424970}

TBR=chrishtr@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
==========

to

==========
Revert of Don't expand empty rect during rect mapping (patchset #4 id:60001 of
https://codereview.chromium.org/2416693002/ )

Reason for revert:
It changed
  transform->mapRect(enclosingIntRect(rect))
to
  transform->mapRect(FloatRect(enclosingIntRect(rect)))
causing missing enclosingIntRect() after the transform.

We may also need enclosingIntRect for empty rects because even empty
object can have visible filter depending on pixel snapping here.

BUG=669774
TEST=paint/invalidation/rotated-subpixel.html

Original issue's description:
> Don't expand empty rect during rect mapping
> 
> For example, enclosingIntRect(LayoutRect(0.5, 0.5, 0, 0)) results
> IntRect(0, 0, 1, 1). This causes unnecessary non-empty visual rects
> mapped from empty LayoutRects.
> 
> This CL is mainly to reduce the difference between the old path and
> slimmingPaintInvalidation with pixel snapping feature.
> 
> Committed: https://crrev.com/67c516aaed2486295a98d7f0ff2235b921cd7933
> Cr-Commit-Position: refs/heads/master@{#424970}

TBR=chrishtr@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
==========

[commit-bot: I haz the power, 2016-12-01 19:30:38]

Committed patchset #4 (id:250001)

[commit-bot: I haz the power, 2016-12-01 19:34:01]

Description was changed from

==========
Revert of Don't expand empty rect during rect mapping (patchset #4 id:60001 of
https://codereview.chromium.org/2416693002/ )

Reason for revert:
It changed
  transform->mapRect(enclosingIntRect(rect))
to
  transform->mapRect(FloatRect(enclosingIntRect(rect)))
causing missing enclosingIntRect() after the transform.

We may also need enclosingIntRect for empty rects because even empty
object can have visible filter depending on pixel snapping here.

BUG=669774
TEST=paint/invalidation/rotated-subpixel.html

Original issue's description:
> Don't expand empty rect during rect mapping
> 
> For example, enclosingIntRect(LayoutRect(0.5, 0.5, 0, 0)) results
> IntRect(0, 0, 1, 1). This causes unnecessary non-empty visual rects
> mapped from empty LayoutRects.
> 
> This CL is mainly to reduce the difference between the old path and
> slimmingPaintInvalidation with pixel snapping feature.
> 
> Committed: https://crrev.com/67c516aaed2486295a98d7f0ff2235b921cd7933
> Cr-Commit-Position: refs/heads/master@{#424970}

TBR=chrishtr@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
==========

to

==========
Revert of Don't expand empty rect during rect mapping (patchset #4 id:60001 of
https://codereview.chromium.org/2416693002/ )

Reason for revert:
It changed
  transform->mapRect(enclosingIntRect(rect))
to
  transform->mapRect(FloatRect(enclosingIntRect(rect)))
causing missing enclosingIntRect() after the transform.

We may also need enclosingIntRect for empty rects because even empty
object can have visible filter depending on pixel snapping here.

BUG=669774
TEST=paint/invalidation/rotated-subpixel.html

Original issue's description:
> Don't expand empty rect during rect mapping
>
> For example, enclosingIntRect(LayoutRect(0.5, 0.5, 0, 0)) results
> IntRect(0, 0, 1, 1). This causes unnecessary non-empty visual rects
> mapped from empty LayoutRects.
>
> This CL is mainly to reduce the difference between the old path and
> slimmingPaintInvalidation with pixel snapping feature.
>
> Committed: https://crrev.com/67c516aaed2486295a98d7f0ff2235b921cd7933
> Cr-Commit-Position: refs/heads/master@{#424970}

TBR=chrishtr@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.

Committed: https://crrev.com/4774275d950261bf9ef99c55b326979298b5c558
Cr-Commit-Position: refs/heads/master@{#435677}
==========

[commit-bot: I haz the power, 2016-12-01 19:34:02]

Patchset 4 (id:??) landed as
https://crrev.com/4774275d950261bf9ef99c55b326979298b5c558
Cr-Commit-Position: refs/heads/master@{#435677}