User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.6) Gecko/20060814 SeaMonkey/1.0.4 Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a2pre) Gecko/20061221 Minefield/3.0a2pre There can be gaps between an image and its border. I believe this is caused by NSToIntRound() and NSToCoordRound() rounding -x.5px down and +x.5px up instead of consistently rounding up or down (note that NSToCoordRound() is consistent if NS_COORD_IS_FLOAT). http://lxr.mozilla.org/seamonkey/source/xpcom/ds/nsUnitConversion.h#63 #define ROUND_CONST_FLOAT 0.5f inline nscoord NSToCoordRound(float aValue) { #ifdef NS_COORD_IS_FLOAT return float(floor(aValue + ROUND_CONST_FLOAT)); #else return ((0.0f <= aValue) ? nscoord(aValue + ROUND_CONST_FLOAT) : nscoord(aValue - ROUND_CONST_FLOAT)); #endif } inline PRInt32 NSToIntRound(float aValue) { return ((0.0f <= aValue) ? PRInt32(aValue + ROUND_CONST_FLOAT) : PRInt32(aValue - ROUND_CONST_FLOAT)); } Reproducible: Always Steps to Reproduce: 1. Load testcase Actual Results: Gaps. Expected Results: No gaps.

Are you reporting this in 1.8.0.6 or 1.9a2pre? It's a duplicate either way, just of different bugs.

I report this in 1.9a2pre, but the upper left icon has also gaps in 1.8.0.6. Thus, if bug 363220 is a recent regression, this one cannot be a duplicate. Anyways, the main point of this bug was to report a flaw in the code; the symptom was just an illustration.

I don't think that's a flaw any more than rounding at all is. NSToCoordRound might be used on coordinates, on offsets, or on sizes -- you can't really tell. nscoords themselves are not pixels, they're much smaller. In any case, rounding is complicated and messy, and you're going to have to point to actual symptoms if you want to convince me that that's wrong. If you're reporting two problems, one of which is in 1.8.0.6 and one of which is not, then you're reporting two separate bugs that belong in two separate bug reports.

That said, it's entirely possible that the rounding functions could be improved, but I don't think they're related to any of the things in that testcase.

The problem with the upper left icon does look like a problem with a rounding function that's similar, but it's a problem dealing with coordinates at a point when they've been transformed to widget-relative coordinates in pixels, which means things are already past all the layout and style code and a good bit of the graphics code. So the component of the bug report is definitely wrong. That said, I could believe that the problem you're pointing out could cause that -- probably due to the calls to NSToCoordRound in nsTransform2D. And I suspect it requires that p2t is an even number.

Testcase for only the problem covered by this bug; the rest is a duplicate.

To be more specific: The use of NSToIntRound() in http://lxr.mozilla.org/seamonkey/source/gfx/src/thebes/nsThebesRenderingContext.cpp#1004 probably causes the gaps in the upper left icon (I constructed the testcase after reading that code in bug 177805 and expected to see this bug). > If you're reporting two problems, one of which is in 1.8.0.6 and one of which > is not, then you're reporting two separate bugs that belong in two separate bug > reports. I tested in 1.8.0.6 only after you asked me. The gaps in the lower two icons may indeed be unrelated and be a duplicate of bug 363220, but I did not know this at the time I filed this bug. I simply tried out a few more cases with half pixel positions and found these.

Yeah, right, nsTransform2D might not be used on the trunk anymore. Yep, thanks for the report, and sorry I didn't believe you. (So many bug reports are invalid/duplicate/incomprehensible, I get in the habit of expecting them all to be.) Do you see a simpler way to write an always-round-halves-up rounding function than: if (f > 0.0f) return nscoord(f + 0.5f); nscoord ceil = nscoord(f); if (float(ceil) - f <= 0.5f) return ceil; return ceil - 1; ?

(I wonder if lrintf helps us; I don't know what the man page means by "current rounding direction".)

"current rounding direction" is defined by fesetround() (see man fenv), but fesetround(FE_UPWARD) rounds everything up, not just halfway cases. FE_TONEAREST (the default) is actually "round to even", which will cause even more problems. A simpler way to write an always-round-halves-up rounding function would be using floor() (or better floorf()) for the integer case too, but your code is likely to be faster (the first if should have a >= instead of >).

The testcase was fixed by the patch to bug 369882, although there may still be some other testcases showing the problem.

Having this wrong is causing bad widget coordinate calculations in bug 382595.

(In reply to comment #14) > Created an attachment (id=270339) [details] > Patch (at least partially) - return nscoord(NS_roundf(aValue)); + return nscoord(NS_floorf(aValue + 0.5f)); Is aValue guaranteed to be positive here? This will break on negative numbers.

(In reply to comment #15) > (In reply to comment #14) > > Created an attachment (id=270339) [details] [details] > > Patch (at least partially) > > - return nscoord(NS_roundf(aValue)); > + return nscoord(NS_floorf(aValue + 0.5f)); > > Is aValue guaranteed to be positive here? This will break on negative numbers. > Ignore this comment. I forgot how the floor function worked. This looks OK to me now.

Checked in. (Widget problems are not reftestable.)