Closed Bug 1501073 Opened Last year Closed Last year

Size offscreen render targets more intelligently

Categories

(Core :: Graphics: WebRender, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox65 --- fixed

People

(Reporter: bholley, Assigned: bholley)

References

(Blocks 2 open bugs)

Details

Attachments

(4 files)

Right now, every offscreen render target is 2048x2048 at minimum, and larger if it has a primitive that doesn't fit in that region.

Long-term, this will get a lot better when we have full tiling support, since then we can just make every render target 512x512 and stop worrying about the sizing tradeoffs. That's a ways off though, and we can do a lot better in the mean time.

I have patches that shave ~100mb of render-target memory off an average memory report, and I'm hopeful they'll make a dent in our AWSY deficit.
The main constraint is that the dimensions in the viewport need to match
those in the ortho matrix.

This work paves the way for using render target textures that are larger
than the associated RenderTarget requires, by reducing our reliance on
the RenderTarget's |max_size| field.
Depends on D9434
(In reply to Bobby Holley (:bholley) from comment #3)
> Created attachment 9019153 [details]
> Bug 1501073 - Shrink render targets. r?kvark
> 
> Depends on D9434

This second patch caused a few subpixel reftest issues [1], which the patch in comment 2 does not trigger [2].

The failures are:

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/transform-3d/mask-layer-2.html == file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/transform-3d/mask-layer-ref.html | image comparison, max difference: 16, number of differing pixels: 100
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/transform-3d/mask-layer-3.html == file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/transform-3d/mask-layer-ref.html | image comparison, max difference: 16, number of differing pixels: 100

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/svg/svg-integration/clip-path/clip-path-polygon-013.html == file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/svg/svg-integration/clip-path/clip-path-stripes-003-ref.html | image comparison, max difference: 8, number of differing pixels: 20

The reftest analyzer shows these to be related to a few subpixel differences around the edge [3]. Glenn, is this issue likely to be analogous to [4]? If so, should I just mark the tests as fuzzy? It'd be helpful if you could write a few sentences to help me understand what's going on here, which I can then reference in the annotations if we do that.


[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=eaad9763a28d6881ad3d12bf3a1c2736bc6477b3
[2] https://treeherder.mozilla.org/#/jobs?repo=try&revision=67cc0251c253bc065b32adb0829e7baca292cf0c
[3] https://hg.mozilla.org/mozilla-central/raw-file/tip/layout/tools/reftest/reftest-analyzer.xhtml#logurl=https://queue.taskcluster.net/v1/task/FuDaD4FiR1KmSKfFHufFdw/runs/0/artifacts/public/logs/live_backing.log&only_show_unexpected=1
[4] https://github.com/servo/webrender/pull/3212#issuecomment-430832826
Flags: needinfo?(gwatson)
It's hard to know without a detailed investigation, but these don't seem like just simple subpixel accuracy differences to me. It seems like quite a large number of pixels, and the max difference is each channel is quite large (rasterizer subpixel accuracy differences are usually very small) . I think this probably deserves some further investigation to see what is going on. It might be exposing a snapping bug elsewhere, or similar?
Flags: needinfo?(gwatson)
Looks like the fuzziness actually only affects linux. I filed bug 1501421, and we can mark the tests as fuzzy on linux when they land.
Attachment #9019495 - Flags: review?(kats)
Comment on attachment 9019495 [details] [diff] [review]
Test expectation adjustments.

Review of attachment 9019495 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the fix below

::: layout/reftests/svg/svg-integration/clip-path/reftest.list
@@ +13,5 @@
>  == clip-path-polygon-009.html clip-path-square-002-ref.html
>  == clip-path-polygon-010.html clip-path-stripes-001-ref.html
>  == clip-path-polygon-011.html clip-path-stripes-001-ref.html
>  == clip-path-polygon-012.html clip-path-stripes-001-ref.html
> +fuzzy-if(webrender&&gtkWidget, 8-8, 20-20) fuzzy-if(skiaContent,0-1,0-20) fails-if(webrender&&!gtkWidget) == clip-path-polygon-013.html clip-path-stripes-003-ref.html

The spaces will make this fail to parse correctly. I can fix it when I import the patch for the WR update.
Attachment #9019495 - Flags: review?(kats) → review+
Depends on: 1501319
Depends on: 1501518
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/faaec607a74f
Test expectation adjustments. r=kats
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #8)
> The spaces will make this fail to parse correctly. I can fix it when I
> import the patch for the WR update.

I also had to move the new clause to after the skiaContent one in order for it to take precedence.
https://hg.mozilla.org/mozilla-central/rev/faaec607a74f
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Priority: -- → P2
From bug 1501319:

(In reply to Ionuț Goldan [:igoldan], Performance Sheriffing from comment #6)
> Big AWSY improvements thanks to this patch! \o/
> 
> == Change summary for alert #17104 (as of Tue, 23 Oct 2018 22:41:53 GMT) ==
> 
> Improvements:
> 
>  13%  Heap Unclassified linux64-qr opt stylo        270,342,638.64 ->
> 234,166,175.73
>   7%  Resident Memory windows10-64-qr opt stylo     743,486,639.50 ->
> 690,920,875.56
>   7%  Explicit Memory linux64-qr opt stylo          551,409,069.41 ->
> 512,646,735.85
>   5%  Resident Memory linux64-qr opt stylo          972,615,529.23 ->
> 925,883,616.28
> 
> For up to date results, see:
> https://treeherder.mozilla.org/perf.html#/alerts?id=17104
You need to log in before you can comment on or make changes to this bug.