Support dashed borders with new border path. #1175

glennw · 2017-04-28T02:51:52Z

Dashed border edges are handled by the standard ps_border_edge shader.

Dashed border corners instead create a clip mask for the border and
draw each dash stroke to the clip mask. The math for determining the correct
dash strokes is too complex to evaluate efficiently in a shader, doing
it this way allows us to support arbitrary kinds of dash strokes.

In the future, we will either specialize this to a border render task (to
avoid the clip mask target allocation for the whole border rect), or make
use of upcoming optimization work in the clip mask code to handle this
case more efficiently.

This will also serve as the basis for dotted border corners, which are also
too complex to evaluate inside a shader.

For now, we just treat everything as an ellipse when evaluating the dash
strokes. This gives the correct result for ellipses and circle radii,
but is very inefficient for circles. As a follow up, we will include
a fast path for corners where the x/y radii are equal.

Deciding the exact dash length etc is not well specified. The current
code is a rough approximation to what Gecko does. In the future we
can iterate on this and make it closer to Gecko's rules for dash length.

This change is

glennw · 2017-04-28T02:52:31Z

Example of what the clip mask looks like for border dashing:

glennw · 2017-04-28T02:52:57Z

r? @kvark

Sorry for another large patch - hopefully the remaining border work should be much smaller patches after this one :)

leeoniya · 2017-04-28T04:21:31Z

having been part of the Gecko discussion regarding dot/dash spacing, gonna drop this here for posterity: https://bugzilla.mozilla.org/show_bug.cgi?id=382721

glennw · 2017-04-28T04:33:38Z

@leeoniya Thanks! This patch will definitely need a lot more work to get the exact results and edges cases that Gecko handles - but it puts the foundation in place :)

kvark · 2017-04-28T18:27:14Z

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

webrender/res/cs_clip_border.fs.glsl, line 1 at r1 (raw file):

/* This Source Code Form is subject to the terms of the Mozilla Public

let's add #line 1 here

webrender/res/cs_clip_border.fs.glsl, line 25 at r1 (raw file):

    // Apply AA over half a device pixel for the clip.
    float d = smoothstep(-0.5 * afwidth,
                         0.5 * afwidth,

I don't understand this part. So afwidth is an average magnitude of the local position delta per pixel. The maximum d would then be 0.5 * afwidth. So if we have local positions matches with the screen (thus, afwidth==1), it's only going to write 0.5 there at max?

webrender/res/cs_clip_border.fs.glsl, line 26 at r1 (raw file):

    float d = smoothstep(-0.5 * afwidth,
                         0.5 * afwidth,
                         max(d0, -d1));

shouldn't we use the minimum of clip distances instead of the maximum?

webrender/res/cs_clip_border.vs.glsl, line 16 at r1 (raw file):

    vec4 data[2] = fetch_data_2(index);
    return BorderCorner(RectWithSize(data[0].xy, data[0].zw),
                        vec2(data[1].xy),

nit: vec2() not needed

webrender/res/cs_clip_border.vs.glsl, line 49 at r1 (raw file):

    // TODO(gw): We could reduce the number of pixels written here
    // by calculating a tight fitting bounding box of the dash itself.
    vec2 pos = mix(corner.rect.p0,

nit: I think doing vec2 pos = corner.rect.p0 + aPosition.xy * corner.rect.size; is both shorter and faster, even if it doesn't matter here

webrender/res/cs_clip_border.vs.glsl, line 66 at r1 (raw file):

    // Calculate the local space position for this vertex.
    vec4 layer_pos = get_layer_pos(world_pos.xy, layer);

why is this pos different from the previous declaration of pos? we didn't do anything during the transformation to screen space

webrender/res/cs_clip_border.vs.glsl, line 66 at r1 (raw file):

    // Calculate the local space position for this vertex.
    vec4 layer_pos = get_layer_pos(world_pos.xy, layer);

in a similar call to get_layer_pos from write_transform_vertex, we are also dividing by uDevicePixelRatio. Is it missing here?

webrender/res/ps_border_edge.vs.glsl, line 69 at r1 (raw file):

        case BORDER_STYLE_DASHED: {
            float desired_dash_length = border_width * 3.0;
            float dash_count = ceil(0.5 * edge_length / desired_dash_length);

what's the reason for 0.5 here?

webrender/src/border.rs, line 98 at r1 (raw file):

            (BorderStyle::Dashed, BorderStyle::Dashed) => {
                let size = LayerSize::new(width0.max(radius.width), width1.max(radius.height));
                let (origin, clip_center, sign_modifier) = match corner {

it appears that you can produce only sign_modifier by the match and derive everything else from it computationally

webrender/src/ellipse.rs, line 30 at r1 (raw file):

    // arc length of an ellipse segment. We can probably use
    // a faster / more accurate method!
    fn get_simpson_length(&self, theta: f32) -> f32 {

Heavy math methods like this ask for a few unit tests to be sane.
A small benchmark would be extra bonus.
That's for the later work, of course ;)

webrender/src/renderer.rs, line 500 at r1 (raw file):

    cs_clip_rectangle: LazilyCompiledShader,
    cs_clip_image: LazilyCompiledShader,
    cs_clip_border: LazilyCompiledShader,

it's great to see the clip mechanics helping out in seemingly unrelated areas ;) 👍

Comments from Reviewable


        Support dashed borders with new border path.

Dashed border edges are handled by the standard ps_border_edge shader. Dashed border corners instead create a clip mask for the border and draw each dash stroke to the clip mask. The math for determining the correct dash strokes is too complex to evaluate efficiently in a shader, doing it this way allows us to support arbitrary kinds of dash strokes. In the future, we will either specialize this to a border render task (to avoid the clip mask target allocation for the whole border rect), or make use of upcoming optimization work in the clip mask code to handle this case more efficiently. This will also serve as the basis for dotted border corners, which are also too complex to evaluate inside a shader. For now, we just treat everything as an ellipse when evaluating the dash strokes. This gives the correct result for ellipses and circle radii, but is very inefficient for circles. As a follow up, we will include a fast path for corners where the x/y radii are equal. Deciding the exact dash length etc is not well specified. The current code is a rough approximation to what Gecko does. In the future we can iterate on this and make it closer to Gecko's rules for dash length.

glennw · 2017-05-01T21:20:05Z

Review status: 13 of 18 files reviewed at latest revision, 11 unresolved discussions.

webrender/res/cs_clip_border.fs.glsl, line 1 at r1 (raw file):