1471639 - Move tile edge padding to work on the paint thread

Status: RESOLVED FIXED

(Core :: Graphics: Layers, enhancement, P3)

Description

[Ryan Hunt [:rhunt]]

On android we have tile edge padding enabled to prevent sampling artifacts. [1]

This operation happens after we paint and needs to be ported to the paint thread to work properly with OMTP.

[1] https://searchfox.org/mozilla-central/rev/14cb8f1c238735ba1abe18ad44e39808983c2572/gfx/layers/client/MultiTiledContentClient.cpp#328

Comment 1

[Ryan Hunt [:rhunt]]

Attachment: Bug 1471639 - Move edge padding to the paint thread.

This commit ports over the last remaining operation for tiling that doesn't work
on the paint thread.

The difficult part for edge padding is that it is done outside of ValidateTile
so it doesn't have an associated CapturedTilePaintState to be added to as an
operation. We need it to be in the same paint state so that it's guaranteed
to be run after painting has finished.

This commit changes edge padding to instead be decided inside of ValidateTile
and either sent to the paint thread if there is OMTP or executed right away.

Review commit: https://reviewboard.mozilla.org/r/257612/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/257612/

Comment 2

[Ryan Hunt [:rhunt]]

Attachment: Bug 1471639 - Allow OMTP with edge-padding.

Review commit: https://reviewboard.mozilla.org/r/257614/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/257614/

Comment 3

[Code Review Bot [:reviewbot]]

Comment on attachment 8992769 [details]
Bug 1471639 - Move edge padding to the paint thread.

https://reviewboard.mozilla.org/r/257612/#review264518


Code analysis found 3 defects in this patch:
 - 3 defects found by clang-tidy

You can run this analysis locally with:
 - `./mach static-analysis check path/to/file.cpp` (C/C++)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: gfx/layers/PaintThread.h:229
(Diff revision 1)
>    };
>  
> +  struct EdgePad {
> +    EdgePad(RefPtr<gfx::DrawTarget> aTarget,
> +            nsIntRegion&& aValidRegion)
> +      : mTarget(aTarget)

Warning: Parameter 'aTarget' is passed by value and only copied once; consider moving it to avoid unnecessary copies [clang-tidy: performance-unnecessary-value-param]

      : mTarget(aTarget)
                ^~~~~~~~
                std::move()

::: gfx/layers/PaintThread.h:229
(Diff revision 1)
>    };
>  
> +  struct EdgePad {
> +    EdgePad(RefPtr<gfx::DrawTarget> aTarget,
> +            nsIntRegion&& aValidRegion)
> +      : mTarget(aTarget)

Warning: Parameter 'aTarget' is passed by value and only copied once; consider moving it to avoid unnecessary copies [clang-tidy: performance-unnecessary-value-param]

      : mTarget(aTarget)
                ^~~~~~~~
                std::move()

::: gfx/layers/PaintThread.h:229
(Diff revision 1)
>    };
>  
> +  struct EdgePad {
> +    EdgePad(RefPtr<gfx::DrawTarget> aTarget,
> +            nsIntRegion&& aValidRegion)
> +      : mTarget(aTarget)

Warning: Parameter 'aTarget' is passed by value and only copied once; consider moving it to avoid unnecessary copies [clang-tidy: performance-unnecessary-value-param]

      : mTarget(aTarget)
                ^~~~~~~~
                std::move()

Comment 4

[Nicolas Silva [:nical]]

Comment on attachment 8992769 [details]
Bug 1471639 - Move edge padding to the paint thread.

https://reviewboard.mozilla.org/r/257612/#review264630

Comment 5

[Nicolas Silva [:nical]]

Comment on attachment 8992770 [details]
Bug 1471639 - Allow OMTP with edge-padding.

https://reviewboard.mozilla.org/r/257614/#review264632

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/299faea63d79
Move edge padding to the paint thread. r=nical
https://hg.mozilla.org/integration/mozilla-inbound/rev/a1175f58a6a5
Allow OMTP with edge-padding. r=nical

Comment 7

[Ryan Hunt [:rhunt]]

I did a try run with this patch in bug 1467151. I also did a try run of building on all platforms to catch unified build bustage from adding a file [1].

[1] https://treeherder.mozilla.org/#/jobs?repo=try&author=[e-mail]&selectedJob=188646094

Comment 8

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/299faea63d79
https://hg.mozilla.org/mozilla-central/rev/a1175f58a6a5