Skip to content

Border and Outline are two model classes for one line, and neither can describe it #943

Description

@dkulyk

This is a design question rather than a bug report, and it wants a maintainer's decision before
anything is written. Three defects and three user reports below all follow from the same split,
so I would rather ask once than patch each symptom.

Style\Border and Style\Outline both describe the stroke of a shape. Both are written as
a:ln in PowerPoint2007 (writeBorder(), AbstractDecoratorWriter.php:63, and writeOutline(),
:234) and as draw:stroke / svg:stroke-* in ODPresentation. Which class a given object gets is
decided by history, not by the model:

carries Border carries Outline
AbstractShape — so every shape Shape\AutoShape
Shape\Chart\Legend Shape\Chart\Axis
Shape\Chart\Marker Shape\Chart\Gridlines
a table Cell (four of them, via Borders) Shape\Chart\Series

Outline was added in c25b553 (2016, #169) to give a chart series a line width and colour.
Border already carried a line width, a colour, a dash style and a compound style at that point.

Neither class covers CT_LineProperties

a:ln is one type: w, cap, cmpd, algn, then a fill group (a:noFill | a:solidFill |
a:gradFill | a:pattFill), then a dash group, then join, arrow ends, a:extLst.

  • Border has lineWidth, lineStyle, dashStyle, Color. It cannot describe a gradient or
    pattern line, because its colour is a Color and not a Fill.
  • Outline has width and a Fill. It cannot describe a dash or a compound line, because it
    has neither field.

Between them they cover the type; separately, neither does. writeBorder() emits w, cap,
cmpd, algn and a:prstDash; writeOutline() emits w and a fill and nothing else.

LibreOffice keeps the same information in one struct — oox/inc/drawingml/lineproperties.hxx:

struct LineProperties {
    FillProperties      maLineFill;      // what this library calls Outline
    DashStopVector      maCustomDash;
    std::optional<sal_Int32> moLineWidth, moPresetDash, moLineCompound, moLineCap, moLineJoint;
};

and LineProperties::getLineStyle() has to read both halves to answer one question:

if (maLineFill.moFillType.value() == XML_noFill) return LineStyle_NONE;
if (moPresetDash && *moPresetDash != XML_solid)  return LineStyle_DASH;
return LineStyle_SOLID;

Whether a line exists is a property of the fill — this library's Outline. Whether it is solid or
dashed is a property of the dash — this library's Border. Neither PHP class can see the other.

What follows from the split, measured on master

1. A LINE_DOUBLE shape is written to ODP as no line at all.
Content.php:1298 (writeLineStyle()) switches on getLineStyle() and maps everything that is
not LINE_SINGLE or LINE_NONE to draw:stroke="none". So LINE_DOUBLE, LINE_THICKTHIN,
LINE_THINTHICK and LINE_TRI — four of the six constants — silently delete the line. ODF's
draw:stroke takes only none, solid and dash, so a compound line genuinely has no ODF
spelling; falling back to solid would at least keep the line.

2. The same width means different sizes. getLineWidth() and getWidth() have no documented
unit, and every writer picked its own. Measured with a single Border::setLineWidth(10) on one
RichText, written to both formats:

output in points
PowerPoint2007 <a:ln w="95250"> 7.5pt (read as pixels)
ODPresentation svg:stroke-width="0.353cm" 10pt (read as points)

Outline is inconsistent within one file. One chart, Outline::setWidth(10) on the X axis and
on the series:

style output in points
styleAxisX 0.353cm 10ptpointsToCentimeters(), ObjectsChart.php:375
styleSeries0 0.265cm 7.5ptpixelsToCentimeters(), ObjectsChart.php:824

ObjectsChart.php:888 (the chart wall) uses pixels as well, :415 (gridlines) uses points.
#703 fixed one of these sites in the PowerPoint2007 writer and the others were never revisited —
which is what a value with no owning contract does.

3. Border::lineStyle conflates two things, one level further down.
LINE_SINGLE = sng, LINE_DOUBLE = dbl, LINE_THICKTHIN, LINE_THINTHICK, LINE_TRI are
ST_CompoundLine values, and writeBorder() writes them to cmpd. LINE_NONE = none is not a
valid cmpd value at all — it is a sentinel that the writer turns into a:noFill
(AbstractDecoratorWriter.php:71-73, :83-84). So one field carries both what shape the line
has
and whether there is a line, which is the same conflation the two classes produce.

What users have run into

  • How to customize the color and linestyle of the axisX and axisY #292 — to colour an axis line, the reporter had to patch the library: add an Outline
    property to Shape\Chart\Axis and a writeOutline() call to writeAxis(). Every shape already
    had getBorder(); Axis is not a shape, so it had none.
  • Format axis line #255"our style requires that there is no visible Y axis line", and the request was
    literally getAxisY()->setLine([outline object]), "similar to the implementation for
    Gridlines"
    . The user's word for the concept is neither of ours.
  • 圆角矩形创建,无法设置边框线条宽度以及颜色 #884AutoShape inherits getBorder() from AbstractShape and owns getOutline()
    separately. The border it hands out is live: it takes a width and a colour and gives them back.
    No writer ever reads it. The reporter found the inherited getter first, which is the plausible
    one, and concluded the feature was missing.

Proposal

The three consequences above are independent of the model question and could be fixed today:
LINE_DOUBLEdraw:stroke="solid" in ODP, and one unit for Outline::getWidth() across the
four ODP sites. The second one changes the rendering of existing decks, which is why it is here
and not in a PR.

For the model itself, three options as I see them, in the order I would rank them:

A. Border grows a Fill; Outline becomes a deprecated view of it.
Border::getFill(): Fill alongside the existing getColor(): ?Color, with the colour kept as the
convenience accessor it already is. The four chart classes gain getBorder()/setBorder(), and
getOutline()/setOutline() stay as @deprecated delegates returning a Border-backed view, so
no existing call breaks in 1.x. writeOutline() becomes a call to writeBorder(). Removal at 2.0.
This direction is the cheaper one: Border is already on AbstractShape and is the one users
reach for by name.

B. The reverseOutline grows the dash and the compound and Border is deprecated. Same end
state, but the deprecation lands on the class with far more call sites, including every table cell.

C. Keep both, document the split, fix only the three defects. Cheapest, and leaves AutoShape
handing out a live border that nothing reads.

I am happy to write A as a PR if that is the direction, in the order: the two defect fixes first as
their own PRs (they stand alone), then the Fill on Border, then the chart classes.

What waits on the answer: #884, and any documentation of AutoShape's line, which today has no
correct way to describe itself.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions