First round of optimizations for box shadows. #1954

glennw · 2017-10-27T06:47:05Z

This applies two optimizations to outset box shadows:

If border radii are uniform, render only the top left corner
of the box shadow, and mirror that across the primitive.
If border radii are non-uniform, render a minimal mask
rect and stretch that across the primitive.

There are several remaining box shadow / clip related optimizations
to come, but this provides some large immediate wins on large
outset box shadows.

Fixes #1571.

This change is

glennw · 2017-10-27T06:47:13Z

r? @kvark

glennw · 2017-10-27T06:50:37Z

@jrmuizel This provides a large performance win on https://bugzilla.mozilla.org/show_bug.cgi?id=1408612.

In the profile bars below, the pink segments are blurs for box shadows. They are an almost negligible amount of time with this optimization.

On my HD4600, this drops the GPU time on bugzilla from ~20ms to ~8ms. The performance with this fix is reasonable now, but there's three major issues outstanding:

No z-buffer wins (known issue being worked on in WR, due to Gecko not using local clips).
Way too many clip masks being drawn (still nearly 3 2048x2048 A8 targets being filled per frame - the purple bars). Need to investigate this more - probably some issues in both Gecko and WR here.
High number of draw calls / bad batching. Needs a bit of investigation - it's worse than I'd expect. Will be resolved anyway as we unify more shaders to brush types.

Follow ups:

Optimize inset shadows.
Specifiy render task cache key for blurs - this will save doing the same blur on different primitives.

glennw · 2017-10-27T06:51:00Z

Pending gecko try:

https://hg.mozilla.org/try/rev/92d0786b9e62d6b6e5ddbb741b1628e17057aaa5
https://treeherder.mozilla.org/#/jobs?repo=try&revision=92d0786b9e62d6b6e5ddbb741b1628e17057aaa5

glennw · 2017-10-27T06:51:19Z

Servo WPT sanity tests pass, I need to do a full run through.

kvark · 2017-10-27T15:58:04Z

Amazing work 👍 ! I just got a few concerns to address.

Reviewed 15 of 15 files at r1.
Review status: all files reviewed at latest revision, 7 unresolved discussions.

webrender/res/brush_image.glsl, line 90 at r1 (raw file):

        }
        case BRUSH_IMAGE_NINEPATCH: {
            uv = clamp(vUv.xy, vec2(0.0), vParams.xy);

this seems incorrect, given that vParams.xy == vec2(0.5) at this point

webrender/res/brush_image.glsl, line 98 at r1 (raw file):

            // Mirror and stretch the box shadow corner over the entire
            // primitives.
            uv = vParams.xy - abs(vUv.xy - vParams.xy);

given that vParams.xy is half-size of the local rect, this would produce uv between the local rect origin and it's half-size. It doesn't appear to be desired?

webrender/res/brush_mask_rounded_rect.glsl, line 72 at r1 (raw file):

    // be made much faster.

    // NOTE: The AA range must be computed outside the if statement,

I think we should rather move the AA stuff out of rounded_rect (than call rounded_rect all the time), so that we can avoid some work here if we are outside of the clip region.

webrender/src/box_shadow.rs, line 20 at r1 (raw file):

// mask. This ensures that we get a few pixels past the corner that can be
// blurred without being affected by the border radius.
pub const MASK_CORNER_PADDING: f32 = 4.0;

hmm, should this depend on the border radius then?

webrender/src/picture.rs, line 41 at r1 (raw file):

        blur_regions: Vec<LayerRect>,
        clip_mode: BoxShadowClipMode,
        has_uniform_radii: bool,

would be nice to have it as an enum for better calling semantics

webrender/src/picture.rs, line 142 at r1 (raw file):

            PictureKind::BoxShadow { blur_radius, clip_mode, has_uniform_radii, .. } => {
                // We need to inflate the content rect if outset.
                if clip_mode == BoxShadowClipMode::Outset {

nit: match would be more clear

webrender/src/prim_store.rs, line 190 at r1 (raw file):

    fn write_gpu_blocks(&self, mut request: GpuDataRequest) {
        match self.kind {
            BrushKind::Mask { clip_mode, ref kind } => {

nit: given the paths are so different, might as well match BrushMaskKind in place here, i.e. BrushKind::Mask { clip_mode, kind: BrushMaskKind::Corner(radius) } => {...}

Comments from Reviewable


        First round of optimizations for box shadows.

This applies two optimizations to outset box shadows: * If border radii are uniform, render only the top left corner of the box shadow, and mirror that across the primitive. * If border radii are non-uniform, render a minimal mask rect and stretch that across the primitive. There are several remaining box shadow / clip related optimizations to come, but this provides some large immediate wins on large outset box shadows. Fixes #1571.


        Address review comments.

glennw · 2017-10-29T22:08:39Z

Review status: 9 of 17 files reviewed at latest revision, 7 unresolved discussions.

webrender/res/brush_image.glsl, line 90 at r1 (raw file):