WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
42605
New border-radius path-based drawing code has some issues with corner-joins
https://bugs.webkit.org/show_bug.cgi?id=42605
Summary
New border-radius path-based drawing code has some issues with corner-joins
Beth Dakin
Reported
2010-07-19 16:37:15 PDT
The new path-based border-radius drawing code in CG has resulted in a corner-join regression. This is a good example of the regression being bad:
http://ie.microsoft.com/testdrive/Graphics/IE%20Logo/Default.html
This is because GraphicsContext::clipConvexPolygon() is currently written to be an anti-aliased clip. (That clipping sets up all the corner joins.) When we anti-alias that clip, we sometimes get these bad corner joints. In the long term, the solution to the problem is to be able to re-write the code so that it is capable of drawing multiple sides of the border at once, because this would not only prevent bad joins in the ie-logo case, but it would also fix our badness with alpha borders (see fast/borders/border-radius-wide-border-03.html). In fact, we have this ialpha-border issue with straight borders too! So that is not specific to border-radius. Right now, doing that seems like a major project, and it also seems messy in it's own way because I can't think of a way to write the code that doesn't involve branching for every possible combination of possibly-same-colored edges. Anyway, that would be good to figure out some day because it would make our corners look much better on borders with alpha. But since it is such a large project (and the alpha issue is not a regression nor is it unique to border-radius), I think it is beyond the scope of fixing this regression. I have a simple patch to fix just this issue that I will upload shortly.
Attachments
Patch
(190.43 KB, patch)
2010-07-19 17:08 PDT
,
Beth Dakin
no flags
Details
Formatted Diff
Diff
New patch
(982.13 KB, patch)
2010-07-21 16:31 PDT
,
Beth Dakin
mitz: review+
Details
Formatted Diff
Diff
Pixie screenshot
(26.36 KB, image/png)
2010-07-22 10:22 PDT
,
Beth Dakin
no flags
Details
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Beth Dakin
Comment 1
2010-07-19 17:08:11 PDT
Created
attachment 62015
[details]
Patch Here is a patch that disables anti-aliasing when the border is all one color. I chose not to disable anti-aliasing all the time because I felt like there were some examples in the fast/border directory that looked worse when anti-aliasing was disabled in all cases. It always looks better when the border is all one color though.
Beth Dakin
Comment 2
2010-07-20 18:54:34 PDT
Comment on
attachment 62015
[details]
Patch Building a better patch…
Beth Dakin
Comment 3
2010-07-21 16:31:40 PDT
Created
attachment 62246
[details]
New patch Here is a smarter patch that can handle anti-aliasing some corners and not others. Dan also suggested changing the boolean parameter to GraphicsContext::clipConvexPolygon() into an enum. I didn't do this only because I couldn't think of a good name for the enum. Any ideas?
mitz
Comment 4
2010-07-21 16:41:03 PDT
Comment on
attachment 62246
[details]
New patch r=me with some polish suggested in person
Beth Dakin
Comment 5
2010-07-21 16:54:05 PDT
Thanks Dan! Fixed with
http://trac.webkit.org/changeset/63864
James Robinson
Comment 6
2010-07-22 00:12:43 PDT
There are visible seams in many of the new expected results - for example the top left and bottom right of
http://trac.webkit.org/export/63877/trunk/LayoutTests/platform/mac/fast/borders/borderRadiusOutset01-expected.png
. It does look better than the previous behavior (which was
http://trac.webkit.org/export/63877/trunk/LayoutTests/platform/chromium-mac/fast/borders/borderRadiusOutset01-expected.png
) but still not very good. Is there another bug to track this?
Beth Dakin
Comment 7
2010-07-22 10:21:32 PDT
(In reply to
comment #6
)
> There are visible seams in many of the new expected results - for example the top left and bottom right of
http://trac.webkit.org/export/63877/trunk/LayoutTests/platform/mac/fast/borders/borderRadiusOutset01-expected.png
. It does look better than the previous behavior (which was
http://trac.webkit.org/export/63877/trunk/LayoutTests/platform/chromium-mac/fast/borders/borderRadiusOutset01-expected.png
) but still not very good. Is there another bug to track this?
Actually I don't seem any visible seems in the results for that test. Could you provide a screenshot demonstrating the problem that you see?
Beth Dakin
Comment 8
2010-07-22 10:22:51 PDT
Created
attachment 62312
[details]
Pixie screenshot Here's a pixie zoom-in of the lower right corner which you said has a visible seam. I don't see one.
James Robinson
Comment 9
2010-07-22 12:55:05 PDT
You're right, that looks perfectly smooth. I must have been seeing things after staring at too many pixel results. Sorry for the noise!
Beth Dakin
Comment 10
2010-07-22 12:57:45 PDT
No worries. I totally know the feeling :-)
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug