Consider subpixel accumulation when invalidating paint

Consider subpixel accumulation when invalidating paint

Pass repaint rect as LayoutRect to
- RenderObject::invalidatePaintUsingContainer(),
- RenderLayerRepainter::setBackingNeedsRepaintInRect()
- CompositedLayerMapping::setContentsNeedDisplayInRect()
and add subpixel accumulation before passing to GraphicsLayer.

BUG=390605
TEST=fast/sub-pixel/repaint-subpixel-layer-in-subpixel-composited-layer.html
R=leviw@chromium.org

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=177408

[Xianzhu, 2014-07-01 23:02:29]

PTAL.

[leviw_travelin_and_unemployed, 2014-07-01 23:37:10]

LGTM. Thank you :)

[commit-bot: I haz the power, 2014-07-01 23:37:55]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/wangxianzhu@chromium.org/353403006/20001

[Xianzhu, 2014-07-02 00:32:18]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2014-07-02 00:32:21]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/wangxianzhu@chromium.org/353403006/40001

[Xianzhu, 2014-07-02 00:43:41]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2014-07-02 00:43:52]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/wangxianzhu@chromium.org/353403006/60001

[commit-bot: I haz the power, 2014-07-02 01:50:58]

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

[commit-bot: I haz the power, 2014-07-02 02:37:58]

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

[commit-bot: I haz the power, 2014-07-02 02:37:59]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/14957)

[Xianzhu, 2014-07-02 03:47:41]

The CQ bit was checked by wangxianzhu@chromium.org

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

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

[chrishtr, 2014-07-02 04:39:58]

https://codereview.chromium.org/353403006/diff/80001/Source/core/rendering/Re...
File Source/core/rendering/RenderLayerRepainter.cpp (right):

https://codereview.chromium.org/353403006/diff/80001/Source/core/rendering/Re...
Source/core/rendering/RenderLayerRepainter.cpp:129: // FIXME: how about subpixel
accumulation?
Instead of a non-squashing specific fix, please change this CL to do the
following before line 128:

r.move(renderer()->subpixelAccumulation());


Then you can remove the FIXME and the subpixelAccumulation part of
CompositedLayerMapping.cpp:1928.

[chrishtr, 2014-07-02 04:40:24]

The CQ bit was unchecked by chrishtr@chromium.org

[chrishtr, 2014-07-02 04:40:46]

I removed the commit bit, hope you don't mind making that change.

[Xianzhu, 2014-07-02 16:19:10]

https://codereview.chromium.org/353403006/diff/80001/Source/core/rendering/Re...
File Source/core/rendering/RenderLayerRepainter.cpp (right):

https://codereview.chromium.org/353403006/diff/80001/Source/core/rendering/Re...
Source/core/rendering/RenderLayerRepainter.cpp:129: // FIXME: how about subpixel
accumulation?
On 2014/07/02 04:39:58, chrishtr wrote:
> Instead of a non-squashing specific fix, please change this CL to do the
> following before line 128:
> 
> r.move(renderer()->subpixelAccumulation());
> 
> 
> Then you can remove the FIXME and the subpixelAccumulation part of
> CompositedLayerMapping.cpp:1928.

Thanks for the solution! Done.

[Xianzhu, 2014-07-02 16:19:15]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2014-07-02 16:19:20]

The CQ bit was unchecked by wangxianzhu@chromium.org

[Xianzhu, 2014-07-02 16:21:46]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2014-07-02 16:22:27]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/wangxianzhu@chromium.org/353403006/100001

[chrishtr, 2014-07-02 16:59:24]

https://codereview.chromium.org/353403006/diff/100001/Source/core/rendering/c...
File Source/core/rendering/compositing/CompositedLayerMapping.cpp (right):

https://codereview.chromium.org/353403006/diff/100001/Source/core/rendering/c...
Source/core/rendering/compositing/CompositedLayerMapping.cpp:1928:
pixelSnappedIntRect(r.location() + m_owningLayer.subpixelAccumulation(),
r.size())
It's not quite what I had in mind. Instead of using
m_owningLayer.subpixelAccumulation() here,
you can use m_renderer.layer()->subpixelAccumulation() for both the squashing
and non-squashing
case in RenderLayerRepainter.

For non-squashed laeyrs, m_renderer.layer() will be the same as m_owningLayer

[Xianzhu, 2014-07-02 17:17:54]

https://codereview.chromium.org/353403006/diff/100001/Source/core/rendering/c...
File Source/core/rendering/compositing/CompositedLayerMapping.cpp (right):

https://codereview.chromium.org/353403006/diff/100001/Source/core/rendering/c...
Source/core/rendering/compositing/CompositedLayerMapping.cpp:1928:
pixelSnappedIntRect(r.location() + m_owningLayer.subpixelAccumulation(),
r.size())
On 2014/07/02 16:59:24, chrishtr wrote:
> It's not quite what I had in mind. Instead of using
> m_owningLayer.subpixelAccumulation() here,
> you can use m_renderer.layer()->subpixelAccumulation() for both the squashing
> and non-squashing
> case in RenderLayerRepainter.
> 
> For non-squashed laeyrs, m_renderer.layer() will be the same as m_owningLayer

I think we should keep 'adjusting of rects with subpixel accumulation' only
above the GraphicsLayer interface, otherwise it's difficult to know which
LayoutRect contains subpixel accumulation which doesn't.

There is a FIXME in RenderLayerRepainter::setBackingNeedsRepaintInRect saying
that we should combine the squashing and non-squashing path. When it is fixed,
the subpixel accumulation adjustment code will be also combined.

[chrishtr, 2014-07-02 17:24:25]

Ok, fair enough.

[commit-bot: I haz the power, 2014-07-02 17:36:47]

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

[commit-bot: I haz the power, 2014-07-02 18:19:21]

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

[commit-bot: I haz the power, 2014-07-02 18:19:22]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/15113)

[Xianzhu, 2014-07-02 18:26:21]

Committed patchset #5 manually as r177408 (presubmit successful).