Make opposite CSS dashed borders symmetrical in various cases where asymmetry is ugly

RESOLVED FIXED in Firefox 57

Status

()

Core
Layout
P2
normal
RESOLVED FIXED
4 months ago
11 days ago

People

(Reporter: karlcow, Assigned: arai)

Tracking

55 Branch
mozilla57
Points:
---
Bug Flags:
webcompat ?

Firefox Tracking Flags

(firefox55 affected, firefox56 affected, firefox57 fixed)

Details

(Whiteboard: [webcompat])

Attachments

(10 attachments, 10 obsolete attachments)

68.00 KB, image/png
Details
79.28 KB, image/png
Details
9.83 KB, image/png
Details
176.69 KB, image/png
Details
7.33 KB, image/png
Details
7.34 KB, image/png
Details
17.52 KB, image/png
Details
2.43 KB, patch
jwatt
: review+
Details | Diff | Splinter Review
8.24 KB, patch
jwatt
: review+
Details | Diff | Splinter Review
5.08 KB, patch
jwatt
: review+
Details | Diff | Splinter Review
(Reporter)

Description

4 months ago
Created attachment 8857769 [details]
hr test firefox safari

When we use border to created a dashed hr, the border is drawn unevenly around the box, creating a wavy effect, not beautiful to the eye. This issue happens only for borders at 1px. When the width is larger the drawing is regular and evenly spaced out.


Tested with Firefox Nightly 55.0a1 (2017-04-10) (64-bit)
Created a test case http://codepen.io/webcompat/pen/VbwGaW
This is a spin-off of the Webcompat issue https://webcompat.com/issues/5616

Simple markup to reproduce
<hr style="width: 200px; border-width: 1px; border-style: dashed;">

Screenshot attached.
Firefox on the left, Safari on the right.
(Reporter)

Updated

4 months ago
Flags: webcompat?
Whiteboard: [webcompat]
cc :arai since she fixed bug 382721 so may have some insight on what should happen here.
(Assignee)

Comment 2

3 months ago
for width<2px dashed border, we simply draw dashed path along the border, in the following direction, and it doesn't guarantee symmetricity (just for performance reason), in nsCSSBorderRenderer::DrawDashedOrDottedSide.

.--->.
^    |
|    |
|    v
.<---.

maybe we could change the direction to the following, so that both horizontal lines (or both vertical lines) are rendered samely, if there's no radius.

.--->.
|    |
|    |
v    v
.--->.

or the following to achieve symmetricity too.

.<-.->.
^     ^
|     |
.     .
|     |
v     v
.<-.->.


or perhaps, we could tweak spacing of dashed pattern to make the border symmetric, in nsCSSBorderRenderer::SetupDashedOptions.
(Assignee)

Comment 3

3 months ago
looks like there's some bug inside nsCSSBorderRenderer::SetupDashedOptions and it doesn't allocate enough length for the segment at the end of the top border.
I'll look into it.
Assignee: nobody → arai.unmht
(Assignee)

Comment 4

3 months ago
to be clear, nsCSSBorderRenderer::SetupDashedOption is already trying to calculate the dashed line segments' length so that it looks symmetric, and I thought it's something related to precision error because of the dashed segment is small compared to border length, but apparently there's simpler issue in the calculation.
(Assignee)

Comment 5

2 months ago
this part seems to be suspicious

https://dxr.mozilla.org/mozilla-central/rev/130efc657df7e7fe291cc42307f3eb3cb0484dfc/layout/painting/nsCSSRenderingBorders.cpp#1873-1880
>   if (borderWidth < 2.0f) {
>     // Round start to draw dot on each pixel.
>     if (IsHorizontalSide(aSide)) {
>       start.x = round(start.x);
>     } else {
>       start.y = round(start.y);
>     }
>   }

it tries to to align the start of the dashed line on the pixel boundary, so that the dashed line is rendered without anti-aliased gray pixel, but because of the asymmetricity how each border is rendered, the affected point differs between opisite sides

top border:    .--->.
               ^
               |
               this point is aligned

bottom border: .<---.
                    ^
                    |
                    this point is aligned

I'll try using the following way at first.

.--->.
|    |
|    |
v    v
.--->.
(Assignee)

Comment 6

2 months ago
nah, the asymmetricity comes from here:

https://dxr.mozilla.org/mozilla-central/rev/130efc657df7e7fe291cc42307f3eb3cb0484dfc/layout/painting/nsCSSRenderingBorders.cpp#3381-3402
>     /*
>      * If we have a 1px-wide border, the corners are going to be
>      * negligible, so don't bother doing anything fancy.  Just extend
>      * the top and bottom borders to the right 1px and the left border
>      * to the bottom 1px.  We do this by twiddling the corner dimensions,
>      * which causes the right to happen later on.  Only do this if we have
>      * a 1.0 unit border all around and no border radius.
>      */
> 
>     NS_FOR_CSS_FULL_CORNERS(corner) {
>       const mozilla::Side sides[2] = { mozilla::Side(corner), PREV_SIDE(corner) };
> 
>       if (!IsZeroSize(mBorderRadii[corner]))
>         continue;
> 
>       if (mBorderWidths[sides[0]] == 1.0 && mBorderWidths[sides[1]] == 1.0) {
>         if (corner == eCornerTopLeft || corner == eCornerTopRight)
>           mBorderCornerDimensions[corner].width = 0.0;
>         else
>           mBorderCornerDimensions[corner].height = 0.0;
>       }
>     }

This algorithm doesn't work properly with dashed/dotted.
(Assignee)

Comment 7

2 months ago
to be clear, because of that adjustment, the border length of top/bottom borders differ.
(Assignee)

Comment 8

2 months ago
Created attachment 8874242 [details]
current mBorderCornerDimensions twiddling for 1px-wide border

this is what happens now.
the length of top/bottom borders differ because of this, and the dashed pattern also differ.
I think we could just suppress this when horizontal or vertical border for the given corner is dashed or dotted.
(Assignee)

Comment 9

2 months ago
Created attachment 8874264 [details]
WIP rendering result

try is running:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=52abc1c2982e15078c61dde84348d73ec74e67cb

it would hit failure on some test that relies on previous behavior (I just should update those tests), and I need to add testcase for this change.
(Assignee)

Comment 10

2 months ago
Created attachment 8874266 [details]
patched mBorderCornerDimensions twiddling for 1px-wide border

this is what the patch does
(Assignee)

Comment 11

2 months ago
now investigating the failure for object-fit-fill-png-001c.html etc
(I'm not sure why it happens at this point...)
https://treeherder.mozilla.org/#/jobs?repo=try&revision=619f0c7e4e760b7271e87a03e22160ef98ac44bc
(Assignee)

Updated

2 months ago
Status: NEW → ASSIGNED
(Assignee)

Comment 12

2 months ago
hmm, I don't see any difference in the parameter while rendering dashed border for object-fit-fill-png-001c.html and object-fit-fill-png-001-ref.html, but the rendering result has differences in the gray color (#c0c0c0 vs #bfbfbf)...
and I don't see the issue when I open the file with built binary...
I'll try building it locally.
(Assignee)

Comment 13

2 months ago
the only difference I see is that, the border is rendered twice for object-fit-fill-png-001c.html.
but I cannot figure out how it results in #c0c0c0 vs #bfbfbf, instead of twice darker color, also appears only in specific part...
(Assignee)

Comment 14

2 months ago
Created attachment 8876478 [details]
object-fit-fill-png-001-ref.html result
(Assignee)

Comment 15

2 months ago
Created attachment 8876479 [details]
object-fit-fill-png-001c.html result
(Assignee)

Comment 16

2 months ago
the issue is reproducible with local build, but only in reftest.
when I open those files in normal session, the issue doesn't happen.
(Assignee)

Comment 17

2 months ago
Created attachment 8876504 [details]
the color of the anti-aliased part of the dashed line

here's the difference
(Assignee)

Comment 18

2 months ago
looks like the fact that the border is rendered twice is not related.
the failure happens even if I skip the first rendering.
(Assignee)

Comment 19

2 months ago
interestingly, the reftest fails even if I use the same content (object-fit-fill-png-001-ref.html) for both TEST and REFERENCE.
so I think there's some trick happening while rendering the dashed line.
(Assignee)

Comment 20

2 months ago
I'm about to allow fuzziness for now, since I think this is not actually a regression from this change, but just discovered by the change.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a16e5ac6eeb3ecc5af63c65aa98b547b86cadf25
(Assignee)

Comment 21

2 months ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c15a8e03c1516ef6b6c29a25e21ebfe8846470dd
(Assignee)

Comment 22

2 months ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c15f651802e5c65e5dd1b707a3308fd131073bd0&group_state=expanded
https://treeherder.mozilla.org/#/jobs?repo=try&revision=60e964a95d5cd7cfa63f6ad57fa4949827cbfab4&group_state=expanded
(Assignee)

Comment 23

2 months ago
Created attachment 8877800 [details] [diff] [review]
Part 1: Adjust the border corner dimension so that the border becomes symmetric.

the issue is described in comment #6, comment #8,
and the solution is described in https://bug1356114.bmoattachments.org/attachment.cgi?id=8874266

so, allocating same area for both sides solves the asymmetricity issue.
Attachment #8877800 - Flags: review?(jwatt)
(Assignee)

Comment 24

2 months ago
Created attachment 8877801 [details] [diff] [review]
Part 2: Start and end with full dashed segment if the box is has 2px width or height.

also, tweaked the line ends for those cases, to start and end with full dashed segment.
Attachment #8877801 - Flags: review?(jwatt)
(Assignee)

Comment 25

2 months ago
Created attachment 8877802 [details] [diff] [review]
Part 3: Update border dashed testcase.

updated the testcase mask
Attachment #8877802 - Flags: review?(jwatt)
(Assignee)

Comment 26

2 months ago
Created attachment 8877804 [details] [diff] [review]
Part 4: Allow fuzziness for dashed line in reftest.

then, I hit possible skia bug that the anti-aliased color becomes different for exactly same parameters for dashed line.

I've added fuzzy-if for affected testcases, but this is not covering all testcases that uses dashed border (there are so many test cases that uses dashed border even if it's not about border), and I'm not sure whether files covered by this patch is sufficient or not...
Attachment #8877804 - Flags: review?(jwatt)

Updated

a month ago
status-firefox56: --- → affected
Priority: -- → P1
Astley, what was the logic behind marking this P1? The asthetics of 1px borders on <hr> seem a bit of an edge case so I haven't given this high priority so far.
Flags: needinfo?(aschen)
(In reply to Jonathan Watt [:jwatt] from comment #27)
> Astley, what was the logic behind marking this P1? The asthetics of 1px
> borders on <hr> seem a bit of an edge case so I haven't given this high
> priority so far.

Generally speaking, for webcompat(author/user facing) issues I will give them at least P2 as we will address them sooner or later based on priorities in our work queue.
For this bug, sorry I originally meant to mark it as P2, my bad.

I agree that this bug seems minor and probably an edge case, given that there are patches waiting for review, it would be great if you have spare time to progress it or just direct to other reviewers if you are busy.
Flags: needinfo?(aschen)
Priority: P1 → P2
(Assignee)

Updated

14 days ago
Attachment #8877800 - Flags: review?(jwatt) → review?(jmuizelaar)
(Assignee)

Updated

14 days ago
Attachment #8877801 - Flags: review?(jwatt) → review?(jmuizelaar)
(Assignee)

Updated

14 days ago
Attachment #8877802 - Flags: review?(jwatt) → review?(jmuizelaar)
(Assignee)

Updated

14 days ago
Attachment #8877804 - Flags: review?(jwatt) → review?(jmuizelaar)
(Assignee)

Comment 29

14 days ago
Created attachment 8893461 [details] [diff] [review]
Part 1.1: Update comment for border corner dimension for 1px borders.

As discussed on IRC, updated the comment above the Part 1 change.
Attachment #8893461 - Flags: review?(jwatt)
(Assignee)

Comment 30

14 days ago
try run on current central
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=21886336d8d72a2ee5231c086890f4fd025e473b&group_state=expanded

there are some tests that fails with similar fuzziness.
(that might imply that the similar fuzziness can start happenning after landing this, with (almost-)unrelated change


also try run with removing non-symmetric case:
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=d245465cfa392aa559ff43fba5b0bfb3167f79ed&group_state=expanded
(Assignee)

Comment 31

13 days ago
also try run only with Part 1 to see when the fuzziness started happening

https://treeherder.mozilla.org/#/jobs?repo=try&revision=54a8d7dc09657f71e6614d9ba13c1c033fce7dab
(Assignee)

Comment 32

13 days ago
apparently the fuzziness started happening in Part 2.
(Assignee)

Comment 33

13 days ago
the set of fuzzy testcases differs for each run in comment #30, but all of them are object-fit-*-png-*.html, that shares same structure.
I think I should add fuzzy-if to more tests (maybe all tests that shares same structure...?)
(In reply to Tooru Fujisawa [:arai] from comment #29)
> Created attachment 8893461 [details] [diff] [review]
> Part 1.1: Update comment for border corner dimension for 1px borders.
> 
> As discussed on IRC, updated the comment above the Part 1 change.

Thanks!
(Assignee)

Comment 35

13 days ago
for future reference, the issue doesn't happen on retina display (since the fast path for 1px border width is not used).
similar issue can be reproduced on retina display by zooming out, to make the border width thinner.
(Assignee)

Comment 36

13 days ago
try run without removing |borderWidth <= 1.0f| condition in Part 2
https://treeherder.mozilla.org/#/jobs?repo=try&revision=96d42b95ae38c19eaa26163afd31187a4b563eff

I hope this can avoid the fuzziness.
(Assignee)

Comment 37

13 days ago
Created attachment 8893730 [details] [diff] [review]
Part 1: Adjust the border corner dimension so that the border becomes symmetric.

Merged 1.1 and also removed old behavior
Attachment #8877800 - Attachment is obsolete: true
Attachment #8893461 - Attachment is obsolete: true
Attachment #8877800 - Flags: review?(jmuizelaar)
Attachment #8893461 - Flags: review?(jwatt)
Attachment #8893730 - Flags: review?(jwatt)
(Assignee)

Comment 38

13 days ago
Created attachment 8893731 [details] [diff] [review]
Part 2: Start and end with full dashed segment if the box is has 2px width or height.

Reverted the change to not-thin box.
so the not-thin box keeps non-symmetric but the case in comment #0 (thin box) gets fixed.

this doesn't cause fuzziness :)
Attachment #8877801 - Attachment is obsolete: true
Attachment #8877804 - Attachment is obsolete: true
Attachment #8877801 - Flags: review?(jmuizelaar)
Attachment #8877804 - Flags: review?(jmuizelaar)
Attachment #8893731 - Flags: review?(jwatt)
(Assignee)

Comment 39

13 days ago
Created attachment 8893732 [details] [diff] [review]
Part 3: Update border dashed testcase.

updated reference mask
Attachment #8877802 - Attachment is obsolete: true
Attachment #8877802 - Flags: review?(jmuizelaar)
Attachment #8893732 - Flags: review?(jwatt)
(Assignee)

Comment 40

13 days ago
I'll file followup bug for symmetricity for not-thin box later.
(Assignee)

Comment 41

13 days ago
Created attachment 8893747 [details] [diff] [review]
Part 3: Update border dashed testcase.

forgot to refresh the queue
Attachment #8893732 - Attachment is obsolete: true
Attachment #8893732 - Flags: review?(jwatt)
Attachment #8893747 - Flags: review?(jwatt)
Comment on attachment 8893730 [details] [diff] [review]
Part 1: Adjust the border corner dimension so that the border becomes symmetric.

Review of attachment 8893730 [details] [diff] [review]:
-----------------------------------------------------------------

Please change the comment to:

  Bug 1356114 - Part 1: Change the code that optimizes away CSS border corner painting to ensure opposite borders have the same length. r=jwatt

  See https://bugzilla.mozilla.org/show_bug.cgi?id=1356114#c8
  for a diagram describing the old behavior.

::: layout/painting/nsCSSRenderingBorders.cpp
@@ +3382,5 @@
> +    // going to be negligible, so don't bother doing anything fancy. Just
> +    // extend the borders along  the longer sides to keep the borders on
> +    // opposite sides the same length, so that dashed/dotted lines become
> +    // symmetrical. We do this by twiddling the corner dimensions, which causes
> +    // the right to happen later on.

Please change this comment to:

    // The corner is going to have negligible size if its two adjacent border
    // sides are only 1px wide and there is no border radius.  In that case we
    // skip the overhead of painting the corner by setting the width or height
    // of the corner to zero, which effectively extends one of the corner's
    // adjacent border sides.  We extend the longer adjacent side so that
    // opposite sides will be the same length, which is necessary for opposite
    // dashed/dotted sides to be symmetrical.

@@ +3385,5 @@
> +    // symmetrical. We do this by twiddling the corner dimensions, which causes
> +    // the right to happen later on.
> +    //
> +    //   if width > height
> +    //     +--+--------+--+    +--------------+

Please add a bit more width to these first two border diagrams to make them wider.

@@ +3408,5 @@
> +    //     |  |        |  |    |  |        |  |
> +    //     |  |        |  |    |  |        |  |
> +    //     +--+--------+--+    |  +--------+  |
> +    //     |  |        |  |    |  |        |  |
> +    //     +--+--------+--+    +--+--------+--+

After these diagrams please append:

    // Note that if we have different border widths we could end up with
    // opposite sides of different length.  For example, if the left and
    // bottom borders are 2px wide instead of 1px, we will end up doing
    // something like:
    //
    //     +----+------------+--+    +----+---------------+
    //     |    |            |  |    |    |               |
    //     +----+------------+--+    +----+------------+--+
    //     |    |            |  |    |    |            |  |
    //     |    |            |  | => |    |            |  |
    //     |    |            |  |    |    |            |  |
    //     +----+------------+--+    +----+------------+--+
    //     |    |            |  |    |    |            |  |
    //     |    |            |  |    |    |            |  |
    //     +----+------------+--+    +--------------------+
    //
    // XXX Should we only do this optimization if |allBordersSameStyle| is true?
    //
    // XXX In fact is this optimization even worth the complexity it adds to
    // the code?  1px wide dashed borders are not overly common, and drawing
    // corners for them is not that expensive.
Attachment #8893730 - Flags: review?(jwatt) → review+
Comment on attachment 8893730 [details] [diff] [review]
Part 1: Adjust the border corner dimension so that the border becomes symmetric.

Review of attachment 8893730 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/painting/nsCSSRenderingBorders.cpp
@@ +3408,5 @@
> +    //     |  |        |  |    |  |        |  |
> +    //     |  |        |  |    |  |        |  |
> +    //     +--+--------+--+    |  +--------+  |
> +    //     |  |        |  |    |  |        |  |
> +    //     +--+--------+--+    +--+--------+--+

Sorry, that should be allBordersSameWidth, not allBordersSameStyle.
Comment on attachment 8893731 [details] [diff] [review]
Part 2: Start and end with full dashed segment if the box is has 2px width or height.

This code is very specific to part 1, what with the very specific border width and rect size checks. At the very least this would need a decent comment to explain what this code is doing because it will not be clear at all other devs that encounter this code.

However, there's way too much code in this file for special cases, and I would really prefer to avoid adding more code for special cases if it's not necessary. (We've been experiencing how much of a headache that code can create while trying to work on this bug the last few days.)

As far as I can tell the purpose of the code above your change is to make us use a half dash when the dashes at the ends of the line will be connecting onto a corner, and use a full dash when they will not. It seems like that's exactly what we would want to do in the case there is no corner (it's empty), which happens when the adjacent side has zero width or when we apply the tweak in part 1. So if we made that more general change we would, I think, improve the behavior and make it more consistent while also fixing the specific case we care about here.

So I'd propose changing that instead of this patch you change the code to be something like this:

    if (mBorderRadii[GetCCWCorner(aSide)].IsEmpty() &&
        (mBorderCornerDimensions[GetCCWCorner(aSide)].IsEmpty() ||
         mBorderStyles[PREV_SIDE(aSide)] == NS_STYLE_BORDER_STYLE_DOTTED ||
         // XXX why this <=1 check?
         borderWidth <= 1.0f)) {
      fullStart = true;
    }

    if (mBorderRadii[GetCWCorner(aSide)].IsEmpty() &&
        (mBorderCornerDimensions[GetCWCorner(aSide)].IsEmpty() ||
         mBorderStyles[NEXT_SIDE(aSide)] == NS_STYLE_BORDER_STYLE_DOTTED)) {
      fullEnd = true;
    }

That replaces the:

  mBorderWidths[PREV_SIDE(aSide)] == 0.0f

check with the more general check:

  mBorderCornerDimensions[GetCWCorner(aSide)].IsEmpty()

which covers both the adjacent side being zero width, but also the corner being zero size.

Also a comment like the following above those code blocks to explain what this code is doing would be great, since it took me a little while to figure it out:

    // If either end of the side is not connecting onto a corner then we want a
    // full dash at that end.
    //
    // Note that in the case that a corner is empty, either the adjacent side
    // has zero width, or else DrawBorders() set the corner to be empty
    // (it does that if the adjacent side has zero length and the border widths
    // of this and the adjacent sides are thin enough that the corner will be
    // insignificantly small).

I have pushed your patches with these changes to part 2 to Try and, other than part 3 needing tweaked again, it seems to pass:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=347bdd9dafbd90fdb36ec0cb898bbcfd600dfd38

I'm going to mark this attached patch as r-, but if you think of/uncover any problems with my proposal then let me know.
Attachment #8893731 - Flags: review?(jwatt) → review-
Note also that your latest version of part 1 which removed the special branch to take the old code path has fixed the following tests to start passing on Linux x64 Stylo:

  layout/reftests/flexbox/flexbox-dyn-insertAroundDiv-3.xhtml
  layout/reftests/flexbox/flexbox-dyn-insertAroundSpan-3.xhtml 

So you'll need to remove the:

  fails-if(gtkWidget&&styloVsGecko)

annotation from:

https://hg.mozilla.org/mozilla-central/annotate/52285ea5e54c73d3ed824544cef2ee3f195f05e6/layout/reftests/flexbox/reftest.list#l46

I guess that change can go in part 3.
(Assignee)

Comment 46

12 days ago
Created attachment 8894084 [details] [diff] [review]
Part 2: Check border corner dimension instead of adjacent corner width when tweaking start/end segment size for dashed border.

Thanks, I agree with this way :)
it fixes both thin and not-thin cases.
Attachment #8893731 - Attachment is obsolete: true
Attachment #8894084 - Flags: review?(jwatt)
(Assignee)

Comment 47

12 days ago
Created attachment 8894085 [details] [diff] [review]
Part 3: Update border dashed testcase.

updated test masks and also known-fail
Attachment #8893747 - Attachment is obsolete: true
Attachment #8893747 - Flags: review?(jwatt)
Attachment #8894085 - Flags: review?(jwatt)
(Assignee)

Comment 48

12 days ago
Created attachment 8894087 [details] [diff] [review]
Part 1: Change the code that optimizes away CSS border corner painting to ensure opposite borders have the same length. r=jwatt
Attachment #8893730 - Attachment is obsolete: true
Attachment #8894087 - Flags: review?(jwatt)
(Assignee)

Comment 49

12 days ago
Created attachment 8894090 [details] [diff] [review]
Part 1: Change the code that optimizes away CSS border corner painting to ensure opposite sides have the same length. r=jwatt

fixed comments
Attachment #8894087 - Attachment is obsolete: true
Attachment #8894087 - Flags: review?(jwatt)
Attachment #8894090 - Flags: review?(jwatt)

Updated

12 days ago
Attachment #8894090 - Flags: review?(jwatt) → review+
Comment on attachment 8894084 [details] [diff] [review]
Part 2: Check border corner dimension instead of adjacent corner width when tweaking start/end segment size for dashed border.

Review of attachment 8894084 [details] [diff] [review]:
-----------------------------------------------------------------

The commit message should make sense to someone in the context of all commits landing in the codebase. It should also focus on the overall aim of the change rather than its mechanics. Please make the commit message "Consistently make the ends of dashed CSS borders a full dash if not connecting to a corner".
Attachment #8894084 - Flags: review?(jwatt) → review+

Updated

12 days ago
Attachment #8894085 - Flags: review?(jwatt) → review+

Updated

12 days ago
Summary: hr with 1px border-width and dashed style is not evenly spaced → Make opposite CSS dashed borders symmetrical in various cases where asymmetry is ugly
Thanks, Tooru. I really appreciate your time and patience the last few days as we worked through various issues on IRC (especially given our timezone difference)! I'm much happier now we've managed to remove some of the unnecessary and confusing special cases rather than adding more of them, not to mention avoiding adding fuzz to lots of reftests. I think the code better for it. Hopefully you feel it was worth it too. :)

Comment 52

12 days ago
Pushed by arai_a@mac.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a69f05ac539a
Part 1: Change the code that optimizes away CSS border corner painting to ensure opposite sides have the same length. r=jwatt
https://hg.mozilla.org/integration/mozilla-inbound/rev/11dfd8e3c520
Part 2: Consistently make the ends of dashed CSS borders a full dash if not connecting to a corner. r=jwatt
https://hg.mozilla.org/integration/mozilla-inbound/rev/647143deab13
Part 3: Update border dashed testcase. r=jwatt
(Assignee)

Comment 53

12 days ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/a69f05ac539a8e39cfdee3d559da4d0cc962f7ed
Bug 1356114 - Part 1: Change the code that optimizes away CSS border corner painting to ensure opposite sides have the same length. r=jwatt

https://hg.mozilla.org/integration/mozilla-inbound/rev/11dfd8e3c5202e7055338f9f5d84c7890cad2a9d
Bug 1356114 - Part 2: Consistently make the ends of dashed CSS borders a full dash if not connecting to a corner. r=jwatt

https://hg.mozilla.org/integration/mozilla-inbound/rev/647143deab1374cf8ae938f7c9a35fd634dbab8a
Bug 1356114 - Part 3: Update border dashed testcase. r=jwatt
https://hg.mozilla.org/mozilla-central/rev/a69f05ac539a
https://hg.mozilla.org/mozilla-central/rev/11dfd8e3c520
https://hg.mozilla.org/mozilla-central/rev/647143deab13
Status: ASSIGNED → RESOLVED
Last Resolved: 11 days ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.