Are Border Gradients Sufficiently Expressive? #1875

Gankra · 2017-10-13T20:09:29Z

I'm still taking this all in, but here's a weird case that @mstange believes can't be captured with the arguments we pass to wr:

<!DOCTYPE html>
<meta charset="utf-8">
<title>border-image with linear-gradient</title>
<style>

div {
  border: 30px solid;
  box-sizing: border-box;
  width: 200px;
  height: 200px;
  border-image: linear-gradient(45deg, red 50px, yellow 50px, yellow 150px, red 150px) 50;
}

</style>

<div></div>

Gankra · 2017-10-13T20:11:19Z

CC @eqrion @emilio

mstange · 2017-10-13T20:17:31Z

In particular I don't think the current API (or at least the part of the API that is exposed through wrench YAML) allows us to specify the intrinsic size of the gradient image.

For correct rendering of border-image gradients, the following pieces of information are necessary:

The size of the gradient image
The gradient color stops, with their positions as fractions relative to "gradient image" space
The slice insets in "gradient image" space
The size of the resulting rendering, in local space
The slice insets for the result, in local space
General border-image features like "should the middle of the 9-piece be filled" and "should I stretch, repeat or round the side parts"

mstange · 2017-10-15T01:48:58Z

Here's the relevant part of the API:

#[repr(C)]
#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)]
pub struct NinePatchDescriptor {
    pub width: u32,
    pub height: u32,
    pub slice: SideOffsets2D<u32>,
}

#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)]
pub struct ImageBorder {
    pub image_key: ImageKey,
    pub patch: NinePatchDescriptor,
    /// Controls whether the center of the 9 patch image is
    /// rendered or ignored.
    pub fill: bool,
    pub outset: SideOffsets2D<f32>,
    pub repeat_horizontal: RepeatMode,
    pub repeat_vertical: RepeatMode,
}

#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)]
pub struct GradientBorder {
    pub gradient: Gradient,
    pub outset: SideOffsets2D<f32>,
}

#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)]
pub struct RadialGradientBorder {
    pub gradient: RadialGradient,
    pub outset: SideOffsets2D<f32>,
}

#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)]
pub enum BorderDetails {
    Normal(NormalBorder),
    Image(ImageBorder),
    Gradient(GradientBorder),
    RadialGradient(RadialGradientBorder),
}

#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)]
pub struct BorderDisplayItem {
    pub widths: BorderWidths,
    pub details: BorderDetails,
}

So NinePatchDescriptor, which is used by ImageBorder but not by the gradient borders, does have width and height properties that describe the source image size. These values will also be needed for gradient borders. Furthermore, these properties ofImageBorder are needed for the gradient variants as well:

    pub fill: bool,
    pub repeat_horizontal: RepeatMode,
    pub repeat_vertical: RepeatMode,

And I don't believe the outset property is even needed for ImageBorder; it's a layout thing that can be baked into the border rect and the border widths by the caller. So here's what I'd recommend as the new API:

#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)]
pub enum NinePartBorderSource {
    Image(ImageKey),
    Gradient(Gradient),
    RadialGradient(RadialGradient),
}

#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)]
pub struct NinePartBorder {
    /// Describes what to use as the 9-part source image. If this is an image,
    /// it will be stretched to fill the size given by width x height. If this
    /// is a type of gradient, the 9-part source image is obtained by rendering
    /// the gradient into a conceptual surface of dimensions width x height.
    pub source: NinePartBorderSource,
    /// The width of the 9-part image.
    pub width: u32,
    /// The height of the 9-part image.
    pub height: u32,
    /// Distances from each edge where the image should be sliced up. These
    /// values are in 9-part-image space (the same space as width and height),
    /// and the resulting image parts will be used to fill the corresponding
    /// parts of the border as given by the border widths. This can lead to
    /// stretching.
    /// Slices can be overlapping. In that case, the same pixels from the
    /// 9-part image will show up in multiple parts of the resulting border.
    pub slice: SideOffsets2D<u32>,
    /// Controls whether the center of the 9 patch image is rendered or
    /// ignored. The center is never rendered if the slices are overlapping.
    pub fill: bool,
    /// Determines what happens if the horizontal side parts of the 9-part
    /// image have a different size than the horizontal parts of the border.
    pub repeat_horizontal: RepeatMode,
    /// Determines what happens if the vertical side parts of the 9-part
    /// image have a different size than the vertical parts of the border.
    pub repeat_vertical: RepeatMode,
}

#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)]
pub enum BorderDetails {
    Normal(NormalBorder),
    NinePart(NinePartBorder),
}

#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)]
pub struct BorderDisplayItem {
    pub widths: BorderWidths,
    pub details: BorderDetails,
}

glennw · 2017-10-15T21:01:30Z

Thanks for doing that detailed write up, sounds good!


        Make border image API more general

This is a preparatory patch to add support for nine-patch like functionality to border gradients. The new API is based on mstage's proposed API from servo#1875.


        Make border image API more general

This is a preparatory patch to add support for nine-patch like functionality to border gradients. The new API is based on mstage's proposed API from servo#1875.


        Auto merge of #2724 - mrobinson:api-preparation-for-revolutionary-nin…

…e-patch-technology, r=Ganko Make border image API more general This is a preparatory patch to add support for nine-patch like functionality to border gradients. The new API is based on mstage's proposed API from #1875.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/2724)


        Implement nine patch support for border gradients

This will allow border-gradients to support the full richness of the CSS API. There are some areas where the implementation doesn't match what CSS expects, but this should move us a good deal closer. We can gradually fix those issues and turn this on in Gecko. Fixes servo#1875.


        Implement nine patch support for border gradients

This will allow border-gradients to support the full richness of the CSS API. There are some areas where the implementation doesn't match what CSS expects, but this should move us a good deal closer. We can gradually fix those issues and turn this on in Gecko. Fixes servo#1875.


        Auto merge of #2848 - mrobinson:nine-patch-gradient, r=gw3583

Implement nine patch support for border gradients This will allow border-gradients to support the full richness of the CSS API. There are some areas where the implementation doesn't match what CSS expects, but this should move us a good deal closer. We can gradually fix those issues and turn this on in Gecko. Fixes #1875.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/2848)


        Implement nine patch support for border gradients

This will allow border-gradients to support the full richness of the CSS API. There are some areas where the implementation doesn't match what CSS expects, but this should move us a good deal closer. We can gradually fix those issues and turn this on in Gecko. Fixes servo#1875.

kvark added difficulty: moderate type: enhancement labels Oct 17, 2017

pyfisch mentioned this issue May 2, 2018

Implement border-image-outset servo/servo#20739

Merged

mrobinson added a commit to mrobinson/webrender that referenced this issue May 3, 2018

Make border image API more general

635cd24

This is a preparatory patch to add support for nine-patch like functionality to border gradients. The new API is based on mstage's proposed API from servo#1875.

mrobinson mentioned this issue May 3, 2018

Make border image API more general #2724

Merged

mrobinson added a commit to mrobinson/webrender that referenced this issue May 3, 2018

Make border image API more general

9508251

This is a preparatory patch to add support for nine-patch like functionality to border gradients. The new API is based on mstage's proposed API from servo#1875.

mrobinson mentioned this issue Jun 26, 2018

Implement nine patch support for border gradients #2848

Merged

bors-servo closed this in #2848 Jun 27, 2018

servo / webrender

Are Border Gradients Sufficiently Expressive? #1875

Are Border Gradients Sufficiently Expressive? #1875

Gankra commented Oct 13, 2017

Gankra commented Oct 13, 2017

mstange commented Oct 13, 2017 •

edited

mstange commented Oct 15, 2017

glennw commented Oct 15, 2017

servo / webrender

Join GitHub today

Are Border Gradients Sufficiently Expressive? #1875

Are Border Gradients Sufficiently Expressive? #1875

Comments

Gankra commented Oct 13, 2017

Gankra commented Oct 13, 2017

mstange commented Oct 13, 2017 • edited

mstange commented Oct 15, 2017

glennw commented Oct 15, 2017

mstange commented Oct 13, 2017 •

edited