Open Bug 28563 Opened 25 years ago Updated 2 years ago

rounded borders are not symmetrical, and not exact enough [borders][gfx][curves]

Categories

(Core :: Layout, defect)

defect

Tracking

()

People

(Reporter: sjoerd, Unassigned)

References

Details

(Keywords: css-moz, helpwanted, testcase)

Attachments

(4 files)

From Bug Helper: User-Agent: Mozilla/4.0 (compatible; MSIE 5.01; Windows 98) BuildID: 2000021908 It looks like the corners on the left look like the corners on the right with 1px smaller radius Reproducible: Always Steps to Reproduce: try attachment
There seems to be code that makes boxes round when the radius>=50% of the box's size. But because the rounded corners are not drawn right, a 6px radius on a 12px box is rounder than a 6px radius on a >12px box. So we can exactly define how a rounded corner should look like, by looking at the circle with that radius: 0-1.5 | 1.5-3 | 3-4.5 | 4.5-6? | | | | XX | XXX | XXX XX | XXXX | XXXXX | XXXXX XX | XXXX | XXXXXXX | XXXXX | XX | XXXXXXX | XXXXXX I don't have a good drawing program here to check this, but it must be something like this.
Summary: rounded borders are not symmetrical → rounded borders are not symmetrical, and not exact enough
You wrote: "a 6px radius on a 12px box is rounder than a 6px radius on a >12px box" but the testcase seems to show instead: "a 6px radius on a 12px box is rounder than a >6px radius on a 12px box" I agree that the 4 corners are not rendered symetrically. It's very clear under a "magnifier glass" when looking at the 5/6/7/8px circles with a 1px wide border. I understand now why our radio-buttons don't look great. Reassigned to dcone and CCd german to decide whether this should make it for beta1.
Assignee: pierre → dcone
The code to draw these rounded borders is perfect.. to see this print these out. Why it looks bad on the screen is because we calculate the rounded code in twips.. then convert to pixels, and for the screen this is much lower resolution. The output of the graphics.. the translations by twips are not always on pixel boundaries.. and they get thrown off.. so one side may look different than another if the correct pixel boundaries are not hit, and that is what is happening. To get this to look correct.. all calculations have to start and end up on pixel boundaries and that is not always happening, and for small radii these offsets (by 7 twips or 1/2 pixel) can end up making things not perfect. The factors that can screw this up are the translation (offset of the graphic from the top corner), scale (twip to pixel conversion) etc. If you go to a higher resolution device.. things will look great. This sort of problem also was evident in table borders, double lines etc, but I fixed that in the translation matrix. Solution is to make sure everything is calculated on pixel boundaries.. for the current device context. Also the subdivision has to occur on pixel boundaries (for the algorithm..)
Status: NEW → ASSIGNED
Attached file some more test (deleted) —
It doesn't look good on my printer either, but the difference are very small, and are different from the screen. It could just be my printer. Some people really like the look of the new widgets, so is it possible that there are platform differences? I've no linux or mac available to test that. I've heard that a 3px circle at (5,5) is different between Windows, Linux and Mac, but I'm not sure. This is really helpfull, isn't it? ;)
I just did a printout of your tests on a 600 dpi printer.. and it looks great. Either you have a great eye or a low res printer. Remember that a twip 1/15th of a pixel.. so even a 600 dpi printer is a lower resolution than a twip. Anyway.. I can fix this.. but it will not be for beta1.
Attached file Crash! (deleted) —
I have a 600dpi bubble jet printer, but it doesn't draw straight line straight either, so forget about that. I'm sure there's no code out there that changes the radius for different corners. There's another little bug, see attachment. It craches Mozilla. It happens when the border and the -moz-border-radius are equal and the width and height are zero. (I tried to draw a funny circle using a grooved border) I think it's a devide by zero or something. The code tries to draw an inner circle with a radius of zero. It's an easy fix I think. Could you post the pointer to the code? I can try to find out some sort of algorithm for this problem. (if you want/need help or more time, that is)
mozilla/layout/style/nsCSSRendering.cpp is the file. Look for all the curve stuff.. there is PaintRoundedBorder I think is the call. I think that last bug is a divide by zero.. that will be easy to fix.
oh boy... Talking about a steep learning curve... ;-)
Target Milestone: M15
*** Bug 30307 has been marked as a duplicate of this bug. ***
Just a little hint, for whoever fixes this: When rounding the position of pixels, err towards rounding outwards (towards a square shape) rather than inwards (towards a diamond shape). Pixelated circles tend to look better that way. (Trust me, I used to design low-resolution bitmap fonts on an Amiga ...)
Read the previous comments.. I use a quadratic curve and sub-division to draw these.. they draw perfect.. so there is no calculating of the position of individual pixels. The issue is that on a lower resolution device.. the rounding of the values returned from the transformation matrix may not be on pixel boundaries. The solution is to make sure that the rectangle passed in -- is on a pixel boundary for the device that is being drawn on. High resolution devices will have a higher chance of being on the correct location, lower devices will need more adjustments. Just a little hint, for whoever fixes this: When rounding the position of pixels, err towards rounding outwards (towards a square shape) rather than inwards (towards a diamond shape). Pixelated circles tend to look better that way. (Trust me, I used to design low-resolution bitmap fonts on an Amiga ...)
The probelm is also evident in the current appearance of XUL titledbuttons. If you look at global.css you can see we used consistent definitions for the corner radii (3px if I remember correctly), yet they look very uneven. At first we thought we could work around this by assigning different values to the corners to make it *look* right, but we discovered quickly that results varied based on platform, screen resolution and position of the widget. I do much appreciate the hard work don and team have done to bring us this great feature. As a lay person on this subject I wonder though what prevents from hard coding a table of pixel values for select low value radii on a screen in order to achieve great on-screen appearance as suggested by sjoerd!
Blocks: 28883
The problems is we calculate everything in twips.. and we have a translation matrix... which makes things act flaky currently, even if it is hard coded. For example if you set two pixels.. depending on the translation matrix (scale and translation), they could be two different pixels or the same pixel. I have to fix the translation matrix first.. then I think the curve stuff will work.. and if it does not.. then we know that the hard coded version will work.
Target Milestone: M15 → M17
This bug has been marked future because we have determined that it is not critical for netscape 6.0. If you feel this is an error, or if it blocks your work in some way -- please attach your concern to the bug for reconsideration.
Target Milestone: M17 → Future
I'll see what I can do when I get back.
Assignee: dcone → BlakeR1234
Status: ASSIGNED → NEW
The only way to fix this bug is to render the curves in pixels instead of twips.. and there is code to do this (RenderingContexts now have curve rendering). I think this is to big a risk to do this so close to shipping though.. that is why it was moved to future.
And it probably won't be fixed. But I don't see any harm in reassigning it to me since it would have lay dormant if future-assigned to you...
Status: NEW → ASSIGNED
Too bad, because if, as I understand correctly, this bug is related to the bizzare, pixellated, asymmetrical (ok, let's just say it: UGLY) radio buttons, fixing the aesthetics will go a long way towards making Mac users accept the non- native widgets...
Feel free to submit a patch.
The code is already in the RenderingContexts for curve rendering in pixels.. and it does seem to fix the problem. The work that remains is to get nsCSSRendering.cpp to call this code instead of calling its curveRendering code which does this in twips. There are a few details that have to be looked into for things like dashed lines.. but on my initial tests the radio buttons looked great. I planned on doing after all printing bugs (high priority bugs) were fixed or for the next realease.. The PDT team did not think this was one that needed to be fixed. Of course outside help (with lots of testing) would get this fixed sooner.
Priority: P3 → P4
Another test is the focus rings for text widgets in Mac Classic: on the left end they are (correctly) rounded by one pixel, but on the right end they are completely square.
No time right now.
Assignee: blakeross → dcone
Status: ASSIGNED → NEW
Priority: P4 → P3
Target Milestone: Future → ---
Status: NEW → ASSIGNED
Netscape's standard compliance QA team reorganised itself once again, so taking remaining non-tables style bugs. Sorry about the spam. I tried to get this done directly at the database level, but apparently that is "not easy because of the shadow db", "plus it screws up the audit trail", so no can do...
QA Contact: chrisd → ian
Summary: rounded borders are not symmetrical, and not exact enough → rounded borders are not symmetrical, and not exact enough [borders][gfx][curves]
Target Milestone: --- → Future
Just wondering if (and hoping that) this bug was going to receive any more attention-- I haven't seen any activity since February (or keywords or status, etc), and I do think this is related to the "fit and finish" aspects of the 1.0 release. Just thought maybe i could get this one back on the radar ;) Looks like the solution was close at hand at one point too. Thanks.
*** Bug 119482 has been marked as a duplicate of this bug. ***
Over to layout, I guess.... Or is nsCSSRendering gfx?
Assignee: dcone → other
Status: ASSIGNED → NEW
Component: Style System → Layout
Priority: P3 → --
Target Milestone: Future → ---
I think at this point: * we've rewritten border-drawing code for Gecko 1.9 * we *should* be pixel-snapping the destination areas before the drawing We should probably add tests for the latter (i.e., make sure that something with rounded borders stays symmetric as it shifts by fractional pixels by drawing it into a canvas and checking that it equals its reflection). The tests don't work anymore because the native-theme drawing is winning.
Flags: in-testsuite?
Attached file "some more tests" updated (deleted) —
Assignee: layout → nobody
QA Contact: ian → layout
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: