141715 - -moz-border-radius should round not bevel -moz-border-colors

Status: RESOLVED FIXED

(Core Graveyard :: GFX, enhancement)

Description

[[e-mail]]

-moz-border-radius rounds borders, except if you use -moz-border-colors, in
which case it bevels them :-(

Comment 1

[[e-mail]]

Attachment: Test Case

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This is as-designed.  -moz-border-colors was designed (by hyatt) to be used for
UI buttons, where the maximum border radius used is about 3px.  Making a
diagonal edge is sufficient for that radius and allows near-perfect imitation of
the color effects in real UI buttons.

Comment 3

[Adam Fletcher]

I vote that this gets re-opened. It is a shame that such a useful feature
(-moz-border-colors) not be useable in cases where I want -moz-border-radius. 

For instance, I am trying to round the borders of large tabs, and the borders
are multiple pixels wide. It would suck to have to figure out how to surround my
tabs in multiple boxes so I can use -moz-border-radius. 

Another example, in our app my company has a number of rounded boxes that, in
our old release (based on 0.9.1), I had to use 3 boxes with 3 sets of
-moz-border-radius's to make a 3 pixel, gradiented, rounded border.
-moz-border-radius + -moz-border-colors would save me that work and look a lot
better. 

If this is by design, can someone point me to the source and I will try and
patch it myself?

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

It's in nsCSSRendering.cpp.  If you can fix it without:
 * slowing drawing down measurably, or
 * messing up the accuracy of the cases used in the UI
then the patch would be worth considering.  However, I suspect that's nearly
impossible to do.  I could be wrong, though.

Comment 5

[Boris Zbarsky [:bzbarsky]]

*** Bug 200240 has been marked as a duplicate of this bug. ***

Comment 6

[Alfred Kayser]

See bug 200240 for a way out:
If you define to boxes in each other with 1 pixel border (i.e. non-composite),
you can simulate the desired effect.

To fix the composite border with radius can thus be done by 
repeating PaintRoundedBorder for each composite border color, decrementing the
size by 1 for each color. This is probably the fastest way to do (beside special
hardware or OS calls).

Look at nsCSSRendering.cpp:
1650   // rounded version of the outline
1651   // check for any corner that is rounded
1652   for(i=0;i<4;i++){
1653     if(borderRadii[i] > 0 && !aBorderStyle.mBorderColors){
1654      
PaintRoundedBorder(aPresContext,aRenderingContext,aForFrame,aDirtyRect,aBorderArea,&aBorderStyle,nsnull,aStyleContext,aSkipSides,borderRadii,aGap,PR_FALSE);
1655       return;
1656     }
1657   }
1658 

Note, constructing boxes inside boxes (using XBL or XUL) is very slow, and was
the reason for existance of -moz-border-*-colors. Using the simple trick above,
it can then also be used for rounded borders.

Hoping that someone is willing to take the challenge, greetings Alfred

Comment 7

[Alfred Kayser]

Can this bug be re-opened, and analyzed whether it is usefull
to include it in some release? The fix seems easy, while delivering
the nice round 'input' boxes, everybody now likes so much (see the new
themes for Phoenix). Right now, they are using fixed image-chunks, but
that's is not scaleable (for big fonts).

Comment 8

[[e-mail]]

Re-opening as an enhancement request.

Comment 9

[Alfred Kayser]

Attachment: A tentative patch.

This patch adds PaintCompositeRounderBorder functionality.
It should work. It does compile, but I have to rebuild
a whole tree, before I can test it.
As one can see from the patch, it only adds code, for the
specific Composite and Rounder border case, so it should not
impact the other code in any way.

Comment 10

[Alfred Kayser]

Attachment: Working patch

With this patch the test examples from this bug, and from
bug 200240 work correctly.

However, in the Modern theme it does create some undesired
effects in the rounded buttons, and the tabs.
But, it seems to be easily fixable.

Comment 11

[Alfred Kayser]

Attachment: Patch from mozilla\layout\html\style\src

Final patch, for review and further testing

Comment 12

[Alfred Kayser]

Attachment: Screenshot showing before and after situation

Comment 13

[Alfred Kayser]

Attachment: Patch to themes\modern

This additional patch increases some radius values for the Modern
theme, to make it play nicer with the improve CompositeRounderBorder patch.
Note: the 'after' screenshots were taken with this patch also applied

Comment 14

[Boris Zbarsky [:bzbarsky]]

I'll try to get to this within the next two weeks, but that's about all I can
promise right now.

Comment 15

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 121662 [details] [diff] [review]
Patch from mozilla\layout\html\style\src

I'd think if you wanted to do this (and I'm not even convinced of the need for
the patch in the first place, so it's probably not a good idea to put too much
work into that until we agree on the need for it):
 * you'd want to remove the current code that draws the beveled borders
 * you'd want to avoid copying code (was some of the new code here copied from
elsewhere?), but instead refactor existing code.

Comment 16

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 121662 [details] [diff] [review]
Patch from mozilla\layout\html\style\src

Please ask me for sr once the reviewer (dbaron) is happy with the patch.

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 121662 [details] [diff] [review]
Patch from mozilla\layout\html\style\src

Marking review- based on my previous comment, although feel free to re-request
if you think I was wrong, since I didn't look too closely.

Comment 18

[Alfred Kayser]

I agree with the review-, the code is not finished yet.
Just wanted to have some look at it from the experts.
The following things need to be done:
* Assess importance of this feature (only 2 votes, and 3 CC'ers)
* Figure out, how to prevent holes 
* Remove redundancy
  * Remove or 'bevel composite radius' or make it a new style?
  * Refactor code to reduce code
* More testing, also on other platforms

So, as soon I've got some sparetime to waste, and I
want it to waste it on this feature, I will have another
go it. So no promises yet.

Comment 19

[Hixie (not reading bugmail)]

note: please don't attach stuff as .zip unless the files are too large to attach
to bugzilla. (And if screenshots are too big, you should reencode them using a
more optimal file format!) Thanks.

Comment 20

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Fixed by patch for bug 368247.

Comment 21

[Alfred Kayser]

Let me re-open this bug, after the patch for bug 368247.

While the result is much better, running the test case (attachment #1 [details] [diff] [review]) still clearly shows a big difference between normal border and -moz-border-colors rounding...  The difference is so clear, that one can easily conclude that for -moz-border-colors radius drawing, the radius is two times as big as it should be.

See also bug 380601, bug 379505, bug 379436 and attachment 7 [details] (testcase 2).
Making therefor this bug blocking the border rewrite bug 368247.

Testcase 3 shows all border radius issues between background, border and border-colors drawing

Comment 22

[Alfred Kayser]

Attachment: Testcase 2: show problems between normal border radius and -moz-border-* radius

Comment 23

[Alfred Kayser]

Bug 379474 as another testcase showing the radius calculation problem.

Comment 24

[Alfred Kayser]

Attachment: Testcase 3: Even more border rounding

Comment 25

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Fixed by checked-in patch from bug 368247.

Comment 26

[Gábor Stefanik]

Well, not exactly, the background is still strange, but that's bug 382613. It will get fixed soon.