Bug 41301 – Drawing border-radius from path sometimes fails to round outer border in the double style when it should

• Component: CSS
• Status: RESOLVED FIXED

[Beth Dakin]

http://trac.webkit.org/changeset/62035 introduced a new method of drawing border-radius using paths. Right now, this new code is only enabled for some platforms…if you want to know if your favorite platform has the new code path enabled, see if it has been added to #define HAVE_PATH_BASED_BORDER_RADIUS_DRAWING in RenderObject.h.

Attached is a test case that demonstrates a bug in the new code. The outer border should be rounded on the inside. This should probably be fixed in RenderBoxModelObject's borderWillArcInnerEdge. I included the following FIXME--> 

// FIXME: This test is insufficient. We need to take border style into account.

[Beth Dakin]

Created attachment 59936 [details]
Test case

[Jerry Seeger]

Created attachment 74826 [details]
Page demonstrating several border issues, along with proposed fix.

This is a link to a page that compares the current browser's border-radius implementation to a separate implementation I have done in javaScript. The javascript implementation has several advantages over the current WebKit implementation:

1) Dots are round, and taper with the changing width of a corner
2) Many, many cases where gaps in corners don't occur (border width > radius is killer).
3) No need for modification to Core Graphics (can close bugs 41309-41313).
4) This also fixes open bugs 41302 and 41304

Manipulating the sliders on that page will also reveal many other issues with the current WebKit build, for which I don't see bug reports.

Unless someone strongly objects, I would like to submit a patch for WebKit based on this code. I created it as an exercise for myself but I've kept in mind the possibility porting it. (Currently the code uses SVG paths, but it will be simple to replace those with WebKit Paths.) It is a bit spendy on math right now, but is actually quite efficient when it comes to the drawing.

Although I realize that there has been a lot of work done on another approach to the problems, I think ultimately this ground-up rebuild will provide much more maintainable and adaptable code.

[Simon Fraser (smfr)]

Patches are very welcome! Your success at getting it accepted will depend on:
1. Having performance data showing that it's no slower in common cases
2. If possible, splitting the patch up into bite-sized chunks that are easier to review.
3. Having good testcases.

[Jerry Seeger]

Super. I'll take a crack at it, then. I'm assuming there is documentation somewhere about gathering performance data. I should be able to gather a large number of cases where the current implementation falls short. Should I add them all here or open a new bug with them all collected? 

Splitting the patch up might be tricky, since it's a ground-up rebuild, but I'll keep that in mind as I port the code.

(In reply to comment #3)
> Patches are very welcome! Your success at getting it accepted will depend on:
> 1. Having performance data showing that it's no slower in common cases
> 2. If possible, splitting the patch up into bite-sized chunks that are easier to review.
> 3. Having good testcases.

[Simon Fraser (smfr)]

(In reply to comment #4)
> Super. I'll take a crack at it, then. I'm assuming there is documentation somewhere about gathering performance data.

Not really, unfortunately, but once there's a patch here we can run it through some tests.

> I should be able to gather a large number of cases where the current implementation falls short. Should I add them all here or open a new bug with them all collected? 

That depends on whether they are distinct issues that will be fixed by different parts of the patch. One way to keep the patch size down may be to first land some changes that are behaviorally compatible with the current code, then make bugs and patches to fix the remaining issues, if that's possible.

Otherwise, one test case per unique issue is good once all is done.

> Splitting the patch up might be tricky, since it's a ground-up rebuild, but I'll keep that in mind as I port the code.

Understood.

[Simon Fraser (smfr)]

http://trac.webkit.org/changeset/84273