API REVIEW REQUEST: Public API for Backgrounds and Borders on Region

[Richard Bair]

Here is what I just tried, and it seems to work OK in terms of API. I added a new class, CornerRadius, which contains 4 fields:

    /**
     * The length of the horizontal radii of the top-left corner.
     */
    final double horizontalRadius;
    public final double getHorizontalRadius() { return horizontalRadius; }

    /**
     * The length of the vertical radii of the top-left corner.
     */
    final double verticalRadius;
    public final double getVerticalRadius() { return verticalRadius; }

    /**
     * indicates whether {@code horizontalRadius} is interpreted as a value or a percentage.
     */
    final boolean horizontalRadiusAsPercentage;
    public final boolean isHorizontalRadiusAsPercentage() { return horizontalRadiusAsPercentage; }

    /**
     * indicates whether {@code verticalRadius} is interpreted as a value or a percentage.
     */
    final boolean verticalRadiusAsPercentage;
    public final boolean isVerticalRadiusAsPercentage() { return verticalRadiusAsPercentage; }



This CornerRadius is then used for the radius values in both BackgroundFill and BorderStroke. I removed Radii and BorderRadii.

    final CornerRadius topLeftCorner;
    public final CornerRadius getTopLeftCorner() { return topLeftCorner; }

    final CornerRadius topRightCorner;
    public final CornerRadius getTopRightCorner() { return topRightCorner; }

    final CornerRadius bottomRightCorner;
    public final CornerRadius getBottomRightCorner() { return bottomRightCorner; }

    final CornerRadius bottomLeftCorner;
    public final CornerRadius getBottomLeftCorner() { return bottomLeftCorner; }



I also removed BorderImageSlices, and instead I just use BorderWidths and have "filled" on the BorderImage itself:

    final BorderWidths slices;
    public final BorderWidths getSlices() { return slices; }

    /**
     * Specifies whether or not the center patch (as defined by the left, right, top, and bottom slices)
     * should be drawn.
     */
    final boolean filled;
    public final boolean isFilled() { return filled; }



Personally, I like the fact that BorderStroke and BackgroundFill now have the same capabilities in terms of the corners, but I'm not convinced that I like the CornerRadius better than having BorderRadii holding all 16 values. Maybe instead I should have kept BorderRadii and just renamed it to CornerRadii, and then put it on both BackgroundFill (replacing Radii) and BorderStroke.

I also like removing BorderImageSlices and reusing BorderWidths. It seems much cleaner.

What do you think?

Richard



On Aug 27, 2012, at 10:43 AM, Richard Bair wrote:

> Hi David,
> 
> Yes, Radii, BorderRadii, BorderWidths, and BorderImageSlices all bother me. Radii and BorderRadii bother me because conceptually, the radius values applied to the BackgroundFill *could be* the same as the radius values for a BorderStroke -- they just aren't because our CSS support presently treats them differently (BackgroundFill cannot have percentages for the radii, and in CSS 3's latest spec there are 8 values defining the radii for the stroke borders). But just because our CSS support doesn't expose everything doesn't mean the API couldn't be regularized between the two. In fact, it probably wouldn't be hard to support in NGRegion[1].
> 
> BorderWidths bothers me because it is basically an Insets but with the possibility of being percentage-based for any of the four values. And BorderImageSlices bothers me because it is exactly the same as BorderWidths, but adds "filled". "filled" could have lived on BorderImage from an API perspective, but I'm not sure how to configure the converters to do that.
> 
> Suppose I had a "Size" class such that:
> 
> public final class Size {
>    public final double getValue();
>    public final boolean isPercentage();
> }
> 
> I could flatten things then, such that BackgroundFill had 4 sizes for the four corner radii (public Size getTopLeftCornerRadius()), BorderStroke would have 8 sizes (8 corner radii, two per corner for each dimension). BorderImage and BorderStroke would have 4 sizes for topWidth, rightWidth, bottomWidth, leftWidth. And BorderImageSlices would go away, and instead BorderImage would sprout 4 more sizes for sliceTop, sliceRight, sliceBottom, and sliceLeft and would also get the "filled" boolean.
> 
> This would cut all four of these classes which bother me. The only oddness in the API would then be that we have some properties like "topWidth" which are broken out on the class, and then we also have Insets which is a single object rather than also being broken out such as "topInset", "rightInset", "bottomInset", and "leftInset". I could flatten those as well, but since we have an Insets class already, reusing it doesn't seem too bad.
> 
> Richard
> 
> [1] As an aside, NGRegion has a fast path for the case that all regions are the same. In that case, it just draws a rounded rect. In the case that you have different corner radii, it creates a Path and then fills it (or strokes it, as appropriate). Since the border case is going to force me to deal with different corner radii and produce a Path capable of stroking it, then that means I also will have the ability to reuse that logic for creating a path that can be filled instead. So probably this isn't any more complicated in my code to handle this case.