Last Comment Bug 382721 - Dotted/dashed border-radiused corners are rendered as solid (no border-style support)
: Dotted/dashed border-radiused corners are rendered as solid (no border-style ...
Status: RESOLVED FIXED
: dev-doc-complete, testcase
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: All All
: -- normal with 102 votes (vote)
: mozilla50
Assigned To: Tooru Fujisawa [:arai]
:
: Jet Villegas (:jet)
Mentors:
: 540276 947833 973037 995677 1012868 1120933 1225837 1257447 1306615 (view as bug list)
Depends on: 1287076 1289828 19963 652650 995677 1294015 1318625
Blocks: border-radius 13944 368247 379303 468835 1225837
  Show dependency treegraph
 
Reported: 2007-05-31 16:53 PDT by Gábor Stefanik
Modified: 2016-11-18 17:58 PST (History)
104 users (show)
gavin.sharp: firefox‑backlog-
roc: blocking1.9.2-
roc: wanted1.9.2-
vladimir: blocking1.9-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
50+


Attachments
Testcase, dotted circle is rendered as solid. (498 bytes, text/html)
2007-05-31 16:53 PDT, Gábor Stefanik
no flags Details
Testcase with round borders for all possible border styles. (494 bytes, text/html)
2009-03-29 13:19 PDT, Marco Perez
no flags Details
Testcase with round borders for all valid (CSS 2.1) border styles (870 bytes, text/html)
2012-10-19 12:30 PDT, Marco Perez
no flags Details
shows boder-radius bug with radius 50% (137 bytes, text/html)
2014-03-06 11:05 PST, Bill Reed
no flags Details
newtab_dashed.png (92.41 KB, image/png)
2014-10-16 07:27 PDT, Elbart
no flags Details
outline_customization.png (55.38 KB, image/png)
2014-11-21 03:00 PST, Elbart
no flags Details
(WIP) Support dotted/dashed border-radiused corners. (35.20 KB, patch)
2015-07-21 04:35 PDT, Tooru Fujisawa [:arai]
no flags Details | Diff | Splinter Review
static test for various width (3.98 KB, text/html)
2015-07-21 04:36 PDT, Tooru Fujisawa [:arai]
no flags Details
controllable test (3.61 KB, text/html)
2015-07-21 04:37 PDT, Tooru Fujisawa [:arai]
no flags Details
result of WIP patch (264.31 KB, image/png)
2015-07-21 04:39 PDT, Tooru Fujisawa [:arai]
no flags Details
(WIP 2) Support dotted/dashed border-radiused corners. (56.56 KB, patch)
2015-07-22 08:54 PDT, Tooru Fujisawa [:arai]
no flags Details | Diff | Splinter Review
result of WIP 2 patch (232.35 KB, image/png)
2015-07-22 08:55 PDT, Tooru Fujisawa [:arai]
no flags Details
Rough algorithm used by the WIP 2 path (710.68 KB, image/jpeg)
2015-07-22 13:35 PDT, Tooru Fujisawa [:arai]
no flags Details
Design issue on dotted corner (84.68 KB, image/jpeg)
2015-07-22 13:37 PDT, Tooru Fujisawa [:arai]
no flags Details
Possible improvement for dashed corner joint (187.47 KB, image/jpeg)
2015-07-23 03:28 PDT, Tooru Fujisawa [:arai]
no flags Details
(WIP 3) Support dotted/dashed border-radiused corners. (56.95 KB, patch)
2015-07-25 06:14 PDT, Tooru Fujisawa [:arai]
no flags Details | Diff | Splinter Review
result of WIP 3 patch (546.10 KB, image/png)
2015-07-25 06:15 PDT, Tooru Fujisawa [:arai]
no flags Details
dot size and dash length issue (137.65 KB, image/jpeg)
2015-07-25 15:09 PDT, Tooru Fujisawa [:arai]
no flags Details
(WIP 4) Support dotted/dashed border-radiused corners. (64.01 KB, patch)
2015-07-28 16:58 PDT, Tooru Fujisawa [:arai]
no flags Details | Diff | Splinter Review
result of WIP 4 patch (564.55 KB, image/png)
2015-07-28 16:59 PDT, Tooru Fujisawa [:arai]
no flags Details
static test for various width (25.40 KB, text/html)
2015-07-29 10:54 PDT, Tooru Fujisawa [:arai]
no flags Details
(WIP 5) Support dotted/dashed border-radiused corners. (70.59 KB, patch)
2015-08-01 09:09 PDT, Tooru Fujisawa [:arai]
no flags Details | Diff | Splinter Review
static test for various width (26.96 KB, text/html)
2015-08-01 09:23 PDT, Tooru Fujisawa [:arai]
no flags Details
result of WIP 5 patch (580.33 KB, image/png)
2015-08-01 09:25 PDT, Tooru Fujisawa [:arai]
no flags Details
(WIP 6) Support dotted/dashed border-radiused corners. (72.35 KB, patch)
2015-08-05 20:55 PDT, Tooru Fujisawa [:arai]
no flags Details | Diff | Splinter Review
static test for various width (10.74 KB, text/html)
2015-08-05 20:56 PDT, Tooru Fujisawa [:arai]
no flags Details
controllable test (5.94 KB, text/html)
2015-08-05 20:57 PDT, Tooru Fujisawa [:arai]
no flags Details
result of WIP 6 patch (2.57 MB, image/png)
2015-08-05 21:00 PDT, Tooru Fujisawa [:arai]
no flags Details
(WIP 7) Support dotted/dashed border-radiused corners. (72.69 KB, patch)
2015-08-13 14:25 PDT, Tooru Fujisawa [:arai]
no flags Details | Diff | Splinter Review
result of WIP 7 patch (2.59 MB, image/png)
2015-08-13 14:25 PDT, Tooru Fujisawa [:arai]
no flags Details
DottedCornerFinder's behavior (357.22 KB, image/png)
2015-08-13 14:27 PDT, Tooru Fujisawa [:arai]
no flags Details
DashedCornerFinder's behavior (175.91 KB, image/png)
2015-08-13 14:27 PDT, Tooru Fujisawa [:arai]
no flags Details
corner interaction between dotted and other side (276.11 KB, image/png)
2015-08-13 14:28 PDT, Tooru Fujisawa [:arai]
no flags Details
(WIP 8) Support dotted/dashed border-radiused corners. (213.14 KB, patch)
2015-08-30 18:25 PDT, Tooru Fujisawa [:arai]
no flags Details | Diff | Splinter Review
static test for various width (17.71 KB, text/html)
2015-08-30 18:26 PDT, Tooru Fujisawa [:arai]
no flags Details
result of WIP 8 patch (2.68 MB, image/png)
2015-08-30 18:26 PDT, Tooru Fujisawa [:arai]
no flags Details
Another design issue in dotted side with too-short length (29.26 KB, image/png)
2015-08-30 18:27 PDT, Tooru Fujisawa [:arai]
no flags Details
Rendering issue with border-radius extends into the opposite side (61.26 KB, image/png)
2015-08-30 18:28 PDT, Tooru Fujisawa [:arai]
no flags Details
(WIP 9) Support dotted/dashed border-radiused corners. (212.92 KB, patch)
2015-09-01 03:13 PDT, Tooru Fujisawa [:arai]
no flags Details | Diff | Splinter Review
result of WIP 9 patch (2.68 MB, image/png)
2015-09-01 03:14 PDT, Tooru Fujisawa [:arai]
no flags Details
Part 0: Add missing includes and namespaces. (10.52 KB, patch)
2015-09-10 08:47 PDT, Tooru Fujisawa [:arai]
no flags Details | Diff | Splinter Review
Part 1: Fix spacing of simple 2px dotted border without radius. (1.12 KB, patch)
2015-09-10 08:52 PDT, Tooru Fujisawa [:arai]
no flags Details | Diff | Splinter Review
Rendering result of 2px dotted border (4.88 KB, image/png)
2015-09-10 08:53 PDT, Tooru Fujisawa [:arai]
no flags Details
Part 2: Split constants for border rendering to BorderConsts.h. (3.93 KB, patch)
2015-09-10 08:54 PDT, Tooru Fujisawa [:arai]
no flags Details | Diff | Splinter Review
Part 3: Improve spacing and corner interaction of dashed/dotted border. (51.97 KB, patch)
2015-09-10 09:05 PDT, Tooru Fujisawa [:arai]
no flags Details | Diff | Splinter Review
Part 4: Support dotted/dashed border-radiused corners. (90.42 KB, patch)
2015-09-10 09:07 PDT, Tooru Fujisawa [:arai]
no flags Details | Diff | Splinter Review
Part 5: Add testcases for dashed/dotted borders. (79.69 KB, patch)
2015-09-10 09:08 PDT, Tooru Fujisawa [:arai]
no flags Details | Diff | Splinter Review
Rendering result of static test (2.68 MB, image/png)
2015-09-10 09:08 PDT, Tooru Fujisawa [:arai]
no flags Details
Two dots at the corner with small radius and same border-width. (66.09 KB, image/png)
2015-09-16 10:57 PDT, Tooru Fujisawa [:arai]
no flags Details
Part 3: Improve spacing and corner interaction of dashed/dotted border. (55.82 KB, patch)
2015-09-17 03:00 PDT, Tooru Fujisawa [:arai]
jmuizelaar: review+
Details | Diff | Splinter Review
Part 4: Support dotted/dashed border-radiused corners. (91.79 KB, patch)
2015-09-17 03:00 PDT, Tooru Fujisawa [:arai]
jmuizelaar: review-
Details | Diff | Splinter Review
Part 5: Add testcases for dashed/dotted borders. (79.76 KB, patch)
2015-09-17 03:01 PDT, Tooru Fujisawa [:arai]
jmuizelaar: review+
Details | Diff | Splinter Review
Rendering result of static test (2.70 MB, image/png)
2015-09-17 03:01 PDT, Tooru Fujisawa [:arai]
no flags Details
Time taken by nsCSSBorderRenderer::DrawBorders (18.12 KB, text/html)
2015-09-22 13:02 PDT, Tooru Fujisawa [:arai]
no flags Details
Rendering result of "Time taken by nsCSSBorderRenderer::DrawBorders" file (219.61 KB, image/png)
2015-09-22 13:03 PDT, Tooru Fujisawa [:arai]
no flags Details
Time taken by nsCSSBorderRenderer::DrawBorders (25.07 KB, text/html)
2015-09-22 14:24 PDT, Tooru Fujisawa [:arai]
no flags Details
Rendering result of "Time taken by nsCSSBorderRenderer::DrawBorders" file (230.20 KB, image/png)
2015-09-22 14:25 PDT, Tooru Fujisawa [:arai]
no flags Details
Time taken by nsCSSBorderRenderer::DrawBorders (with Part 1-3) (31.56 KB, text/html)
2015-10-13 21:35 PDT, Tooru Fujisawa [:arai]
no flags Details
Rendering result of "Time taken by nsCSSBorderRenderer::DrawBorders (with Part 1-3)" file with Part 1-3 (256.46 KB, image/png)
2015-10-13 22:10 PDT, Tooru Fujisawa [:arai]
no flags Details
Part 0-followup: Add missing includes and namespaces. (3.87 KB, patch)
2015-11-07 12:37 PST, Tooru Fujisawa [:arai]
jmuizelaar: review+
Details | Diff | Splinter Review
Part 4: Support dotted/dashed border-radiused corners. (95.94 KB, patch)
2015-12-26 20:11 PST, Tooru Fujisawa [:arai]
jmuizelaar: review+
Details | Diff | Splinter Review
Part 0: Add missing includes and namespaces. (9.18 KB, patch)
2015-12-26 20:14 PST, Tooru Fujisawa [:arai]
jmuizelaar: review+
Details | Diff | Splinter Review
Part 0-followup: Add missing includes and namespaces. r=jrmuizel (2.57 KB, patch)
2015-12-26 20:15 PST, Tooru Fujisawa [:arai]
arai.unmht: review+
Details | Diff | Splinter Review
Part 1: Fix spacing of simple 2px dotted border without radius. (1.12 KB, patch)
2015-12-26 20:16 PST, Tooru Fujisawa [:arai]
jmuizelaar: review+
Details | Diff | Splinter Review
Part 2: Split constants for border rendering to BorderConsts.h. (3.93 KB, patch)
2015-12-26 20:17 PST, Tooru Fujisawa [:arai]
jmuizelaar: review+
Details | Diff | Splinter Review
Part 3: Improve spacing and corner interaction of dashed/dotted border. r=jrmuizel (55.83 KB, patch)
2015-12-26 20:17 PST, Tooru Fujisawa [:arai]
arai.unmht: review+
Details | Diff | Splinter Review
Part 6: Flush path while building long dotted/dashed border. (4.64 KB, patch)
2016-06-09 08:29 PDT, Tooru Fujisawa [:arai]
jmuizelaar: review+
Details | Diff | Splinter Review
Part 7: Render only dirty area for dotted side and dotted/dashed corner. (16.57 KB, patch)
2016-06-09 08:31 PDT, Tooru Fujisawa [:arai]
no flags Details | Diff | Splinter Review
Part 8: Render too large dotted/dashed corner with solid style. (6.93 KB, patch)
2016-06-09 08:34 PDT, Tooru Fujisawa [:arai]
jmuizelaar: review+
Details | Diff | Splinter Review
Part 9: Warn about too large dotted/dashed corner. (11.19 KB, patch)
2016-06-09 08:35 PDT, Tooru Fujisawa [:arai]
no flags Details | Diff | Splinter Review
Part 9: Warn about too large dotted/dashed corner. (11.44 KB, patch)
2016-06-09 10:36 PDT, Tooru Fujisawa [:arai]
jmuizelaar: review-
Details | Diff | Splinter Review
Part 7: Render only dirty area for dotted side and dotted/dashed corner. (12.72 KB, patch)
2016-06-09 17:09 PDT, Tooru Fujisawa [:arai]
jmuizelaar: review+
Details | Diff | Splinter Review
Part 9: Warn about too large dotted/dashed corner. (12.57 KB, patch)
2016-06-09 17:11 PDT, Tooru Fujisawa [:arai]
jmuizelaar: review+
Details | Diff | Splinter Review
Rendering problem (51.74 KB, image/jpeg)
2016-11-18 00:49 PST, Massamino
no flags Details

Description Gábor Stefanik 2007-05-31 16:53:55 PDT
Created attachment 266827 [details]
Testcase, dotted circle is rendered as solid.

QUOTE(Vladimir Vukicevic, 2007-05-27 21:16:10 PDT, Bug 368247 comment #30):
"-moz-border-radius: 100% will not result in
a dotted/dashed circle, because the entire radius is considered a "corner". 
The latter (well, both) could be fixed at some later time if someone's bored."
This bug is looking for "bored" people who could fix that. Testcase is attached.
Comment 1 Vladimir Vukicevic [:vlad] [:vladv] 2007-06-03 22:17:40 PDT
This is not a regression; Firefox 2 ignores dotted/dashed on borders with border-radius.
Comment 2 Gábor Stefanik 2007-09-27 13:22:35 PDT
Changing summary because this is not a regression. Note however that the first approach of bug 368247 would have fixed this.
Comment 3 Thomas Goldstein 2008-09-03 09:44:36 PDT
I've had both bugs in my votes for a while, and I've just noticed this now... but isn't this bug a dupe of bug 13944?
Comment 4 Gábor Stefanik 2008-09-03 10:11:34 PDT
Bug 13944 is essentially fixed now - it was about -moz-border-radius completely disabling dotted/dashed borders. This bug is about only the corners being solid.
Comment 5 Marco Perez 2009-03-29 13:19:40 PDT
Created attachment 369916 [details]
Testcase with round borders for all possible border styles.
Comment 6 Ria Klaassen (not reading all bugmail) 2010-01-17 08:04:52 PST
*** Bug 540276 has been marked as a duplicate of this bug. ***
Comment 7 Alexandre Folle de Menezes 2010-03-16 19:35:42 PDT
This works on the new IE9 preview.  Does it count as an "IE parity" blocker?
It also works on Opera 10.5, and does not work in Safari 4 and Chrome 4.
Comment 8 d 2010-03-23 17:53:52 PDT
It should, in my opinion, be marked as parity blockers, yes. Also, what are the actual issue(s) that makes fixing this hard?
Comment 9 Zack Weinberg (:zwol) 2010-06-21 13:26:09 PDT
(In reply to comment #8)
> what are the actual issue(s) that makes fixing this hard?

We need an algorithm to place the dots and dashes around the curve, and smoothly scale their sizes, and so on.  We don't have one.  Several people have tried and failed to come up with one.
Comment 10 fantasai 2010-06-21 13:47:38 PDT
I drew up a math problem to model this, here:
  http://fantasai.inkedblade.net/style/discuss/border-transitions/find-the-dots
I posted it to a bunch of my friends, and one of them came up with a Python script and some comments. I haven't looked it over, but here's his comment:

====== Message from Michael O'Kelly ======

See pretty pictures:

http://sharing.highlightcam.com.s3.amazonaws.com/ellipses3a.png
http://sharing.highlightcam.com.s3.amazonaws.com/ellipses3b.png
http://sharing.highlightcam.com.s3.amazonaws.com/ellipses3c.png

Code:
http://sharing.highlightcam.com.s3.amazonaws.com/ellipses3.py

First finds a chain of circles assuming m=0, then uses the resulting overlap to calculate an optimal m. Inner loops mostly use rapidly-converging searches, along the lines of Victor Shnayder's proposal. They could be replaced with algebraic solutions (e.g. Jon Snitow's) for dramatic speedup.

This code doesn't make any assumptions about the input curves--they could be ellipses or anything sufficiently smooth.

====== End Message ======

The key thing to note is that the centers of the circles don't lie on an ellipse:
  http://sharing.highlightcam.com.s3.amazonaws.com/ellipses2.png

I believe James Socol has some of the actual math worked out, but he says it's very messy and doesn't have a full solution.

Maybe that helps, maybe not, but I thought I'd post it here. :)
Comment 11 Zéfling 2010-06-21 15:56:27 PDT
I wrote some tests and in all web engines the result is little strange:  
http://ikilote.net/Programmation/CSS/Test/borders_radius_1.htm
Comment 12 Zack Weinberg (:zwol) 2010-06-21 16:01:23 PDT
(In reply to comment #11)
> I wrote some tests and in all web engines the result is little strange:  
> http://ikilote.net/Programmation/CSS/Test/borders_radius_1.htm

That doesn't surprise me.  Would you be willing to draw, with SVG or whatever, suggested renderings of each of those test boxes?  That would be quite helpful.

Be aware of bug 19963, which will also cause poor rendering of these examples - probably it should be a dependency, come to think.
Comment 13 Zéfling 2010-06-21 17:32:24 PDT
SVG like this ? :
http://ikilote.net/Programmation/CSS/Test/borders_radius_1_test_1.svg

I draw this (box1, 2, 5 & 6) rapidly... for example.
Comment 14 Anthony Ricaud (:rik) 2010-06-29 02:52:45 PDT
Just for the record, WebKit is currently working on this problem. See https://bugs.webkit.org/show_bug.cgi?id=9197 and http://trac.webkit.org/changeset/62035
Comment 15 Benjamin CHERY 2011-02-01 02:14:40 PST Comment hidden (spam)
Comment 16 John Wilander 2011-08-16 15:23:02 PDT Comment hidden (spam)
Comment 17 fantasai 2011-08-16 17:10:17 PDT
Benjamin, John, please do not post comments in bugs unless they are actively contributing towards fixing the bug. It clutters the bug report and unnecessarily spams everyone on the CC list. See
  https://bugzilla.mozilla.org/page.cgi?id=etiquette.html
Comment 18 Darren 2012-10-19 05:47:02 PDT Comment hidden (spam)
Comment 19 Marco Perez 2012-10-19 12:30:00 PDT
Created attachment 673358 [details]
Testcase with round borders for all valid (CSS 2.1) border styles

Support for -moz-border-radius has been removed starting with Firefox 13.
Updating the testcase and hiding invalid ones. Sorry for the spam.
Comment 20 Sterling Camden 2012-11-07 11:46:40 PST Comment hidden (spam)
Comment 21 i_sanjurjo 2012-12-04 04:44:33 PST Comment hidden (spam)
Comment 22 Beanow 2013-02-18 02:50:21 PST Comment hidden (spam)
Comment 23 sebas 2013-05-17 07:27:33 PDT Comment hidden (spam)
Comment 24 Evyatar 'Tron' Amitay (everything.me) 2013-07-17 01:03:27 PDT Comment hidden (spam)
Comment 25 u476579 2013-07-30 22:58:52 PDT Comment hidden (spam)
Comment 26 Erwann Mest 2013-08-03 10:39:02 PDT Comment hidden (spam)
Comment 27 Erwann Mest 2013-08-03 10:39:47 PDT Comment hidden (spam)
Comment 28 Christophe Verbinnen 2013-08-15 13:04:18 PDT Comment hidden (spam)
Comment 29 Mats Palmgren (:mats) 2013-12-09 10:23:10 PST
*** Bug 947833 has been marked as a duplicate of this bug. ***
Comment 30 Kumar Harsh 2013-12-29 04:56:42 PST Comment hidden (spam)
Comment 31 Bill Reed 2014-03-06 11:05:57 PST
Created attachment 8386965 [details]
shows boder-radius bug with radius 50%

When the border-radius is 50%, making a circle the border is rendered as solid
Comment 32 Alex Oshchepkov 2014-05-07 12:18:56 PDT Comment hidden (spam)
Comment 33 rsx11m 2014-05-17 06:30:41 PDT
(In reply to fantasai from comment #10)
> http://fantasai.inkedblade.net/style/discuss/border-transitions/find-the-dots

This first link is still active, but unfortunately

> ====== Message from Michael O'Kelly ======
> http://sharing.highlightcam.com.s3.amazonaws.com/ellipses3a.png
> http://sharing.highlightcam.com.s3.amazonaws.com/ellipses3b.png
> http://sharing.highlightcam.com.s3.amazonaws.com/ellipses3c.png
> http://sharing.highlightcam.com.s3.amazonaws.com/ellipses3.py
> http://sharing.highlightcam.com.s3.amazonaws.com/ellipses2.png

are all gone now. Did anyone make copies to attach here to this bug, especially of the Python code?

> I believe James Socol has some of the actual math worked out, but he says
> it's very messy and doesn't have a full solution.

It might be helpful to get a hold on that draft as well, even if not fully developed. Someone may have an idea where to go from here. But then, all of this was in 2010 and may be lost by now.  :-\
Comment 34 u476579 2014-05-17 09:53:40 PDT Comment hidden (spam)
Comment 35 rsx11m 2014-05-17 10:08:20 PDT
Patt, this wasn't the kind of feedback I wanted to solicit with my comment #33, and not really helpful towards solving this issue. There are plenty of old (and older) bugs here. The point is that someone will have to come up with an approach to smoothly connect straight and curved parts of the border, either conceptually or in some pseudo or scripted code, and eventually as a patch.
Comment 36 Elbart 2014-05-17 11:04:15 PDT
Why not take a look at how Google solved it in Chromium? No point in reinventing the wheel.
Comment 37 Boris Zbarsky [:bz] (still a bit busy) 2014-05-19 13:54:58 PDT
*** Bug 1012868 has been marked as a duplicate of this bug. ***
Comment 38 nemo 2014-05-19 13:58:06 PDT
WRT comment #36 - Elbart, I'd just as soon not do Chrome's rendering 'cause it is kinda unattractive.  Compare http://m8y.org/tmp/testcase375.xhtml in IE9/10/11 vs Chrome.  Has a notch cut out of it.  I think IE is intelligently adjusting the dash size or something.
Comment 39 mtr 2014-05-20 00:03:26 PDT
(In reply to rsx11m from comment #35)
> The point is that someone will have to come up
> with an approach to smoothly connect straight and curved parts of the
> border, either conceptually or in some pseudo or scripted code, and
> eventually as a patch.

Any support is better than none. Do we have to aim at "smooth" solution ? See Chrome example. It's not perfect but it is usable.
Comment 40 Alex Oshchepkov 2014-05-20 00:12:42 PDT
What don't you like about chrome's rendering?  http://m8y.org/tmp/testcase375.xhtml is marginal.

In IE dashed borders in real life cases (like text input borders) seem more like dotted.
Comment 41 nemo 2014-05-20 05:55:51 PDT
Well, what I clearly don't like about chrome's rendering is it causes gaps.  The circles is one example, but I don't see that as "marginal"

IE's is maybe not ideal because they might allow the dashes to become too small, but that doesn't seem intrinsic to avoiding gaps.

That said, if it isn't convenient to do something similar to IE's approach and copying the code is more convenient, by all means do chrome's.  Was just an opinion that IE's is more attractive and and handles more cases nicely.
Comment 42 HHahn 2014-05-23 05:40:26 PDT
On http://forums.mozillazine.org/viewtopic.php?f=9&t=2834003 it was suggested that I post my solution proposal here. As I don't know how to post a document here, I try to copy the entire text here, hoping that the layout will not be disturbed too much:

Suggestions for solving the dotted curves problem

Unlike Chrome, Internet Explorer, Opera and Safari, Firefox v29 still renders dotted curves (e.g. rounded corners of boxes) as solid lines. I complained about this at http://forums.mozillazine.org/viewtopic.php?f=9&t=2834003. However, they suggested that I post my ideas about a possible solution here.

1. The problem
When a box is given a dotted border, the straight border sections are correctly dotted. However, when the corners are given a radius, the curved parts are rendered solid instead. This is said to be due to a lack of a good rendering algorithm for dotted curves. I don't have an algorithm either, but I do have ideas for how to possibly find a solution.

2. Suggestions for solutions
a.	For the time being, I would rather not bother too much about accuracy. Anything is better than the cur-rent situation. Also, a box is not a stand-alone item; it will always be part of a page that contains more information, which will distract much of the reader’s from tiny inaccuracies – but obviously not from the big ones we have now. At a later stage, the solution might be further improved, if need be.
b.	The exact dot size is more important than the exact dot spacing. The latter may be varied slightly in order to make ‘the ends meet’ correctly.
c.	There is no need that the straight sections begin and end exactly at the centre of a dot! If they did, a box with four curved corners would need exact ‘fitting’ at eight positions – two for each curved corner. This is not necessary. It would be quite acceptable if all dots flow continuously along the border until the ends meet at one point.

3. Suggestions for algorithms
I would describe the lines and curves in terms of analytical geometry. Dots will be placed on ‘carrier paths’, which may be any desired shape, and will normally be invisible. When one dot has been positioned, the next dot position can be calculated by moving along the carrier line. Depending on the situation, it may be on the same carrier path section, or on the next one. This is repeated until either the line ends, or the starting point is reached. (I don’t know if curves crossing itself – e.g. a lemniscate – are possible. If so, then be sure that the crossing point will be passed twice before the process is considered complete. If need be, fine-tuning – having only one dot at the crossing point – can be solved at a later stage.)

For a curved carrier path section, there are two ways to calculate the next dot position:

a.	Along the curve itself: Walk along this circle either over an angle of rotation equal to the spacing needed divided by the border radius (if the curve is a circle), or in small steps until the required spacing is reached.

b.	Along the secant line to the next point: Draw a circle, centred at the current dot position, and with a ra-dius equal to the spacing needed. The next point is at the intersection of this circle and the carrier path.

Method a may result in a visually slightly smaller dot spacing, as the (invisible) actual carrying line is circu-lar, whereas in method b it is straight. I don’t expect this to be a problem; or otherwise it may be simply compensated by a tiny increase in the dot spacing in a later version. At this moment, a basic solution is ur-gently needed; if need be, fine-tuning may be achieved in a later version.

4. Fine-tuning the result
First, calculate the actual circumference length, without actually placing the dots. Then divide this circumference by the sum of the required dot size and dot spacing. If the result is not an integer number, then round it to the nearest integer. This rounded result is the actual number of dots needed. The circumference divided by this number of dots produces the actual sum of dot size and dot spacing needed. The dots can then be placed as described in section 3 above, using this modified ‘dot frequency’ (= dot size plus dot spacing).

If this fine-tuning is a problem (which I can hardly believe), this fine-tuning might be delayed to a later version. It is too bad that for more than a decade or so this problem has persisted in a browser that has a reputation of being the best W3C-compliant browser available. This problem should be resolved at least basically as soon as possible.

H. Hahn
Comment 43 Florian Bender 2014-05-23 23:17:48 PDT
Can you provide a patch (see https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide for instructions), or failing that provide an implementation of the algorithm that is as close to being integrated into the source as possible? You may be able to find the current code at dxr.mozilla.org or mxr.mozilla.org. Thanks!
Comment 44 HHahn 2014-05-24 05:19:37 PDT
I'm afraid I cannot. I never developed browsers or anything alike. It would take far too much time to study dig up how it currently works or how to describe it in more detail so that you could use it as a basis for further work on it.
If there are details in my text that are not clear, then you may of course ask me for further explanation. I would rather not dive into coding issues myself.

I just described what I think is a logical approach to the problem.
After all, if all other browsers did solve the problem, it cannot be that complicated?

Anyway, I would emphasize again that fine-tuning is not the most urgent problem. A basic solution would do for the time being.
Comment 45 sjw 2014-05-24 11:55:15 PDT
The testcases in attachment 673358 [details] are very simple and the most common cases.
While fixing this bug, we should keep an eye on more complex use cases, like diffent radius and styles in one box and may be fix them later in a further step.
Comment 46 HHahn 2014-05-24 12:18:44 PDT
@sjw@gmx.ch:
Different radiuses should not be more complicated. Just "follow the curve", whatever the radius at the current section. It does not even need to be circular. Start with a temporary solid line; its centreline will be the carrier path. When all dots are positioned, the centreline may be removed or made invisible.
Comment 47 HHahn 2014-05-24 12:26:18 PDT
One more point: Firefox already correctly rotates objects if asked to do so. Apparently it can rotate an object along a specified centre point, over a specified angle. That is just what must be done to "follow a curve": begin to move straight ahead over a very small distance delta (x), and then "rotate" the last-drawn "delta (x)" by its starting point just as far as needed to get back on the curve. Repeat this until the sum of all delta (x) steps equals the required do spacing. Then drop the next dot. Etcetera.
Comment 48 Elbart 2014-10-16 07:27:29 PDT
Created attachment 8506150 [details]
newtab_dashed.png

As of Firefox 33, about:newtab is using a rounded dash-border to signal an empty tile-space. Thus this bug has been made more obvious to users.
Comment 49 Greg Edwards 2014-10-16 07:39:22 PDT
Even an imperfect algorithm would be better than nothing at this point.
Comment 50 Florian Bender 2014-10-23 12:37:31 PDT
(In reply to Elbart from comment #48)
> As of Firefox 33, about:newtab is using a rounded dash-border to signal an
> empty tile-space. Thus this bug has been made more obvious to users.

Though this is a platform bug, putting it on the backlog since it affects Frontend.
Comment 51 :Gavin Sharp [email: gavin@gavinsharp.com] 2014-10-31 15:20:15 PDT
We don't use firefox-backlog for that kind of "Firefox wanted" tracking.
Comment 52 Elbart 2014-11-21 03:00:24 PST
Created attachment 8526673 [details]
outline_customization.png

For the sake of completeness: outline is also affected, as seen in the placeholders in the right panel of the Customization-window.

http://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/customizableui/customizeMode.inc.css?rev=3adeab7e472d#22
Comment 53 Igor 2014-12-25 05:09:29 PST Comment hidden (spam)
Comment 54 :Gijs Kruitbosch 2015-01-13 07:17:07 PST
*** Bug 1120933 has been marked as a duplicate of this bug. ***
Comment 55 Bhumi 2015-04-28 10:26:09 PDT Comment hidden (spam)
Comment 56 Kumar Harsh 2015-04-28 23:04:48 PDT
To everyone who is saying the "Just follow Chrome/ium", I say "Please, NO!"

For dotted borders, Chrome renders ugly square bits. Although for 1px thick borders, most of the users would not notice, but designers would. And, if the borders get any thicker than 2px, it is apparent to everyone. And square bits look fugly to say it lightly.

Any updates on how far we are into this? If a perfect solution is not possible at the moment, perhaps a quick solution would be OK, perhaps it can be improved with feedback in the FF developer edition rather than not implementing anything.
Comment 57 nemo 2015-04-29 04:44:57 PDT
Agreed on prior comment.  
While Firefox is outright buggy, Chrome is the most unattractive.  Safari is a bit better than Chrome, and IE seems to do the best at spacing and rendering dots/dashes.
Comment 58 Neil Robinson 2015-04-29 08:43:02 PDT Comment hidden (spam)
Comment 59 nemo 2015-04-29 08:55:41 PDT Comment hidden (off-topic)
Comment 60 Neil Robinson 2015-04-29 09:15:08 PDT Comment hidden (off-topic)
Comment 61 Kumar Harsh 2015-04-29 09:37:11 PDT Comment hidden (off-topic)
Comment 62 Neil Robinson 2015-04-29 09:45:23 PDT Comment hidden (off-topic)
Comment 63 nathan.noom 2015-04-29 10:02:54 PDT Comment hidden (spam)
Comment 64 HHahn 2015-04-30 05:34:38 PDT
I have seen several Firefox "updates" coming along during the last half year or so, but often I was wondering what was the actual urgency of the problems they solved.
I am glad to see now that more and more people agree that the dotted-curve problem is getting really urgent. The way it is now, is a real shame for what is considered the most reliable browser around.
Now who is going to solve it? Once again, not everybody writing here is sufficiently familiar with ins and outs of the Firefox innards. So not everyone of us is able to actually solve the problem. But there must be people who are -- it is they who produce all those tiny improvements we do get. So why not tackle this ugly-curve problem as soon as possible?
Comment 65 phaikawl 2015-07-04 19:51:59 PDT
There's an easy way to make it render fine for the majority of use cases without having to implement something complicated.
If the box has equal width on all corners, we could draw the rounded rectangle with a dotted/dashed pen, this is easy. Otherwise we resort to the current painting.
People rarely use different widths for the corners a lot in the real world, even if they want, they probably don't need to.
Comment 66 Tooru Fujisawa [:arai] 2015-07-21 04:35:34 PDT
Created attachment 8636535 [details] [diff] [review]
(WIP) Support dotted/dashed border-radiused corners.

Here is a WIP patch (just for record), there should be a lot of space to improve accuracy/performance, and cleanup, but at least it works in most cases.

I'll post the some testcases and rendering result.
Comment 67 Tooru Fujisawa [:arai] 2015-07-21 04:36:21 PDT
Created attachment 8636537 [details]
static test for various width
Comment 68 Tooru Fujisawa [:arai] 2015-07-21 04:37:24 PDT
Created attachment 8636538 [details]
controllable test
Comment 70 Tooru Fujisawa [:arai] 2015-07-21 06:20:13 PDT
Just noticed following note.

https://drafts.csswg.org/css-backgrounds-3/#border-style
> Note: There is no control over the spacing of the dots and dashes,
> nor over the length of the dashes. Implementations are encouraged to
> choose a spacing that makes the corners symmetrical.

If we tweak the spacing of each edge to make them symmetric, corner implementation will become simpler :)
I'll do it first, then will bring some things to discuss.
Comment 71 Tooru Fujisawa [:arai] 2015-07-22 08:54:24 PDT
Created attachment 8637263 [details] [diff] [review]
(WIP 2) Support dotted/dashed border-radiused corners.

Several improvement from previous one.

* variable overlap for dotted
* variable dash length for dashed
* symmetric border side
* merge dotted+dotted corner with same width and no radius into single dot
Comment 72 Tooru Fujisawa [:arai] 2015-07-22 08:55:56 PDT
Created attachment 8637265 [details]
result of WIP 2 patch
Comment 73 Tooru Fujisawa [:arai] 2015-07-22 13:35:07 PDT
Created attachment 8637436 [details]
Rough algorithm used by the WIP 2 path

Attached image describes the way the patch uses.  There're several issues for now.

One of the big issue is the performance.
Currently there're nested binary seaches, up to 5 loops nesting, so it's heavy when border-width is thin and curve is long.  I guess some of them could be solved mathematically, by directly calculating the value instead of searching, and others may be improved by applying more linear approximation.  Any hint is welcome :)
Comment 74 Tooru Fujisawa [:arai] 2015-07-22 13:37:52 PDT
Created attachment 8637440 [details]
Design issue on dotted corner

Then, here's a design issue in dotted corner
In attached image, to fill the corner area with circles, there can be a circle which is smaller than circles at both ends, even if both border-width is same.
Does anyone have any opinion on this, or have a reference image?
Comment 75 fantasai 2015-07-22 14:30:12 PDT
Have you looked at the patches from Bug 652650 ? They might have some usefuless.
Comment 76 Robin Whittleton 2015-07-23 00:43:44 PDT
From the look of your renderings this work will fix bug #19963 as well?

Also, thank you for your these patches. I can’t comment on their technical proficiency, but I love the diagrams explaining the algorithm you’re using.
Comment 77 Tooru Fujisawa [:arai] 2015-07-23 03:28:39 PDT
Created attachment 8637818 [details]
Possible improvement for dashed corner joint

(In reply to fantasai from comment #75)
> Have you looked at the patches from Bug 652650 ? They might have some
> usefuless.

Thank you for letting me know that :D
Yes, it helps a lot.  So, you used elliptic arc instead of cubic bezier curve for calculation.  It may be simpler than cubic bezier (yeah, this cubic bezier itself is the approximation of elliptic arc, but original elliptic arc will be suitable for some case), I'll try it.

Also, the testcase there revealed that we need half offset for entire dashed side and corner.
  data:text/html;charset=utf-8,<!DOCTYPE html>%0D%0A<style>%0D%0A  p { height%3A 100px%3B width%3A 200px%3B border-width%3A 10px 20px%3B border-color%3A orange blue%3B border-radius%3A 60px%3B border-style%3A dashed%3B }%0D%0A<%2Fstyle>%0D%0A<p>and


btw, there exists the code which calculates gradient color for corner, but it's not a required thing now, right?

https://drafts.csswg.org/css-backgrounds-3/#changes-2009
> 9.6. Changes Since the 17 December 2009 Candidate Recommendation
> Removal of recommendation to use gradients for color transitions when
> ‘border-radius’ produces a curve.
Comment 78 Tooru Fujisawa [:arai] 2015-07-23 03:48:07 PDT
(In reply to Robin Whittleton from comment #76)
> From the look of your renderings this work will fix bug #19963 as well?

This patch follows the rule in the bug only in dotted+dotted with same border-width and no border-radius case.
"24 dotted" case in attachment 8637265 [details] is this case.

For other cases that dot overlaps with other side, this patch joints them with solid corner (may have small radius).
like "16 dotted 24 dotted" case in attachment 8637265 [details], which draws heart mark at every corner, that is half dots in both side, and solid rectangle in the corner.

For example, the dotted+solid case is differ from attachment 323815 [details], with this patch, it's drawn as "16 solid 24 dotted" case.
Comment 79 Tooru Fujisawa [:arai] 2015-07-25 06:14:17 PDT
Created attachment 8638919 [details] [diff] [review]
(WIP 3) Support dotted/dashed border-radiused corners.

Fixed the dashed and several minor rendering, and some cleanup.
Not yet touched to performance.
Comment 80 Tooru Fujisawa [:arai] 2015-07-25 06:15:03 PDT
Created attachment 8638920 [details]
result of WIP 3 patch
Comment 81 Leon Sorokin 2015-07-25 11:27:00 PDT
@Tooru thanks for your work so far.

In the latest example I'm finding the simple dashed/dotted rendering to be strange. The corner dots or dashes appear smaller and/or offset inward by 1px. The dot at -90deg is also stretched rather than staying uniform while the spacing is adjusted instead.

See: https://i.imgur.com/7cbmmLf.png

Thanks!
Comment 82 Tooru Fujisawa [:arai] 2015-07-25 15:09:52 PDT
Created attachment 8638960 [details]
dot size and dash length issue

(In reply to Leon Sorokin from comment #81)
> @Tooru thanks for your work so far.
> 
> In the latest example I'm finding the simple dashed/dotted rendering to be
> strange. The corner dots or dashes appear smaller and/or offset inward by
> 1px. The dot at -90deg is also stretched rather than staying uniform while
> the spacing is adjusted instead.
> 
> See: https://i.imgur.com/7cbmmLf.png
> 
> Thanks!

Thank you for pointing them out.

About the dotted style, the actual issue is that dots in straight side (including -90deg dot) are not perfect circles, but 1px wider, this is an existing bug (in other words, the circles in corner are rendered with expected size).  Currently we're rendering it as dashed line with [0, 2 * borderWidth] pattern and round line cap (0 length line with round line cap in both side, and gap between them),  it's rendered like that.  We might have to find another way to render dotted side, to improve this.  maybe render each circles separately?  I'll try this, and check the performance.

About dashed style, this patch is now calculating the width of dash/gap separately for side and corner.  So side and corner may have different dash/gap width.  Also, in attached case, left side has 1 px length which is rendered as solid border, since no dashed line fits there.  this makes the segment in -90deg a bit longer.  To improve them, I think we have to calculate dashed width and offset for side and corner at the same time (so, we shouldn't joint corner and side with half segment), at least for simple case, like all border has same width, and radii are large enough.
Comment 83 Tooru Fujisawa [:arai] 2015-07-28 16:58:09 PDT
Created attachment 8640231 [details] [diff] [review]
(WIP 4) Support dotted/dashed border-radiused corners.

Addressed the dotted side's issue, and improved performance for some simple cases.
Comment 84 Tooru Fujisawa [:arai] 2015-07-28 16:59:15 PDT
Created attachment 8640232 [details]
result of WIP 4 patch
Comment 85 fantasai 2015-07-28 17:18:12 PDT
(In reply to Tooru Fujisawa [:arai] from comment #77)
> 
> btw, there exists the code which calculates gradient color for corner, but
> it's not a required thing now, right?
> 
> https://drafts.csswg.org/css-backgrounds-3/#changes-2009
> > 9.6. Changes Since the 17 December 2009 Candidate Recommendation
> > Removal of recommendation to use gradients for color transitions when
> > ‘border-radius’ produces a curve.

It is not required, no, because nobody wanted to implement it. But I think if it looks better, then that's better, no?
Comment 86 Tooru Fujisawa [:arai] 2015-07-28 17:29:00 PDT
(In reply to fantasai from comment #85)
> (In reply to Tooru Fujisawa [:arai] from comment #77)
> > 
> > btw, there exists the code which calculates gradient color for corner, but
> > it's not a required thing now, right?
> > 
> > https://drafts.csswg.org/css-backgrounds-3/#changes-2009
> > > 9.6. Changes Since the 17 December 2009 Candidate Recommendation
> > > Removal of recommendation to use gradients for color transitions when
> > > ‘border-radius’ produces a curve.
> 
> It is not required, no, because nobody wanted to implement it. But I think
> if it looks better, then that's better, no?

Yeah, it might be better.  But I'd like to leave it to another bug, since it also affects other styles.
Comment 87 Leon Sorokin 2015-07-29 09:04:55 PDT
WIP 4 looks much better, though the spacing is still somewhat biased towards the top & bottom edges: https://i.imgur.com/wXSIPkc.png
Comment 88 Tooru Fujisawa [:arai] 2015-07-29 10:54:12 PDT
Created attachment 8640602 [details]
static test for various width

testcase used for WIP 4 result
Comment 89 Tooru Fujisawa [:arai] 2015-07-29 12:01:47 PDT
(In reply to Leon Sorokin from comment #87)
> WIP 4 looks much better, though the spacing is still somewhat biased towards
> the top & bottom edges: https://i.imgur.com/wXSIPkc.png

Thank you for pointing that out, the glitch was caused by floating point number handling and optimization for simple case didn't work  (it still exists in semi-optimized case tho).
Comment 90 Tooru Fujisawa [:arai] 2015-07-30 02:50:45 PDT
On Flame, displaying attachment 8640602 [details] takes about 5 seconds to redraw, after scroll.  I guess this is not acceptable performance :/
Comment 91 Tooru Fujisawa [:arai] 2015-08-01 09:09:41 PDT
Created attachment 8642053 [details] [diff] [review]
(WIP 5) Support dotted/dashed border-radiused corners.

Fixed the floating point number handling, and improved dotted overlap/dashed length finding algorithms, not they can find the best value almost within 8 iterations.

This takes 3 or 4 seconds for each redraw on Flame, slightly faster than WIP 4, but still too slow.
btw, is there any proper way to evaluate the rendering performance?
Comment 92 Tooru Fujisawa [:arai] 2015-08-01 09:23:33 PDT
Created attachment 8642055 [details]
static test for various width

added 2 more test case with 0 border-width
Comment 93 Tooru Fujisawa [:arai] 2015-08-01 09:25:35 PDT
Created attachment 8642056 [details]
result of WIP 5 patch
Comment 94 Tooru Fujisawa [:arai] 2015-08-05 20:55:26 PDT
Created attachment 8644116 [details] [diff] [review]
(WIP 6) Support dotted/dashed border-radiused corners.

* improved search algorithms
* fixed bug 19963 (I guess I need to split this patch into several parts later)
  * corner between dotted side and non-dotted side
  * corner between dotted sides with different border widths
  * now no *unintentional* heart mark
* some minor optimization
* cleanup
Comment 95 Tooru Fujisawa [:arai] 2015-08-05 20:56:53 PDT
Created attachment 8644117 [details]
static test for various width

added more tests
Comment 96 Tooru Fujisawa [:arai] 2015-08-05 20:57:53 PDT
Created attachment 8644118 [details]
controllable test

added more options
Comment 97 Tooru Fujisawa [:arai] 2015-08-05 21:00:43 PDT
Created attachment 8644119 [details]
result of WIP 6 patch

so much testcases :)

corner between moz-border-colors is not supported
Comment 98 Tooru Fujisawa [:arai] 2015-08-13 14:25:16 PDT
Created attachment 8647735 [details] [diff] [review]
(WIP 7) Support dotted/dashed border-radiused corners.

can I get some feedback on this?

Changes are following (I'll post some figures shortly):
  * Applied dotted/dashed to border radius corner
    + nsCSSBorderRenderer::DrawDashedCorner
    + nsCSSBorderRenderer::DrawDottedCornerSlow and DottedCornerFinder
    + nsCSSBorderRenderer::DrawDashedCornerSlow and DashedCornerFinder

  * Made dotted/dashed border side symmetric, by changing the spacing
    + nsCSSBorderRenderer::CalculateDashedOptions from
      nsCSSBorderRenderer::DrawDashedSide
      and nsCSSBorderRenderer::DrawDottedSide

  * Draw dotted side with separated circles, instead of dashed with ROUND
    + nsCSSBorderRenderer::DrawDottedSide

  * Made interaction between dotted side and other side better
    + SIDE_CLIP_RECTANGLE_CORNER and SIDE_CLIP_RECTANGLE_NO_CORNER in
      nsCSSBorderRenderer::GetSideClipSubPath
    + nsCSSBorderRenderer::IsCornerMergeable for dotted sides
    + start/end position calculation (nsCSSBorderRenderer::GetStraightBorderPoint)
      in nsCSSBorderRenderer::DrawDashedSide and nsCSSBorderRenderer::DrawDottedSide

  * Changed DASH_LENGTH from 3 to 2
    rendering result seems to be much better with 2

  * Added several utility functions for Corner/Side
    + GetHorizontalSide/GetVerticalSide
    + GetCWSide/GetCCWSide
    + GetCWCorner/GetCCWCorner


here's try run result
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=3a20c3e08cf1
the failure in 461512-1.html seems to be almost same reason as dotted cannot be tested, so I think that part should also be commented out.


then, I have some questions:
  1. What would be the best way to test border-radius rendering?

  2. What kind of performance test should I do?
     is there any upper bound for time taken by each redraw (especially with mobile device)?
Comment 99 Tooru Fujisawa [:arai] 2015-08-13 14:25:58 PDT
Created attachment 8647736 [details]
result of WIP 7 patch
Comment 100 Tooru Fujisawa [:arai] 2015-08-13 14:27:27 PDT
Created attachment 8647737 [details]
DottedCornerFinder's behavior
Comment 101 Tooru Fujisawa [:arai] 2015-08-13 14:27:57 PDT
Created attachment 8647738 [details]
DashedCornerFinder's behavior
Comment 102 Tooru Fujisawa [:arai] 2015-08-13 14:28:31 PDT
Created attachment 8647739 [details]
corner interaction between dotted and other side
Comment 103 Robert O'Callahan (:roc) (email my personal email if necessary) 2015-08-19 21:40:04 PDT
This looks really amazingly impressive, and I'm excited about getting it landed! But it's going to be hard to review.

The diagrams should be checked in alongside the code, and you should reference them in comments.

(In reply to Tooru Fujisawa [:arai] from comment #98)
> then, I have some questions:
>   1. What would be the best way to test border-radius rendering?

That's a very good question!

One way to test this would be to write a mochitest that draws borders into a canvas --- e.g. by creating an SVG image containing a foreignobject containing some HTML+CSS to render:
http://robert.ocallahan.org/2011/11/drawing-dom-content-to-canvas.html
Then, for each rendered border, call getImageData and probe specific pixels to make sure they have the expected value. E.g. test that points definitely inside a dot or dash are drawn, and test that points definitely outside a dot or dash are not drawn.

>   2. What kind of performance test should I do?
>      is there any upper bound for time taken by each redraw (especially with
> mobile device)?

It's very important that common cases of simple borders be fast. Maybe nothing you're changing here actually affects this.

It's also very important that repeated drawing of the same border be fast. So you might want to look at the parts that are expensive and think about adding some caching. It would be extra-good if the caching is designed so that when corners are symmetrical (which they usually are) we can calculate and cache values for the top-left corner and use the cached values for the other corners.
Comment 104 Jeff Muizelaar [:jrmuizel] 2015-08-20 07:58:12 PDT
(In reply to Tooru Fujisawa [:arai] from comment #99)
> Created attachment 8647736 [details]
> result of WIP 7 patch

Impressive work. I've only looked briefly at this bug and your patch so might be completely misunderstanding things but I noticed that you're doing a lot of the analytical work on bezier curves. Is this easier than working with the ellipses directly?
Comment 105 Tooru Fujisawa [:arai] 2015-08-20 17:10:08 PDT
Thank you roc and jrmuizel :)

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #103)
> This looks really amazingly impressive, and I'm excited about getting it
> landed! But it's going to be hard to review.
> 
> The diagrams should be checked in alongside the code, and you should
> reference them in comments.
> 
> (In reply to Tooru Fujisawa [:arai] from comment #98)
> > then, I have some questions:
> >   1. What would be the best way to test border-radius rendering?
> 
> That's a very good question!
> 
> One way to test this would be to write a mochitest that draws borders into a
> canvas --- e.g. by creating an SVG image containing a foreignobject
> containing some HTML+CSS to render:
> http://robert.ocallahan.org/2011/11/drawing-dom-content-to-canvas.html
> Then, for each rendered border, call getImageData and probe specific pixels
> to make sure they have the expected value. E.g. test that points definitely
> inside a dot or dash are drawn, and test that points definitely outside a
> dot or dash are not drawn.

Thanks, now I understand the meaning of the mask images in bug 19963.  So I can mask the rendered image and check the correctness.  I'll try it :)

> >   2. What kind of performance test should I do?
> >      is there any upper bound for time taken by each redraw (especially with
> > mobile device)?
> 
> It's very important that common cases of simple borders be fast. Maybe
> nothing you're changing here actually affects this.
> 
> It's also very important that repeated drawing of the same border be fast.
> So you might want to look at the parts that are expensive and think about
> adding some caching. It would be extra-good if the caching is designed so
> that when corners are symmetrical (which they usually are) we can calculate
> and cache values for the top-left corner and use the cached values for the
> other corners.

I'll prepare cache for overlap and dash length calculation in finder classes.  Those parts take so much time in the calculation, and it should be same for symmetrical borders.


(In reply to Jeff Muizelaar [:jrmuizel] from comment #104)
> (In reply to Tooru Fujisawa [:arai] from comment #99)
> > Created attachment 8647736 [details]
> > result of WIP 7 patch
> 
> Impressive work. I've only looked briefly at this bug and your patch so
> might be completely misunderstanding things but I noticed that you're doing
> a lot of the analytical work on bezier curves. Is this easier than working
> with the ellipses directly?

There are some reasons why I use bezier curve in many places:

1. In most case, I need to handle the curve as parametric curve like x = Fx(t) and y = Fy(t), and in that case I think bezier curve is less-expensive than elliptic arc.  iiuc, elliptic arc needs sin/cos or sqrt or something like that, doesn't it?

2. in dashed border, finally we need to fill each segment with the region consisted with bezier curves and straight lines (am I correct?), so we need control points for them, and in that case calculating everything with bezier curve will be straightforward.

3. I cannot find a good estimation for elliptic arc's length that is better than bezier curve's length for given segment.

4. Just because I'm not familiar with elliptic arc.  so, if you know better solution with elliptic arc, let me know that ;)

I use elliptic arc when it seems to be simpler, like when calculating whole arc's length (GetQuarterEllipticArcLength), solving equation with x and y (CalculateDistanceToEllipticArc), etc.
Comment 106 Tooru Fujisawa [:arai] 2015-08-30 18:25:15 PDT
Created attachment 8654682 [details] [diff] [review]
(WIP 8) Support dotted/dashed border-radiused corners.

Changes:
  * Added several figures as comments

  * Added cache for best overlap and dash length for given corner parameters
    Now it takes up to 1 or 2 seconds for each redraw on Flame

  * Added reftests
    It renders dashed/dotted borders, overlays two masks:
    not-filled region with filled-color, and not-unfilled region with unfilled-color,
    and compares them with full-filled/fill-unfilled boxes

  * Always calculate best dashed length for dashed side (even with width <= 2.0)

  * Separated into several files

  * Changed DASH_LENGTH back to 3 (looks like [1,3] range is good enough)

  * Use arc length instead of direct distance in dashed corner calculation

  * If dot may overflow from outer rect, don't draw it (will post in next comment)

  * If corner between dotted sides is merged into one dot and sides have same color,
    draw the dot with single path instead of 2 half circles

  * Applied mozilla coding style for newly added code

  * Fixed dash offset in simple case for dotted border in nsCSSBorderRenderer::DrawBorders
    there strokeOptions.mDashOffset is set to 0.5f but it should be
    0.5f * mBorderWidths[0], former results in 3 dots filled 1 dot gap in 2px dotted border because of AntialiasMode::NONE

  * Removed unused method declaration SetupStrokeStyle

  * Added some missing #include and using namespace.

Questions:
  1. Where should I store cache data?
     Currently cache is allocates as a global variable, but I think that data should be associated with document.

  2. What would be the best data structure to store cache data?
     Is there any good data structure or existing template that provides O(log(n)) search and I can remove old data?
     In BorderCache.h, I use 2 data structures, a ring map and hash map (using either one depending on #ifdef),
     former is O(n) search but it removes oldest data when adding new one,
     latter one cannot remove oldest data.
Comment 107 Tooru Fujisawa [:arai] 2015-08-30 18:26:03 PDT
Created attachment 8654683 [details]
static test for various width
Comment 108 Tooru Fujisawa [:arai] 2015-08-30 18:26:34 PDT
Created attachment 8654684 [details]
result of WIP 8 patch
Comment 109 Tooru Fujisawa [:arai] 2015-08-30 18:27:39 PDT
Created attachment 8654685 [details]
Another design issue in dotted side with too-short length

One more question, what would be the expected rendering in attached image?
If width or height is smaller than border-width, there's no space to draw a circle with border-width/2 radius in that side.
In WIP 8 patch, I just skip rendering that circle, but it results in empty border.
Comment 110 Tooru Fujisawa [:arai] 2015-08-30 18:28:12 PDT
Created attachment 8654686 [details]
Rendering issue with border-radius extends into the opposite side

When the border-radius curve extends into the opposite side, the side and corner overlaps and the side overflows from the curve's revion.
  https://drafts.csswg.org/css-backgrounds-3/#corner-shaping
I'd like to leave them to another bug since it happens also with solid border.  We'll need to handle that case specially.
Comment 111 Leon Sorokin 2015-08-31 08:19:49 PDT
looks like the spacing uniformity in the simple dotted case has regressed since WIP7

https://i.imgur.com/i2KCztK.png
Comment 112 Tooru Fujisawa [:arai] 2015-09-01 02:19:55 PDT
(In reply to Leon Sorokin from comment #111)
> looks like the spacing uniformity in the simple dotted case has regressed
> since WIP7
> 
> https://i.imgur.com/i2KCztK.png

Thank you for pointing that out :)
Yeah, in that one, previous rendering should be better, I'll check them.

however, I don't do anything for "uniformity", each side and corner is rendered separately and spacing is also calculated separately, to place dots at the end of each side, and make corner starts and ends with dots (otherwise corner rendering becomes more difficult), so spacing uniformity is not guaranteed now. (of course, I'll try choosing better spacing for given length)
Comment 113 Tooru Fujisawa [:arai] 2015-09-01 03:13:49 PDT
Created attachment 8655353 [details] [diff] [review]
(WIP 9) Support dotted/dashed border-radiused corners.

oops, that was just a typo :P

> static bool
> IsSingleCurve(Float aMinR, Float aMaxR,
>               Float aMinBorderRadius, Float aMaxBorderRadius)
> {
>   return aMinR > 0.0f &&
>          aMinBorderRadius > aMaxR * 4.0f &&
>-         aMinBorderRadius / aMaxBorderRadius < 0.5f;
>+         aMinBorderRadius / aMaxBorderRadius > 0.5f;
> }

no other changes from WIP 8 in Comment #106.

re-posting questions just in case:
  1. Where should I store cache data?
     Currently cache is allocates as a global variable, but I think that data should be associated with document.

  2. What would be the best data structure to store cache data?
     Is there any good data structure or existing template that provides O(log(n)) search and I can remove old data?
     In BorderCache.h, I use 2 data structures, a ring map and hash map (using either one depending on #ifdef),
     former is O(n) search but it removes oldest data when adding new one,
     latter one cannot remove oldest data.
Comment 114 Tooru Fujisawa [:arai] 2015-09-01 03:14:28 PDT
Created attachment 8655355 [details]
result of WIP 9 patch
Comment 115 Robert O'Callahan (:roc) (email my personal email if necessary) 2015-09-08 22:37:01 PDT
Comment on attachment 8655353 [details] [diff] [review]
(WIP 9) Support dotted/dashed border-radiused corners.

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

::: layout/base/BezierUtils.h
@@ +48,5 @@
> +mozilla::gfx::Float CalculateDistanceToEllipticArc(const mozilla::gfx::Point& P,
> +                                                   const mozilla::gfx::Point& normal,
> +                                                   const mozilla::gfx::Point& origin,
> +                                                   mozilla::gfx::Float width,
> +                                                   mozilla::gfx::Float height);

Add comments explaining all these functions, especially where you have "2", "A" and "B" versions.

::: layout/base/BorderCache.h
@@ +13,5 @@
> +
> +namespace mozilla {
> +// Cache for best overlap and best dashLength.
> +
> +struct fourFloats

FourFloats

@@ +96,5 @@
> +};
> +
> +#else
> +
> +class fourFloatsHashKey : public PLDHashEntryHdr

FourFloatsHashKey
Comment 116 Robert O'Callahan (:roc) (email my personal email if necessary) 2015-09-08 22:40:29 PDT
(In reply to Tooru Fujisawa [:arai] from comment #113)
> re-posting questions just in case:
>   1. Where should I store cache data?
>      Currently cache is allocates as a global variable, but I think that
> data should be associated with document.

I think it should be global. It's common to have multiple documents with the same styles styles --- multiple pages open from the same site.

>   2. What would be the best data structure to store cache data?
>      Is there any good data structure or existing template that provides
> O(log(n)) search and I can remove old data?
>      In BorderCache.h, I use 2 data structures, a ring map and hash map
> (using either one depending on #ifdef),
>      former is O(n) search but it removes oldest data when adding new one,
>      latter one cannot remove oldest data.

A simple approach that should be OK is to use a hashmap and clear the entire cache when it gets full.
Comment 117 Tooru Fujisawa [:arai] 2015-09-10 08:47:39 PDT
Created attachment 8659312 [details] [diff] [review]
Part 0: Add missing includes and namespaces.

Thank you for your feedback :)
Now I split patches into 6 parts, for each change.

First, I bumped into some errors when adding files to UNIFIED_SOURCES, and there are some more errors while testing non-unified build in layout/base/.  So, as Part 0, I'd fix them.

[layout/base/AccessibleCaretEventHub.cpp]
> +#include "nsCanvasFrame.h"
https://dxr.mozilla.org/mozilla-central/rev/dd2a1d737a64d9a3f23714ec5cc623ec8933b51f/layout/base/AccessibleCaretEventHub.cpp#391
> !aPresShell->GetCanvasFrame()->GetCustomContentContainer()) {
nsPresShell::GetCanvasFrame returns nsCanvasFrame*.

[layout/base/AccessibleCaretManager.cpp]
> +#include "mozilla/Preferences.h"
https://dxr.mozilla.org/mozilla-central/rev/dd2a1d737a64d9a3f23714ec5cc623ec8933b51f/layout/base/AccessibleCaretManager.cpp#863
>    Preferences::AddUintVarCache(&caretTimeoutMs,
Preferences is used here.

> +#include "nsContainerFrame.h"
https://dxr.mozilla.org/mozilla-central/rev/dd2a1d737a64d9a3f23714ec5cc623ec8933b51f/layout/base/AccessibleCaretManager.cpp#536
> focusableFrame = focusableFrame->GetParent();
nsIFrame::GetParent returns nsContainerFrame.

[layout/base/ActiveLayerTracker.cpp]
> +using namespace dom;
https://dxr.mozilla.org/mozilla-central/rev/dd2a1d737a64d9a3f23714ec5cc623ec8933b51f/layout/base/ActiveLayerTracker.cpp?offset=0#381
> KeyframeEffectReadOnly* effect =
mozilla::dom:: is omitted.

[layout/base/MobileViewportManager.cpp]
> +#include "gfxPrefs.h"
https://dxr.mozilla.org/mozilla-central/rev/dd2a1d737a64d9a3f23714ec5cc623ec8933b51f/layout/base/MobileViewportManager.cpp?offset=100#281
>  if (gfxPrefs::APZAllowZooming()) {
gfxPrefs is used here.

> +#include "nsIDOMEvent.h"
https://dxr.mozilla.org/mozilla-central/rev/dd2a1d737a64d9a3f23714ec5cc623ec8933b51f/layout/base/MobileViewportManager.cpp#88
>  event->GetType(type);
nsIDOMEvent is used here.

> +#include "nsIFrame.h"
https://dxr.mozilla.org/mozilla-central/rev/dd2a1d737a64d9a3f23714ec5cc623ec8933b51f/layout/base/MobileViewportManager.cpp?offset=0#213
>   if (!nsLayoutUtils::GetDisplayPort(root->GetContent(), nullptr)) {
root is nsIFrame*.

> +#include "nsIScrollableFrame.h"
https://dxr.mozilla.org/mozilla-central/rev/dd2a1d737a64d9a3f23714ec5cc623ec8933b51f/layout/base/MobileViewportManager.cpp?offset=200#217
>    nsIScrollableFrame* scrollable = do_QueryFrame(root);
'do_QueryFrame::operator nsIScrollableFrame *<nsIScrollableFrame>' is requested here.

> +#include "nsLayoutUtils.h"
https://dxr.mozilla.org/mozilla-central/rev/dd2a1d737a64d9a3f23714ec5cc623ec8933b51f/layout/base/MobileViewportManager.cpp#123
>  LayoutDeviceToLayerScale res(nsLayoutUtils::GetResolution(mPresShell));
nsLayoutUtils is used here.

> +#include "nsPresContext.h"
https://dxr.mozilla.org/mozilla-central/rev/dd2a1d737a64d9a3f23714ec5cc623ec8933b51f/layout/base/MobileViewportManager.cpp#121
>  CSSToLayoutDeviceScale cssToDev((float)nsPresContext::AppUnitsPerCSSPixel()
nsPresContext is used here.

> +#include "UnitTransforms.h"
https://dxr.mozilla.org/mozilla-central/rev/dd2a1d737a64d9a3f23714ec5cc623ec8933b51f/layout/base/MobileViewportManager.cpp#138
>    CSSToParentLayerScale zoom = ViewTargetAs<ParentLayerPixel>(defaultZoom,
ViewTargetAs is used here.

[layout/base/RestyleManager.cpp]
> +using namespace dom;
https://dxr.mozilla.org/mozilla-central/rev/dd2a1d737a64d9a3f23714ec5cc623ec8933b51f/layout/base/RestyleManager.cpp#2956
>  FlattenedChildIterator it(aElement);
mozilla::dom:: is omitted.

[layout/base/TouchCaret.cpp]
> +#include "nsDocShell.h"
>  docShell->AddWeakScrollObserver(this);
nsDocShell is used here.

[layout/base/TouchCaret.h]
> +class nsDocShell;
https://dxr.mozilla.org/mozilla-central/rev/dd2a1d737a64d9a3f23714ec5cc623ec8933b51f/layout/base/TouchCaret.h#294
>  WeakPtr<nsDocShell> mDocShell;
nsDocShell is used here.

[layout/base/TouchManager.cpp]
> +#include "mozilla/TouchEvents.h"
https://dxr.mozilla.org/mozilla-central/rev/dd2a1d737a64d9a3f23714ec5cc623ec8933b51f/layout/base/TouchManager.cpp#59
>            WidgetTouchEvent event(true, NS_TOUCH_END, widget);
WidgetTouchEvent is used here.

> +#include "nsIFrame.h"
> +#include "nsView.h"
https://dxr.mozilla.org/mozilla-central/rev/dd2a1d737a64d9a3f23714ec5cc623ec8933b51f/layout/base/TouchManager.cpp#57
>          nsCOMPtr<nsIWidget> widget = frame->GetView()->GetNearestWidget(&pt);
frame is nsIFrame*, and GetView() returns nsView*.

> +using namespace mozilla;
https://dxr.mozilla.org/mozilla-central/rev/dd2a1d737a64d9a3f23714ec5cc623ec8933b51f/layout/base/TouchManager.cpp#12
> nsRefPtrHashtable<nsUint32HashKey, dom::Touch>* TouchManager::gCaptureTouchList;
mozilla:: is omitted.

> +using namespace mozilla::dom;
https://dxr.mozilla.org/mozilla-central/rev/dd2a1d737a64d9a3f23714ec5cc623ec8933b51f/layout/base/TouchManager.cpp#214
>       nsCOMPtr<EventTarget> targetPtr = oldTouch->mTarget;
mozilla::dom:: is omitted.

[layout/base/TouchManager.h]
> +#include "mozilla/BasicEvents.h"
https://dxr.mozilla.org/mozilla-central/rev/dd2a1d737a64d9a3f23714ec5cc623ec8933b51f/layout/base/TouchManager.h#27
>  bool PreHandleEvent(mozilla::WidgetEvent* aEvent,
WidgetEvent is used here

> +#include "mozilla/dom/Touch.h"
> +#include "nsRefPtrHashtable.h"
https://dxr.mozilla.org/mozilla-central/rev/dd2a1d737a64d9a3f23714ec5cc623ec8933b51f/layout/base/TouchManager.h#34
>  static nsRefPtrHashtable<nsUint32HashKey, mozilla::dom::Touch>* gCaptureTouchList;
nsRefPtrHashtable and mozilla::dom::Touch are used here

[layout/base/ZoomConstraintsClient.cpp]
> +#include "gfxPrefs.h"
https://dxr.mozilla.org/mozilla-central/rev/dd2a1d737a64d9a3f23714ec5cc623ec8933b51f/layout/base/ZoomConstraintsClient.cpp?offset=0#166
>  constraints.mAllowZoom = aViewportInfo.IsZoomAllowed() && gfxPrefs::APZAllowZooming();
gfxPrefs is used here

> +#include "mozilla/Preferences.h"
https://dxr.mozilla.org/mozilla-central/rev/dd2a1d737a64d9a3f23714ec5cc623ec8933b51f/layout/base/ZoomConstraintsClient.cpp#80
>  Preferences::RemoveObserver(this, "browser.ui.zoom.force-user-scalable");
Preferences is used here.

[layout/base/nsLayoutDebugger.cpp]
> +#include "nsAttrValue.h"
https://dxr.mozilla.org/mozilla-central/rev/dd2a1d737a64d9a3f23714ec5cc623ec8933b51f/layout/base/nsLayoutDebugger.cpp#158
>      content->GetClasses()->ToString(tmp);
GetClasses() returns nsAttrValue*.

[layout/base/nsLayoutUtils.cpp]
> +#include "nsTextFragment.h"
https://dxr.mozilla.org/mozilla-central/rev/dd2a1d737a64d9a3f23714ec5cc623ec8933b51f/layout/base/nsLayoutUtils.cpp#8640
>      GetText()->AppendTo(aResult, offset, length);
GetText() returns nsTextFragment*

[layout/base/nsPresShell.cpp]
> +#include "gfxPrefs.h"
https://dxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp#986
>    if (gfxPrefs::MetaViewportEnabled() || gfxPrefs::APZAllowZooming()) {
gfxPrefs is used here

[layout/generic/nsCanvasFrame.h]
> +#include "mozilla/dom/Element.h"
https://dxr.mozilla.org/mozilla-central/rev/dd2a1d737a64d9a3f23714ec5cc623ec8933b51f/layout/generic/nsCanvasFrame.h#34
>  explicit nsCanvasFrame(nsStyleContext* aContext)
nsCOMPtr<mozilla::dom::Element>::assert_validity is requested in nsCanvasFrame ctor.

[layout/style/nsStyleStruct.h]
> +class nsStyleContext;
https://dxr.mozilla.org/mozilla-central/rev/dd2a1d737a64d9a3f23714ec5cc623ec8933b51f/layout/style/nsStyleStruct.h#1369
>                              nsStyleContext* aContext) const;
nsStyleContext needs declaration.
Comment 118 Tooru Fujisawa [:arai] 2015-09-10 08:52:05 PDT
Created attachment 8659314 [details] [diff] [review]
Part 1: Fix spacing of simple 2px dotted border without radius.

Part 1 fixes the rendering result of 2px dotted border with no radius.  Because of AntialiasMode::NONE is used and mDashOffset is 0.5, there 3px filled and 1px spacing, on OSX.  mDashOffset should be relative to border width.

I'll post different in next comment.

I don't add test for this because the rendering result still depends on OS, especially the at the corner.
Comment 119 Tooru Fujisawa [:arai] 2015-09-10 08:53:27 PDT
Created attachment 8659316 [details]
Rendering result of 2px dotted border

Part 1 fixes attaches case.
Comment 120 Tooru Fujisawa [:arai] 2015-09-10 08:54:19 PDT
Created attachment 8659317 [details] [diff] [review]
Part 2: Split constants for border rendering to BorderConsts.h.

Part 2 just makes BorderConsts.h, which has some definition used by several files (also files added by Part 4)
Comment 121 Tooru Fujisawa [:arai] 2015-09-10 09:05:56 PDT
Created attachment 8659323 [details] [diff] [review]
Part 3: Improve spacing and corner interaction of dashed/dotted border.

Part 3 applies following changes:

* Make spacing of dashed/dotted border variable, to make it symmetric

  * Disable or add fuzzy option to test which is affected by anti-alias caused by spacing change

* Draw dots in dotted border with width>2px separately, to make it perfect circle

* Make corner interaction between dotted sides, or dotted and other sides.

  * at corner between dotted sides with same width, dots are merged into single dot
  * at corner between dotted sides with different width, wider side draws the dot at corner
  * at corner between dotted side and other side, other side always draws the corner
Comment 122 Tooru Fujisawa [:arai] 2015-09-10 09:07:03 PDT
Created attachment 8659324 [details] [diff] [review]
Part 4: Support dotted/dashed border-radiused corners.

Part 4 adds support for border-radius with dotted/dashed.

Now cache is implemented by hashmap on global, entries are cleared when 257-th cache is stored.  Static test doesn't hit this, only 120 to 130 cache entries.  I might have to investigate how much caches we need for real websites (do you know where it's used in practice?).

Then, in WIP 9 patch, the cache didn't work correctly (the performance improvement comes from other changes, like parameter tweak), now it's fixed and the static test is shown almost smoothly on Flame, it takes 1 or 2 seconds on some heavy style, but I don't need to wait for rendering for other cases, while scrolling.
Comment 123 Tooru Fujisawa [:arai] 2015-09-10 09:08:03 PDT
Created attachment 8659325 [details] [diff] [review]
Part 5: Add testcases for dashed/dotted borders.

Part 5 adds reftests for Part 3 and Part 4.

Almost green on try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6364a3081956
Comment 124 Tooru Fujisawa [:arai] 2015-09-10 09:08:50 PDT
Created attachment 8659326 [details]
Rendering result of static test
Comment 125 Jeff Muizelaar [:jrmuizel] 2015-09-14 13:10:08 PDT
A couple of us in the Toronto office are going to try to take a look reviewing this during our lunch break.
Comment 126 Jeff Muizelaar [:jrmuizel] 2015-09-16 09:58:44 PDT
Comment on attachment 8659323 [details] [diff] [review]
Part 3: Improve spacing and corner interaction of dashed/dotted border.

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

::: layout/base/nsCSSRenderingBorders.cpp
@@ +666,5 @@
>  
> +Point
> +nsCSSBorderRenderer::GetStraightBorderPoint(mozilla::css::Side aSide,
> +                                            mozilla::css::Corner aCorner,
> +                                            bool* aIsUnfilled)

It would be nice to have a better description of what this method does and why you want to do it.

@@ +764,5 @@
> +        //   |#########|
> +        //   | ####### |
> +        //   |  #####  |
> +        //   |         |
> +        Float minimum = borderWidth * /*(1.0f + sqrt(2.0f)) / 2.0f*/ 1.5f ;

Why are you approximating this as 1.5?
Comment 127 Jeff Muizelaar [:jrmuizel] 2015-09-16 10:21:33 PDT
Also, if you can add a paragraph of text describing the overall approach you're taking in Part 3 and Part 4 that would be helpful in reviewing.
Comment 128 Tooru Fujisawa [:arai] 2015-09-16 10:57:16 PDT
Created attachment 8661946 [details]
Two dots at the corner with small radius and same border-width.

Thank you for reviewing :D

(In reply to Jeff Muizelaar [:jrmuizel] from comment #126)
> ::: layout/base/nsCSSRenderingBorders.cpp
> @@ +666,5 @@
> >  
> > +Point
> > +nsCSSBorderRenderer::GetStraightBorderPoint(mozilla::css::Side aSide,
> > +                                            mozilla::css::Corner aCorner,
> > +                                            bool* aIsUnfilled)
> 
> It would be nice to have a better description of what this method does and
> why you want to do it.
(In reply to Jeff Muizelaar [:jrmuizel] from comment #127)
> Also, if you can add a paragraph of text describing the overall approach
> you're taking in Part 3 and Part 4 that would be helpful in reviewing.

Sure, I'll add those comments, will post updated patches by weekend (hopefully tomorrow).

> @@ +764,5 @@
> > +        //   |#########|
> > +        //   | ####### |
> > +        //   |  #####  |
> > +        //   |         |
> > +        Float minimum = borderWidth * /*(1.0f + sqrt(2.0f)) / 2.0f*/ 1.5f ;
> 
> Why are you approximating this as 1.5?

Sorry, I forgot to remove old code and update comment.  1.5 is better than (1+sqrt(2))/2.
(1+sqrt(2))/2 is the value that two dots contact.  But we should make some gap between them even if radius is small (but larger than border-width/2).
(see attached image)
There will be better value than 1.5, but I don't have any idea now.
Comment 129 Jeff Muizelaar [:jrmuizel] 2015-09-16 11:53:57 PDT
(In reply to Tooru Fujisawa [:arai] from comment #128)
> 
> Sorry, I forgot to remove old code and update comment.  1.5 is better than
> (1+sqrt(2))/2.
> (1+sqrt(2))/2 is the value that two dots contact.  But we should make some
> gap between them even if radius is small (but larger than border-width/2).
> (see attached image)
> There will be better value than 1.5, but I don't have any idea now.

It's worth adding a comment about this.
Comment 130 Tooru Fujisawa [:arai] 2015-09-17 03:00:26 PDT
Created attachment 8662302 [details] [diff] [review]
Part 3: Improve spacing and corner interaction of dashed/dotted border.

Added comments to GetStraightBorderPoint and DrawDashedOrDottedSide in Part 3, and DrawDashedOrDottedCorner in Part 4.  Tell me if those comments doesn't make sense, or if some required parts are not described.  Thanks!

Also, fixed the minimum length to draw dash in corner/side in Part 3, there, minimum dash length was lower than 1 (that is too short dashed segment) in previous patch, but dash length should be in [1, 3] range.  This affects to some testcase, so updated them in Part 5.
Comment 131 Tooru Fujisawa [:arai] 2015-09-17 03:00:53 PDT
Created attachment 8662303 [details] [diff] [review]
Part 4: Support dotted/dashed border-radiused corners.
Comment 132 Tooru Fujisawa [:arai] 2015-09-17 03:01:17 PDT
Created attachment 8662304 [details] [diff] [review]
Part 5: Add testcases for dashed/dotted borders.
Comment 133 Tooru Fujisawa [:arai] 2015-09-17 03:01:45 PDT
Created attachment 8662305 [details]
Rendering result of static test
Comment 134 Jeff Muizelaar [:jrmuizel] 2015-09-22 09:57:48 PDT
Tooru, do you think you could get some timings of the border drawing call before and after this code? That will give us a feeling for whether we need to worry about the performance of the new implementation.
Comment 135 Tooru Fujisawa [:arai] 2015-09-22 13:02:40 PDT
Created attachment 8664431 [details]
Time taken by nsCSSBorderRenderer::DrawBorders

Here's the result of performance comparison :)
(I'll post screenshot of patched rendering in next comment)

Let me know if more cases or detailed data is needed.
Comment 136 Tooru Fujisawa [:arai] 2015-09-22 13:03:37 PDT
Created attachment 8664434 [details]
Rendering result of "Time taken by nsCSSBorderRenderer::DrawBorders" file
Comment 137 Jeff Muizelaar [:jrmuizel] 2015-09-22 13:12:23 PDT
(In reply to Tooru Fujisawa [:arai] from comment #136)
> Created attachment 8664434 [details]
> Rendering result of "Time taken by nsCSSBorderRenderer::DrawBorders" file

That's great. If it's easy to do, it would be nice to separate out the Moz2D/drawing portion of the time from the nsCSSBorderRenderer::DrawBorders calculations.
Comment 138 Tooru Fujisawa [:arai] 2015-09-22 14:24:55 PDT
Created attachment 8664479 [details]
Time taken by nsCSSBorderRenderer::DrawBorders

Updated.
in P1 and P2, 1st number (red) is the time taken by calculation, 2nd number (blue) is the time taken by drawing.
Comment 139 Tooru Fujisawa [:arai] 2015-09-22 14:25:41 PDT
Created attachment 8664480 [details]
Rendering result of "Time taken by nsCSSBorderRenderer::DrawBorders" file
Comment 140 Jeff Muizelaar [:jrmuizel] 2015-09-22 14:35:19 PDT
(In reply to Tooru Fujisawa [:arai] from comment #139)
> Created attachment 8664480 [details]
> Rendering result of "Time taken by nsCSSBorderRenderer::DrawBorders" file

What OS was the timing done on?
Comment 141 Tooru Fujisawa [:arai] 2015-09-22 14:40:27 PDT
oh, sorry I forgot to note the basic information.
I tested on OSX 10.10.5, with 64bit build nightly, on iMac Late 2013 (2.9 GHz Intel Core i5, 16 GB 1600 MHz DDR3, NVIDIA GeForce GT 750M 1024 MB)
Comment 142 Jeff Muizelaar [:jrmuizel] 2015-10-13 14:46:06 PDT
Sorry for the delay, I'm going to try and finish up the review (at least up to part 3) this week. Is it possible to get timing information for just the first part (Up to part 3) of the patch?
Comment 143 Tooru Fujisawa [:arai] 2015-10-13 21:35:14 PDT
Created attachment 8673463 [details]
Time taken by nsCSSBorderRenderer::DrawBorders (with Part 1-3)

Thanks!
Here's result with Part 1-3
Comment 144 Tooru Fujisawa [:arai] 2015-10-13 22:10:45 PDT
Created attachment 8673472 [details]
Rendering result of "Time taken by nsCSSBorderRenderer::DrawBorders (with Part 1-3)" file with Part 1-3

Here's the rendering result with part 1-3 (corners are still rendered with solid stroke, even if they're merged)
Comment 145 Tooru Fujisawa [:arai] 2015-11-07 04:12:21 PST
Thank you for reviewing :)
Is there anything I can do here for now?  (looks like same patch is still applicable and no rebase is needed)
Comment 146 arni2033 2015-11-07 06:45:22 PST
[~OOT] Could somebody please install the patch and tell if this testcase with "border-style: dashed solid;" - lags not more than with "border-style: solid;". This is what I'd expect from the patch.
> the testcase:   https://bug780366.bmoattachments.org/attachment.cgi?id=8684574
Just compare the testcase with checkbox "border-style" enabled/disabled (you can also try animation)
Comment 147 Tooru Fujisawa [:arai] 2015-11-07 12:34:53 PST
To be clear, this patch does not handle the bug 780366's case, so the rendering result is still wrong and the performance of it won't be meaningful value for the case (I think we need totally different approach for the case, than current one).  Attachment 8654686 [details] in comment #110 is the result for almost same situation.

> lags not more than with "border-style: solid;".
Here's results:
  without animation (10 samples for each)
    "solid" border:        288 [us]
    "dashed solid" border: 641 [us]
  with animation (200 samples for each)
    "solid" border:        307 [us] (min:273, max:364)
    "dashed solid" border: 729 [us] (min:688, max:964)

Those should be in acceptable range.
Comment 148 Tooru Fujisawa [:arai] 2015-11-07 12:37:05 PST
Created attachment 8684600 [details] [diff] [review]
Part 0-followup: Add missing includes and namespaces.

Just noticed that there some more missing #include's added from last time.
No other changes in other parts.
Comment 149 Julien Wajsberg [:julienw] 2015-11-28 06:37:18 PST
*** Bug 1225837 has been marked as a duplicate of this bug. ***
Comment 150 Jeff Muizelaar [:jrmuizel] 2015-12-26 15:09:52 PST
Comment on attachment 8662303 [details] [diff] [review]
Part 4: Support dotted/dashed border-radiused corners.

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

I haven't looked at everything yet, but here are some preliminary comments.

It might be worth packaging up Point[4] into a Curve or Bezier type. This will let you assign them instead of having to memcpy.

::: layout/base/BezierUtils.cpp
@@ +260,5 @@
> +Float
> +GetQuarterEllipticArcLength(Float a, Float b)
> +{
> +  Float A = a + b, S = a - b;
> +  Float A2 = pow(A, 2.0f), S2 = pow(S, 2.0f);

It's more performant to write these as:
A2 = A*A;
S2 = S*S;

@@ +262,5 @@
> +{
> +  Float A = a + b, S = a - b;
> +  Float A2 = pow(A, 2.0f), S2 = pow(S, 2.0f);
> +
> +  return M_PI * A * (3.00f * S2 / (A2 * (sqrt(-3.0f * S2 / A2 + 4.0f) + 10.0f)) + 1.0f) / 4.0f;

Where does this formula come from?

::: layout/base/BezierUtils.h
@@ +1,4 @@
> +/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

It would be nice if BezierUtils.cpp and BezierUtils.h could go in gfx/2d as that's a more likely place for people to look for this kind of thing.

@@ +6,5 @@
> +#ifndef mozilla_BezierUtils_h_
> +#define mozilla_BezierUtils_h_
> +
> +#include "mozilla/gfx/2D.h"
> +#include "gfxRect.h"

Is this include needed?

::: layout/base/DashedCornerFinder.cpp
@@ +128,5 @@
> +
> +DashedCornerFinder::Result
> +DashedCornerFinder::Next(void)
> +{
> +  Float lastTo, lastTi, to, ti;

I think calling these lastOuterT and outerT would be clearer than abbreviating down to a single letter.

@@ +197,5 @@
> +  Point innerSectionPoints[4];
> +  SplitBezierB(tmp, mOuterPoints, lastTo);
> +  SplitBezierA(outerSectionPoints, tmp, ConvertRange(to, lastTo));
> +  SplitBezierB(tmp, mInnerPoints, lastTi);
> +  SplitBezierA(innerSectionPoints, tmp, ConvertRange(to, lastTo));

Should this be?
SplitBezierA(innerSectionPoints, tmp, ConvertRange(ti, lastTi)); instead of To?

Would it be better to have a SplitBezier function that takes a t1 and t2?

::: layout/base/DashedCornerFinder.h
@@ +175,5 @@
> +  Point mLastPi;
> +  Float mLastTo;
> +  Float mLastTi;
> +
> +  // Length for each segment, radio of the border width at that point.

Is 'radio' a typo?
Comment 151 Tooru Fujisawa [:arai] 2015-12-26 20:11:21 PST
Created attachment 8702087 [details] [diff] [review]
Part 4: Support dotted/dashed border-radiused corners.

Thank you for reviewing! :D

(In reply to Jeff Muizelaar [:jrmuizel] from comment #150)
> Comment on attachment 8662303 [details] [diff] [review]
> Part 4: Support dotted/dashed border-radiused corners.
> 
> Review of attachment 8662303 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I haven't looked at everything yet, but here are some preliminary comments.
> 
> It might be worth packaging up Point[4] into a Curve or Bezier type. This
> will let you assign them instead of having to memcpy.

Okay, added Bezier type, that has 'Point mPoint[4]' member.


> @@ +262,5 @@
> > +{
> > +  Float A = a + b, S = a - b;
> > +  Float A2 = pow(A, 2.0f), S2 = pow(S, 2.0f);
> > +
> > +  return M_PI * A * (3.00f * S2 / (A2 * (sqrt(-3.0f * S2 / A2 + 4.0f) + 10.0f)) + 1.0f) / 4.0f;
> 
> Where does this formula come from?

I should've added the command and the reference for it.
It's Ramanujan's approximation:
  https://en.wikipedia.org/wiki/Ellipse#Circumference
  http://books.google.com/books?id=oSioAM4wORMC&pg=PA39
Added comment, and slightly modified the formula to reduce the number of division.


> @@ +6,5 @@
> > +#ifndef mozilla_BezierUtils_h_
> > +#define mozilla_BezierUtils_h_
> > +
> > +#include "mozilla/gfx/2D.h"
> > +#include "gfxRect.h"
> 
> Is this include needed?

mozilla::css::Corner is defined there, and GetBezierPointsForCorner uses it.


> @@ +197,5 @@
> > +  Point innerSectionPoints[4];
> > +  SplitBezierB(tmp, mOuterPoints, lastTo);
> > +  SplitBezierA(outerSectionPoints, tmp, ConvertRange(to, lastTo));
> > +  SplitBezierB(tmp, mInnerPoints, lastTi);
> > +  SplitBezierA(innerSectionPoints, tmp, ConvertRange(to, lastTo));
> 
> Should this be?
> SplitBezierA(innerSectionPoints, tmp, ConvertRange(ti, lastTi)); instead of
> To?
> 
> Would it be better to have a SplitBezier function that takes a t1 and t2?

wrapped those 2 functions with GetSubBezier function.
SplitBezierA and SplitBezierB is defined in BezierUtils.cpp with 'static'.


> ::: layout/base/DashedCornerFinder.h
> @@ +175,5 @@
> > +  Point mLastPi;
> > +  Float mLastTo;
> > +  Float mLastTi;
> > +
> > +  // Length for each segment, radio of the border width at that point.
> 
> Is 'radio' a typo?

Oops, it should be 'ratio'.
Comment 152 Tooru Fujisawa [:arai] 2015-12-26 20:14:10 PST
Created attachment 8702088 [details] [diff] [review]
Part 0: Add missing includes and namespaces.

Some files are changed from last time, and fixed gfx/2d directory too, as BezierUtils.cpp is moved there and I hit error there.

[gfx/2d/DataSurfaceHelpers.h]
https://dxr.mozilla.org/mozilla-central/rev/388bdc46ba51ee31da8b8abe977e0ca38d117434/gfx/2d/DataSurfaceHelpers.h#93
>  DataAtOffset(DataSourceSurface* aSurface,
> -             DataSourceSurface::MappedSurface* aMap,
> +             const DataSourceSurface::MappedSurface* aMap,
>               IntPoint aPoint);

This is not include or namespace tho, this declaration does not match to definition in DataSurfaceHelpers.cpp.


[gfx/2d/Quaternion.h]

> +#include <ostream>
https://dxr.mozilla.org/mozilla-central/rev/388bdc46ba51ee31da8b8abe977e0ca38d117434/gfx/2d/Quaternion.h#37
>   friend std::ostream& operator<<(std::ostream& aStream, const Quaternion& aQuat);

ostream is used here


> +#include "mozilla/gfx/Point.h"
https://dxr.mozilla.org/mozilla-central/rev/388bdc46ba51ee31da8b8abe977e0ca38d117434/gfx/2d/Quaternion.h#95
>   Point3D RotatePoint(const Point3D& aPoint) {

Point3D is used here.


[layout/base/DisplayItemScrollClip.cpp]

> +#include "DisplayItemClip.h"
http://hg.mozilla.org/mozilla-central/file/68b33692bed3/layout/base/DisplayItemScrollClip.cpp#l34
>     str.AppendPrintf("<%s>", scrollClip->mClip ? scrollClip->mClip->ToString().get() : "null");

mClip is DisplayItemClip.


[layout/base/GeometryUtils.cpp]

> +#include "nsContentUtils.h"
https://dxr.mozilla.org/mozilla-central/rev/388bdc46ba51ee31da8b8abe977e0ca38d117434/layout/base/GeometryUtils.cpp#233
>   if (nsContentUtils::IsCallerChrome()) {

nsContentUtils is used here.


[layout/base/nsPresShell.cpp]

> +#include "gfxUserFontSet.h"
https://dxr.mozilla.org/mozilla-central/rev/388bdc46ba51ee31da8b8abe977e0ca38d117434/layout/base/nsPresShell.cpp#1088
>     gfxUserFontSet* fs = mPresContext->GetUserFontSet();

gfxUserFontSet is used here.


[layout/generic/nsGridContainerFrame.h]

> +  typedef mozilla::LogicalSize LogicalSize;
http://hg.mozilla.org/mozilla-central/file/a296d149c69e/layout/generic/nsGridContainerFrame.h#l548
>                       const LogicalSize& aComputedMinSize,
>                       const LogicalSize& aComputedSize,
>                       const LogicalSize& aComputedMaxSize);

LogicalSize needs namespace.


[layout/style/nsFontFaceLoader.h]

https://dxr.mozilla.org/mozilla-central/rev/388bdc46ba51ee31da8b8abe977e0ca38d117434/layout/style/nsFontFaceLoader.h#59
> -  TimeStamp               mStartTime;
> +  mozilla::TimeStamp      mStartTime;

TimeStamp needs namespace.
Comment 153 Tooru Fujisawa [:arai] 2015-12-26 20:15:29 PST
Created attachment 8702089 [details] [diff] [review]
Part 0-followup: Add missing includes and namespaces. r=jrmuizel

Just removed unnecessary changes, such as TouchCaret.h that is already removed.
Comment 154 Tooru Fujisawa [:arai] 2015-12-26 20:16:47 PST
Created attachment 8702090 [details] [diff] [review]
Part 1: Fix spacing of simple 2px dotted border without radius.

also, rebase other parts.
Comment 155 Tooru Fujisawa [:arai] 2015-12-26 20:17:23 PST
Created attachment 8702091 [details] [diff] [review]
Part 2: Split constants for border rendering to BorderConsts.h.
Comment 156 Tooru Fujisawa [:arai] 2015-12-26 20:17:55 PST
Created attachment 8702092 [details] [diff] [review]
Part 3: Improve spacing and corner interaction of dashed/dotted border. r=jrmuizel
Comment 157 Tooru Fujisawa [:arai] 2016-01-07 12:33:15 PST
Thank you for reviewing :)
I'm going to land Part 0-2 after try run, as they don't depend on other parts.
Comment 158 Tooru Fujisawa [:arai] 2016-01-07 19:46:15 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/04539f076e4587a0975527828bf43e129e8ed0ba
Bug 382721 - Part 0: Add missing includes and namespaces. r=jrmuizel

https://hg.mozilla.org/integration/mozilla-inbound/rev/1990c87edb47f88fd93e4879d65427974c57fd22
Bug 382721 - Part 1: Fix spacing of simple 2px dotted border without radius. r=jrmuizel

https://hg.mozilla.org/integration/mozilla-inbound/rev/399dfde76376dd73e3d3f03e21afb80204bf796d
Bug 382721 - Part 2: Split constants for border rendering to BorderConsts.h. r=jrmuizel
Comment 160 Boris Zbarsky [:bz] (still a bit busy) 2016-01-08 11:35:31 PST
*** Bug 973037 has been marked as a duplicate of this bug. ***
Comment 161 j.j. 2016-01-09 13:56:22 PST
*** Bug 995677 has been marked as a duplicate of this bug. ***
Comment 162 Mats Palmgren (:mats) 2016-03-17 19:13:26 PDT
*** Bug 1257447 has been marked as a duplicate of this bug. ***
Comment 163 Tooru Fujisawa [:arai] 2016-05-27 13:17:03 PDT
can you review Parts 4-5?
Comment 164 Jeff Muizelaar [:jrmuizel] 2016-06-07 08:22:34 PDT
I'm going to finish this review today.
Comment 165 Tooru Fujisawa [:arai] 2016-06-07 17:52:53 PDT
Thank you for reviewing :)
Will land after testing again.
Comment 166 Jeff Muizelaar [:jrmuizel] 2016-06-07 17:54:50 PDT
(In reply to Tooru Fujisawa [:arai] from comment #165)
> Thank you for reviewing :)
> Will land after testing again.

Sorry it took forever and thanks for being patient and all the work you did.
Comment 167 Tooru Fujisawa [:arai] 2016-06-08 00:43:00 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/e2881c49e123c204f835953d6c37f004f1549545
Bug 382721 - Part 3: Improve spacing and corner interaction of dashed/dotted border. r=jrmuizel

https://hg.mozilla.org/integration/mozilla-inbound/rev/6f2840c13fb097e422df8b293cf6f6e590f4a6c6
Bug 382721 - Part 4: Support dotted/dashed border-radiused corners. r=jrmuizel

https://hg.mozilla.org/integration/mozilla-inbound/rev/e8d313145a591f5601a7b568194864b7dcddc11f
Bug 382721 - Part 5: Add testcases for dashed/dotted borders. r=jrmuizel
Comment 169 Tooru Fujisawa [:arai] 2016-06-08 02:27:12 PDT
It crashes on the following testcase.

https://dxr.mozilla.org/mozilla-central/source/layout/generic/crashtests/730559.html
> <!DOCTYPE html><html style="height: 6523790304542em; width: 6207636626031em; box-sizing: border-box; border-style: dotted; -moz-column-width: 20px;"></html>

it's a huge box with dotted border, but no radius, so apparently from Part 3.
It takes so much time because it tries to draw each dot along super long border.
not yet figures out the actual reason of the crash tho.

should be better testing a huge box with radius also.
Comment 170 Tooru Fujisawa [:arai] 2016-06-08 07:03:41 PDT
It crashes because of OOM while generating large path.
we should flush it after some segment.
Comment 171 Tooru Fujisawa [:arai] 2016-06-08 09:04:50 PDT
the 730559.html's case could be fixed by the comment #170 approach.

But, for a large box with large radius (like 6523790304542em), it's not possible to calculate the fitting path in reasonable time, with current approach, as we need to search it with loops.
We might need to add some fallback for that case, with less complexity, like using single border-width even adjacent borders have different width.

Is there any guideline for the case when webpage specifies unreasonable style?
looks like, bug 730559 adds a limitation on the column count.  is it reasonable to add some limitation here too?
Comment 172 Jeff Muizelaar [:jrmuizel] 2016-06-08 09:41:56 PDT
(In reply to Tooru Fujisawa [:arai] from comment #171)
> the 730559.html's case could be fixed by the comment #170 approach.
> 
> But, for a large box with large radius (like 6523790304542em), it's not
> possible to calculate the fitting path in reasonable time, with current
> approach, as we need to search it with loops.
> We might need to add some fallback for that case, with less complexity, like
> using single border-width even adjacent borders have different width.
> 
> Is there any guideline for the case when webpage specifies unreasonable
> style?
> looks like, bug 730559 adds a limitation on the column count.  is it
> reasonable to add some limitation here too?

Is is possible to look at the clip and restrict the problem?
Comment 173 Jeff Muizelaar [:jrmuizel] 2016-06-08 09:43:16 PDT
(In reply to Jeff Muizelaar [:jrmuizel] from comment #172)
> (In reply to Tooru Fujisawa [:arai] from comment #171)
> > the 730559.html's case could be fixed by the comment #170 approach.
> > 
> > But, for a large box with large radius (like 6523790304542em), it's not
> > possible to calculate the fitting path in reasonable time, with current
> > approach, as we need to search it with loops.
> > We might need to add some fallback for that case, with less complexity, like
> > using single border-width even adjacent borders have different width.

Also, which specific part of the code do we get stuck in for these very large cases?
Comment 174 Tooru Fujisawa [:arai] 2016-06-08 10:21:47 PDT
Thanks :)

There are 3 places that takes time and/or memory.

A) building the path with many parts

nsCSSBorderRenderer::DrawDottedSideSlow
https://hg.mozilla.org/integration/mozilla-inbound/diff/e2881c49e123/layout/base/nsCSSRenderingBorders.cpp#l1.1399
nsCSSBorderRenderer::DrawDottedCornerSlow
https://hg.mozilla.org/integration/mozilla-inbound/diff/6f2840c13fb0/layout/base/nsCSSRenderingBorders.cpp#l1.411
nsCSSBorderRenderer::DrawDashedCornerSlow
https://hg.mozilla.org/integration/mozilla-inbound/diff/6f2840c13fb0/layout/base/nsCSSRenderingBorders.cpp#l1.446

This exists both in side and corner, and this is mostly memory issue.
They could be improved by skipping paths outside clip.
(the clip you're saying is the currently visible or updating area on the screen, right?)


B) finding the best overlap for dotted, and the best dash length for dashed

DottedCornerFinder::FindBestOverlap
https://hg.mozilla.org/integration/mozilla-inbound/diff/6f2840c13fb0/layout/base/DottedCornerFinder.cpp#l1.371
DashedCornerFinder::FindBestDashLength
https://hg.mozilla.org/integration/mozilla-inbound/diff/6f2840c13fb0/layout/base/DashedCornerFinder.cpp#l1.295

This exists only in corner, and this is time issue.

I'm not sure if this can be solved by clip, without loosing accuracy,
as we're searching an overlap/length that satisfies dotted/dashed corner connects smoothly with adjacent sides, that's not related to the clipped area.

If we don't have to connect to adjacent side smoothly, like, if inconsistent gap between corner and side are allowed for those cases, we could skip this.


C) finding next dot or dash

DottedCornerFinder::FindNext
https://hg.mozilla.org/integration/mozilla-inbound/diff/6f2840c13fb0/layout/base/DottedCornerFinder.cpp#l1.243
DashedCornerFinder::FindNext
https://hg.mozilla.org/integration/mozilla-inbound/diff/6f2840c13fb0/layout/base/DashedCornerFinder.cpp#l1.206

This also exists only in corner, and is also time issue.

They are called inside B, and while generating paths, both with loop along the corner.

If we have to generate a consistent rendering throughout scrolling, we should calculate whole paths along the corner, or at least up to that point, so clip won't improve much (at most it could reduce the time taken by it to half, in average).

So, to reduce the time taken by this, we need some different way.
Will try finding out better solution.
Comment 175 Jeff Muizelaar [:jrmuizel] 2016-06-08 10:24:26 PDT
(In reply to Tooru Fujisawa [:arai] from comment #171)
> Is there any guideline for the case when webpage specifies unreasonable
> style?
> looks like, bug 730559 adds a limitation on the column count.  is it
> reasonable to add some limitation here too?

I think it is completely reasonable to add a limitation here. It's worth looking at what other browsers do in these more extreme cases.
Comment 176 Tooru Fujisawa [:arai] 2016-06-08 10:57:07 PDT
(In reply to Jeff Muizelaar [:jrmuizel] from comment #175)
> (In reply to Tooru Fujisawa [:arai] from comment #171)
> > Is there any guideline for the case when webpage specifies unreasonable
> > style?
> > looks like, bug 730559 adds a limitation on the column count.  is it
> > reasonable to add some limitation here too?
> 
> I think it is completely reasonable to add a limitation here. It's worth
> looking at what other browsers do in these more extreme cases.

Okay, will add limitation.

About other browsers, here's there result with the following HTML:

<div style="height: 6523790304542em; width: 6207636626031em; box-sizing: border-box; border-top-right-radius: 6523790304542em; border-style: dotted; border-width: 1em 2em;">hello</div>

Safari: content process stops responding, at least 1 minutes
Chrome: corner is rendered as solid style, side is kept dotted/dashed (chrome uses rectangular dotted tho...)

will check Edge after downloading VM, but I think Chrome's way seems reasonable.
Comment 177 Tooru Fujisawa [:arai] 2016-06-08 12:47:38 PDT
Edge: somehow ignores height and displays thin box with no radius.  when I reduce the height and width to 60000em, content process stops responding, at least 1 minutes


I'll ignore dotted/dashed for large corner.
Comment 178 Jeff Muizelaar [:jrmuizel] 2016-06-08 20:06:34 PDT
When using the dirty rectangle it's best to just discard elements that are completely outside of the rectangle. Trying to manually clip the geometry to the dirty rectangle can cause the antialiasing to be different depending on the clip which we'd like to avoid. i.e. ideally the dirty rect shouldn't change any of the actual points in the path geometry.

Hopefully, using the dirty rect and adding some limits is enough to handle the more extreme test cases.
Comment 179 Tooru Fujisawa [:arai] 2016-06-09 01:10:17 PDT
Thanks :)

Now testing with 3 more patches applied.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cc9ccfc2deec

Part 6
  Draw and flush combined path after adding 100 segments
  to avoid generating too large paths and causing OOM
Part 7
  Generate path only inside dirty rect
  for dotted side, it calculates only required area
  for corners, it calculates until path enters and leaves the dirty rect
Part 8
  Render too large dotted/dashed corner with solid style
Comment 180 sjw 2016-06-09 05:41:03 PDT
>   Render too large dotted/dashed corner with solid style

Can we have a hint in the console in this case to help webdevs debugging?
(see also #946430 for a similar cause.)
Comment 181 Tooru Fujisawa [:arai] 2016-06-09 08:29:03 PDT
Created attachment 8761618 [details] [diff] [review]
Part 6: Flush path while building long dotted/dashed border.

Added the limitation on the maximum number of segment (dot or dash) while generating path inside the following method:
  * nsCSSBorderRenderer::DrawDottedSideSlow
  * nsCSSBorderRenderer::DrawDottedCornerSlow
  * nsCSSBorderRenderer::DrawDashedCornerSlow

Now the max is 100, and if it exceeds, it draws the path and continues with new path builder.

This fixes the OOM while building the path.
Comment 182 Tooru Fujisawa [:arai] 2016-06-09 08:31:49 PDT
Created attachment 8761621 [details] [diff] [review]
Part 7: Render only dirty area for dotted side and dotted/dashed corner.

Added aDirtyRect parameter to nsCSSBorderRenderer constructor, and use it inside the following methods:
  * nsCSSBorderRenderer::DrawDottedSideSlow
  * nsCSSBorderRenderer::DrawDottedCornerSlow
  * nsCSSBorderRenderer::DrawDashedCornerSlow

aDirtyRect can be nullptr, as we don't have that information for nsCSSRendering::PaintFocus, and in that case the following checks are just ignored.

In nsCSSBorderRenderer::DrawDottedSideSlow, it draws each dot between |from| and |to|, if the point for |from| and/or |to| is outside of the |dirty rect + radius + anti-aliasing margin|, |from| and/or |to| is changed to fit into the rect.

In nsCSSBorderRenderer::DrawDottedCornerSlow, it checks if each dot is inside the |dirty rect + maximum radius + anti-aliasing margin|, and renders only if it's inside the rect.
Also, it breaks the loop when it enters and leaves the rect, as once it leaves the rect, it won't re-enter.

In nsCSSBorderRenderer::DrawDashedCornerSlow, it checks if a rect that encloses all control points of the dash overlaps with the |dirty rect + anti-aliasing margin|, and renders only if they overlap.
Like dotted, it breaks the loop when it enters and leaves the rect.

This fixes the performance issue when rendering a bit large dotted/dashed box.
Also, this fixes the OOM issue addressed in Part 6 in most case, but as aDirtyRect can be nullptr, Part 6 is still needed.
Comment 183 Tooru Fujisawa [:arai] 2016-06-09 08:34:04 PDT
Created attachment 8761622 [details] [diff] [review]
Part 8: Render too large dotted/dashed corner with solid style.

Added the limitaion on the maximum border-radius of dashed/dotted, now it's 100000px, and if it exceeds, the corner is rendered as solid.

This fixes the performance issue when rendering too large dotted/dashed corner.
Comment 184 Tooru Fujisawa [:arai] 2016-06-09 08:35:20 PDT
Created attachment 8761624 [details] [diff] [review]
Part 9: Warn about too large dotted/dashed corner.

Added a warning message for Part 8's case.
The warning is shown only once per PresContext, to avoid flooding the console.
Comment 185 Zack Weinberg (:zwol) 2016-06-09 08:55:31 PDT
> +TooLargeDashedOrDottedRadius=Border radius for dashed/dotted exceeds the limit (100000px) and rendered as solid style.

Better wording: "Border radius is too large for dashes or dots (the limit is 100000px). Rendering as solid."

Also, when this happens, I think it would look better if the *entire border* were rendered as solid, not just the corner.  That's not very important, though, since this will hardly ever come up in real websites (who even has a monitor that big?)
Comment 186 Tooru Fujisawa [:arai] 2016-06-09 09:21:17 PDT
(In reply to Zack Weinberg (:zwol) from comment #185)
> > +TooLargeDashedOrDottedRadius=Border radius for dashed/dotted exceeds the limit (100000px) and rendered as solid style.
> 
> Better wording: "Border radius is too large for dashes or dots (the limit is
> 100000px). Rendering as solid."

Thanks :)
Is "dashes or dots" better than "dashed or dotted" ?
I feel it should be better using the actual value (dashed or dotted).


> Also, when this happens, I think it would look better if the *entire border*
> were rendered as solid, not just the corner.  That's not very important,
> though, since this will hardly ever come up in real websites (who even has a
> monitor that big?)

as border style can be different for each side (top, right, bottom, and left), what we can do is to use solid for the side
which contains too large radius.
Then, actually, that needs more change than current one (as there are interaction between dotted and others at the corner without radius), so I'd like to keep using current patch's way if it's acceptable.
Comment 187 Zack Weinberg (:zwol) 2016-06-09 09:34:53 PDT
(In reply to Tooru Fujisawa [:arai] from comment #186)
> (In reply to Zack Weinberg (:zwol) from comment #185)
> > > +TooLargeDashedOrDottedRadius=Border radius for dashed/dotted exceeds the limit (100000px) and rendered as solid style.
> > 
> > Better wording: "Border radius is too large for dashes or dots (the limit is
> > 100000px). Rendering as solid."
> 
> Thanks :)
> Is "dashes or dots" better than "dashed or dotted" ?
> I feel it should be better using the actual value (dashed or dotted).

In this context, "dashed" and "dotted" would appear ungrammatical.  (The sentence is in the present tense, so -ed verb forms look improperly conjugated.)  You could do

    Border radius is too large for 'dashed' or 'dotted' style

without grammar problems, but "Border radius is too large for dashes or dots" communicates the same thing and is shorter, and this message is already rather long.

> as border style can be different for each side (top, right, bottom, and
> left), what we can do is to use solid for the side
> which contains too large radius.
> Then, actually, that needs more change than current one (as there are
> interaction between dotted and others at the corner without radius), so I'd
> like to keep using current patch's way if it's acceptable.

Oh, good point.  Fine by me to leave as is, then.
Comment 188 Tooru Fujisawa [:arai] 2016-06-09 10:03:21 PDT
(In reply to Zack Weinberg (:zwol) from comment #187)
> (In reply to Tooru Fujisawa [:arai] from comment #186)
> > (In reply to Zack Weinberg (:zwol) from comment #185)
> > > > +TooLargeDashedOrDottedRadius=Border radius for dashed/dotted exceeds the limit (100000px) and rendered as solid style.
> > > 
> > > Better wording: "Border radius is too large for dashes or dots (the limit is
> > > 100000px). Rendering as solid."
> > 
> > Thanks :)
> > Is "dashes or dots" better than "dashed or dotted" ?
> > I feel it should be better using the actual value (dashed or dotted).
> 
> In this context, "dashed" and "dotted" would appear ungrammatical.  (The
> sentence is in the present tense, so -ed verb forms look improperly
> conjugated.)  You could do
> 
>     Border radius is too large for 'dashed' or 'dotted' style
> 
> without grammar problems, but "Border radius is too large for dashes or
> dots" communicates the same thing and is shorter, and this message is
> already rather long.

Thanks again :D

"'dashed' or 'dotted' style" sounds better for me.

to be clear, there are 2 reasons why I feel using 'dashed' or 'dotted' is better:

  1. the message is the target of localization
     I'm not sure if localized "dashes or dots" communicates the same thing as "'dashed' or 'dotted'" in each locale
     so using keywords and avoid translating those words seems better

  2. we cannot point any particular line/rule for the warning message (like, the line 10 of foo.css)
     because it's detected while rendering, not parsing
     so using keywords seems to be more developer-friendly, as they can search with them
Comment 189 Zack Weinberg (:zwol) 2016-06-09 10:06:51 PDT
>  1. the message is the target of localization
>     I'm not sure if localized "dashes or dots" communicates the same thing as "'dashed'
>     or 'dotted'" in each locale so using keywords and avoid translating those words
>     seems better
>
>  2. we cannot point any particular line/rule for the warning message (like, the line 10
>     of foo.css) because it's detected while rendering, not parsing, so using keywords
>     seems to be more developer-friendly, as they can search with them

OK, that's fair.  In that case, could we have two messages, one for dashed and one for dotted?  That would make life even easier for developers.
Comment 190 Tooru Fujisawa [:arai] 2016-06-09 10:36:10 PDT
Created attachment 8761730 [details] [diff] [review]
Part 9: Warn about too large dotted/dashed corner.

Okay, changed the wording, and separated the message for dashed and dotted.
Comment 191 Jeff Muizelaar [:jrmuizel] 2016-06-09 13:03:43 PDT
Comment on attachment 8761621 [details] [diff] [review]
Part 7: Render only dirty area for dotted side and dotted/dashed corner.

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

::: layout/base/nsCSSRendering.cpp
@@ +987,5 @@
>    // to a style context and can use the same logic that PaintBorder
>    // and PaintOutline do.)
>    nsCSSBorderRenderer br(aPresContext->Type(),
>                           aDrawTarget,
> +                         nullptr,

Instead of passing nullptr can we pass focusRect? That should keep the called code simpler.

::: layout/base/nsCSSRenderingBorders.cpp
@@ +2225,5 @@
> +        }
> +      }
> +    }
> +  }
> +

Is this hunk required to avoid OOM or does it just allow us to save time? If we don't need it, I'd prefer to keep it out for now.
Comment 192 Jeff Muizelaar [:jrmuizel] 2016-06-09 13:05:39 PDT
Comment on attachment 8761730 [details] [diff] [review]
Part 9: Warn about too large dotted/dashed corner.

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

::: layout/base/nsCSSRendering.cpp
@@ +806,2 @@
>    nsCSSBorderRenderer br(aPresContext->Type(),
> +                         aPresContext,

If we're going to be passing in the PresContext let's not pass in the type separately.
Comment 193 Tooru Fujisawa [:arai] 2016-06-09 17:09:40 PDT
Created attachment 8761872 [details] [diff] [review]
Part 7: Render only dirty area for dotted side and dotted/dashed corner.

(In reply to Jeff Muizelaar [:jrmuizel] from comment #191)
> Comment on attachment 8761621 [details] [diff] [review]
> Part 7: Render only dirty area for dotted side and dotted/dashed corner.
> 
> Review of attachment 8761621 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: layout/base/nsCSSRendering.cpp
> @@ +987,5 @@
> >    // to a style context and can use the same logic that PaintBorder
> >    // and PaintOutline do.)
> >    nsCSSBorderRenderer br(aPresContext->Type(),
> >                           aDrawTarget,
> > +                         nullptr,
> 
> Instead of passing nullptr can we pass focusRect? That should keep the
> called code simpler.

Yeah, it works, as we're checking overlap with margin.


> ::: layout/base/nsCSSRenderingBorders.cpp
> @@ +2225,5 @@
> > +        }
> > +      }
> > +    }
> > +  }
> > +
> 
> Is this hunk required to avoid OOM or does it just allow us to save time? If
> we don't need it, I'd prefer to keep it out for now.

It's to save time.
Changed DrawDottedSideSlow to use the same way as DrawDottedCornerSlow, and it's still displayed without any notable lag.
Comment 194 Tooru Fujisawa [:arai] 2016-06-09 17:11:02 PDT
Created attachment 8761875 [details] [diff] [review]
Part 9: Warn about too large dotted/dashed corner.

removed nsPresContextType parameter.
Comment 195 Tooru Fujisawa [:arai] 2016-06-09 21:13:31 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/f7cfef76a52cea7fac8fa2204c492e32f0e5e4a0
Bug 382721 - Part 3: Improve spacing and corner interaction of dashed/dotted border. r=jrmuizel

https://hg.mozilla.org/integration/mozilla-inbound/rev/0863ce40d2f743fae75d2951085bff0f546a9b11
Bug 382721 - Part 4: Support dotted/dashed border-radiused corners. r=jrmuizel

https://hg.mozilla.org/integration/mozilla-inbound/rev/63f10496a3e4e3deaa70323f7b63d6e8f500a8cc
Bug 382721 - Part 5: Add testcases for dashed/dotted borders. r=jrmuizel

https://hg.mozilla.org/integration/mozilla-inbound/rev/f8d2cc53a4d653ee631eb77b565da6d2a8e8cdd5
Bug 382721 - Part 6: Flush path while building long dotted/dashed border. r=jrmuizel

https://hg.mozilla.org/integration/mozilla-inbound/rev/ddf602c6e885b6a14b4946eeee44a7f77c5db913
Bug 382721 - Part 7: Render only dirty area for dotted side and dotted/dashed corner. r=jrmuizel

https://hg.mozilla.org/integration/mozilla-inbound/rev/ba37839e4fdebd7575845f4ed0b06a9c191c30b2
Bug 382721 - Part 8: Render too large dotted/dashed corner with solid style. r=jrmuizel

https://hg.mozilla.org/integration/mozilla-inbound/rev/c3f66208fe851ffd0b03dbc1b2cbe6883d2e6d21
Bug 382721 - Part 9: Warn about too large dotted/dashed corner. r=jrmuizel
Comment 196 Carsten Book [:Tomcat] 2016-06-10 06:40:25 PDT
sorry had to back this out since this caused perma failures like https://treeherder.mozilla.org/logviewer.html#?job_id=29956176&repo=mozilla-inbound
Comment 198 Tooru Fujisawa [:arai] 2016-06-10 06:57:40 PDT
hmmmm, so, the code block in nsCSSBorderRenderer::nsCSSBorderRenderer for tweaking |from| and |to| was necessary for Android, with dotted rendering after these patch.
Even just iterating along the path is too heavy for it :(

will post the patch with previous way.
Comment 199 Tooru Fujisawa [:arai] 2016-06-10 07:15:41 PDT
got r+ed over IRC.
will land after extra try run on latest m-i.
Comment 200 Tooru Fujisawa [:arai] 2016-06-10 11:16:17 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/5306a743dbdfbb4cd223fdc0498bcf91d5942abe
Bug 382721 - Part 3: Improve spacing and corner interaction of dashed/dotted border. r=jrmuizel

https://hg.mozilla.org/integration/mozilla-inbound/rev/22e1df7564ca1e04a6e1410e4348336a14d3a62f
Bug 382721 - Part 4: Support dotted/dashed border-radiused corners. r=jrmuizel

https://hg.mozilla.org/integration/mozilla-inbound/rev/f681dfc661e79c5ea57650effda2d6eb217a36d2
Bug 382721 - Part 5: Add testcases for dashed/dotted borders. r=jrmuizel

https://hg.mozilla.org/integration/mozilla-inbound/rev/0d02a845d62dcda3e8b214569093ffcb6ff53632
Bug 382721 - Part 6: Flush path while building long dotted/dashed border. r=jrmuizel

https://hg.mozilla.org/integration/mozilla-inbound/rev/8d81a2b35bc0f525319b097a79fa97ff89d4f514
Bug 382721 - Part 7: Render only dirty area for dotted side and dotted/dashed corner. r=jrmuizel

https://hg.mozilla.org/integration/mozilla-inbound/rev/04c0b0cde81d17048bd42c4d618985b1cd826641
Bug 382721 - Part 8: Render too large dotted/dashed corner with solid style. r=jrmuizel

https://hg.mozilla.org/integration/mozilla-inbound/rev/0ac2613adaeefd45f9cde2d4d16746484a1be5e5
Bug 382721 - Part 9: Warn about too large dotted/dashed corner. r=jrmuizel
Comment 202 Mardeg 2016-06-12 08:46:14 PDT
It appears this also fixes bug 19963, bug 315209 and bug 379303 upon viewing their attached testcases, and also fixes the data: URL tests mentioned in bug 652650 comment 18. Should we resolve them as FIXED now?
Comment 203 Zack Weinberg (:zwol) 2016-06-12 10:01:13 PDT
(In reply to Mardeg from comment #202)
> It appears this also fixes bug 19963, bug 315209 and bug 379303 upon viewing
> their attached testcases, and also fixes the data: URL tests mentioned in
> bug 652650 comment 18. Should we resolve them as FIXED now?

Please make sure the test cases are added to the testsuite, at least.
Comment 204 j.j. 2016-06-12 11:11:27 PDT
(In reply to Mardeg from comment #202)
> It appears this also fixes bug 19963, bug 315209 and bug 379303 

Bug 315209 is not fixed
Comment 205 Tooru Fujisawa [:arai] 2016-06-20 08:46:35 PDT
Tests for Bug 19963 and Bug 652650 are included (slightly different than original one, for simplicity in reftest)
I'm not sure what's the expected behavior for Bug 379303 tho (it just says "pretty"...), dashed/dotted borders are now symmetric and test for that is included.
Bug 315209 is indeed not-yet-fixed, and totally different thing I think.
Comment 206 Jean-Yves Perrier [:teoli] 2016-07-15 06:07:15 PDT
Doc updated:
https://developer.mozilla.org/en-US/Firefox/Releases/50#CSS
as well as a comment under the browser compatibility tables, in shorthand/longhand properties involved:
https://developer.mozilla.org/en-US/docs/Web/CSS/border-radius
Comment 207 Florian Bender 2016-08-19 07:01:49 PDT
Release Note Request (optional, but appreciated)
[Why is this notable]: Long-standing issue that (while small) always drew a lot of attention
[Suggested wording]: Fixed rendering of dashed and dotted borders with rounded corners (border-radius)
[Links (documentation, blog post, etc)]: https://developer.mozilla.org/en-US/docs/Web/CSS/border-radius


Alternative wording adapted from release info linked in comment 206: "Corners with border-radius and dashed or dotted style is now rendered with specified style instead of solid style"


(In reply to Jean-Yves Perrier [:teoli] from comment #206)
> Doc updated:
> https://developer.mozilla.org/en-US/Firefox/Releases/50#CSS
> as well as a comment under the browser compatibility tables, in
> shorthand/longhand properties involved:
> https://developer.mozilla.org/en-US/docs/Web/CSS/border-radius

I updated the CSS/border-radius page again, there was another note that was not updated accordingly.
Comment 208 Gábor Stefanik 2016-08-19 07:34:09 PDT
Reproducible crash using current Developer Edition:

Open https://bugzilla.mozilla.org/attachment.cgi?id=8644118, and set these options:
type: dotted
corner: A
options: unchecked
radius-x: 199
radius-y: 286
top-width: 44
left-width: 0
width: 600
height: 507

Then, quickly drag the left-width slider around, back and forth, repeatedly hitting the 0 end. Tab will crash after a few tries.

Example crash: https://crash-stats.mozilla.com/report/index/55a48e5d-3d2f-4244-8c7f-628f02160819

Looks like some kind of race condition to me.
Comment 209 Ryan VanderMeulen [:RyanVM] 2016-08-19 07:41:10 PDT
Please file a new bug for that crash and mark it blocking this bug rather than adding new comments here that are prone to being lost.
Comment 210 Astley Chen [:astley] UTC+8 2016-08-25 23:05:30 PDT
dev-doc update needed: https://developer.mozilla.org/en-US/docs/Web/CSS/-moz-outline-radius#Notes
Comment 211 Ritu Kothari (:ritu) 2016-09-19 10:02:13 PDT
Added to Fx50 Beta release notes.
Comment 212 Mats Palmgren (:mats) 2016-09-30 09:46:37 PDT
*** Bug 1306615 has been marked as a duplicate of this bug. ***
Comment 214 Massamino 2016-11-18 00:49:36 PST
Created attachment 8812097 [details]
Rendering problem

Hi,

I did a test at https://bugzilla.mozilla.org/attachment.cgi?id=8644118, and set these options:
type: dotted
corner: A
options: unchecked or checked
radius-x: 165
radius-y: 301
top-width: 5
left-width: 0
width: 501
height: 600

I can see artifacts on the left and right (FF 50). There are dots that shouldn't be there.
Hope this is still a problem of this bug.
Comment 215 kamei 2016-11-18 01:01:10 PST
reproduced on Fx 51.0a2
Comment 216 Julien Wajsberg [:julienw] 2016-11-18 01:04:27 PST
Massamino, I think you should file a separate bug with your testcase, with a "blocks" on this bug.

It would also be a good thing if you can simplify it to demonstrate the issue right after opening it.

Thanks a lot !

(note: I reproduce on my Aurora v51)
Comment 217 Massamino 2016-11-18 02:48:50 PST
(In reply to Julien Wajsberg [:julienw] from comment #216)
> Massamino, I think you should file a separate bug with your testcase, with a
> "blocks" on this bug.
> 
> It would also be a good thing if you can simplify it to demonstrate the
> issue right after opening it.
> 
> Thanks a lot !
> 
> (note: I reproduce on my Aurora v51)

I filed a new bug and created my own simplified example.
https://bugzilla.mozilla.org/show_bug.cgi?id=1318625

Note You need to log in before you can comment on or make changes to this bug.