Bug 41301

Summary: Drawing border-radius from path sometimes fails to round outer border in the double style when it should
Product: WebKit Reporter: Beth Dakin <bdakin>
Component: CSSAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, simon.fraser, vikingjs
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Bug Depends on: 58761    
Bug Blocks:    
Attachments:
Description Flags
Test case
none
Page demonstrating several border issues, along with proposed fix. none

Beth Dakin
Reported 2010-06-28 14:01:49 PDT
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.
Attachments
Test case (321 bytes, text/html)
2010-06-28 14:02 PDT, Beth Dakin
no flags
Page demonstrating several border issues, along with proposed fix. (77.76 KB, application/xhtml+xml)
2010-11-24 19:01 PST, Jerry Seeger
no flags
Beth Dakin
Comment 1 2010-06-28 14:02:34 PDT
Created attachment 59936 [details] Test case
Jerry Seeger
Comment 2 2010-11-24 19:01:53 PST
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)
Comment 3 2010-11-26 10:42:30 PST
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
Comment 4 2010-11-27 14:26:16 PST
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)
Comment 5 2010-11-28 09:31:38 PST
(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)
Comment 6 2011-04-19 11:46:46 PDT
Note You need to log in before you can comment on or make changes to this bug.