Closed Bug 1471639 Opened 2 years ago Closed Last year

Move tile edge padding to work on the paint thread

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: rhunt, Assigned: rhunt)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [gfx-noted])

Attachments

(2 files)

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 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 on attachment 8992769 [details]
Bug 1471639 - Move edge padding to the paint thread.

https://reviewboard.mozilla.org/r/257612/#review264630
Attachment #8992769 - Flags: review?(nical.bugzilla) → review+
Comment on attachment 8992770 [details]
Bug 1471639 - Allow OMTP with edge-padding.

https://reviewboard.mozilla.org/r/257614/#review264632
Attachment #8992770 - Flags: review?(nical.bugzilla) → review+
Pushed by rhunt@eqrion.net:
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
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=rhunt@eqrion.net&selectedJob=188646094
https://hg.mozilla.org/mozilla-central/rev/299faea63d79
https://hg.mozilla.org/mozilla-central/rev/a1175f58a6a5
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Depends on: 1477226
No longer depends on: 1477226
You need to log in before you can comment on or make changes to this bug.