Closed
Bug 173051
Opened 22 years ago
Closed 20 years ago
Wrong space between block level elements that have no margin
Categories
(Core :: Layout: Text and Fonts, defect)
Core
Layout: Text and Fonts
Tracking
()
RESOLVED
FIXED
People
(Reporter: masayuki, Assigned: roc)
References
(Blocks 1 open bug)
Details
Attachments
(2 files, 1 obsolete file)
(deleted),
patch
|
dbaron
:
review-
|
Details | Diff | Splinter Review |
(deleted),
patch
|
dbaron
:
review+
dbaron
:
superreview+
|
Details | Diff | Splinter Review |
If some block level elements have no margin,the elements should be connected.
But the following testcase are not always visible so.
* testcase of standards mode.
http://bugzilla.mozilla.gr.jp/showattachment.cgi?attach_id=1236
* testcase of quirks mode.
http://bugzilla.mozilla.gr.jp/showattachment.cgi?attach_id=1237
* the paragraph has "height:1em;" with testcase of standards mode(1236).
http://bugzilla.mozilla.gr.jp/showattachment.cgi?attach_id=1238
I can see space of 1px between the paragraphs.
If you can't see the space, you may use "TextZoom".
This problem seems to be influenced by the font-size.
#screenshot
http://bugzilla.mozilla.gr.jp/showattachment.cgi?attach_id=1239
2002100204/Win-ME/attachment-1236/100%
This problem is reproduced....
2002100608-trunk/WinXP.................all cases
2002091808-trunk/Win98SE...............all cases
2002100308-trunk/MacOS 9.2.............1238 only
2002100208-trunk/FreeBSD...............1238 only
This problem comes Bugzilla-jp.
http://bugzilla.mozilla.gr.jp/show_bug.cgi?id=2718
Comment 1•22 years ago
|
||
worksforme, linux build 2002-10-04-08.... This is probably an OS-dependent font
metrics issue...
Comment 2•22 years ago
|
||
The problem seems caused by accumulated calculation round-off difference.
I could reproduce the bug.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Updated•22 years ago
|
Priority: -- → P3
Target Milestone: --- → Future
Comment 3•22 years ago
|
||
.
Assignee: attinasi → block-and-inline
Component: Layout → Layout: Block & Inline
Priority: P3 → --
QA Contact: cpetersen0953 → ian
Target Milestone: Future → ---
Comment 4•22 years ago
|
||
Has this been seen on any non-Windows platforms? I'm not sure why it's PC/All.
Depends on: 63336
Reporter | ||
Comment 5•22 years ago
|
||
Sorry, my mistake.
I change to All/All.
We could reproduce with...
Win98SE, WinMe, WinXP, MacOS9.2, FreeBSD.
Hardware: PC → All
Comment 6•21 years ago
|
||
*** Bug 218037 has been marked as a duplicate of this bug. ***
Comment 7•20 years ago
|
||
The value of yTop and yBottom are adjusted to alignment of onePixel by trimming
the reminder divided with onePixel.
Updated•20 years ago
|
Component: Layout: Block and Inline → Accessibility APIs
Comment 8•20 years ago
|
||
The following testcase using textzoom(for example 90%) indicates another problem
that is appearance of black lines.
> * the paragraph has "height:1em;" with testcase of standards mode(1236).
> http://bugzilla.mozilla.gr.jp/showattachment.cgi?attach_id=1238
The font size of nsStyleFont of nsTextFrame::Reflow is wrong, since the font
size of 90% is not aligned by onePixel. For exmanple, if default font size of
mDefaultSansSerifFont on PresContext structure is 192 twips, the size of 90% is
172 twips, in result (if one pixel is 12 twips) the remainder is 4 twips.
please refer to following code of content/base/src/nsRuleNode.cpp.
The value of fontData->mFont.size is wrong, since the font size has remainder.
nsRuleNode::SetDefaultOnRoot(const nsStyleStructID aSID, nsStyleContext* aContext)
{
switch (aSID) {
case eStyleStruct_Font:
{
nsStyleFont* fontData = new (mPresContext) nsStyleFont(mPresContext);
nscoord minimumFontSize =
mPresContext->GetCachedIntPref(kPresContext_MinimumFontSize);
if (minimumFontSize > 0 && !IsChrome(mPresContext)) {
fontData->mFont.size = PR_MAX(fontData->mSize, minimumFontSize);
}
else {
fontData->mFont.size = fontData->mSize;
}
aContext->SetStyle(eStyleStruct_Font, fontData);
return fontData;
Comment 9•20 years ago
|
||
Attachment #163919 -
Attachment is obsolete: true
Updated•20 years ago
|
Component: Accessibility APIs → Layout: Fonts and Text
Comment 10•20 years ago
|
||
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b) Gecko/20050205
Firefox/1.0+
Probably still in.
The third testcase using text zoom has the defect.
Updated•20 years ago
|
Attachment #164026 -
Flags: review?(dbaron)
Comment 11•20 years ago
|
||
roc could correct me on this, but I think we've had discussions about whether we
want to do this like the pixel rounding that attachment 164026 [details] [diff] [review] does, and I think
we've decided that we don't want to do this. And if we do, I think this is the
wrong approach -- we shouldn't be changing things in just one or two spots.
Updated•20 years ago
|
Attachment #164026 -
Flags: review?(dbaron) → review-
Assignee | ||
Comment 12•20 years ago
|
||
Well, it's a bit complicated.
On non-antialiasing devices it's fine to just ensure that the blocks are
contiguous in layout coordinate space. Then we can make sure that the gfx layer
rounds correctly so that it sees every pixel is covered by some block. That was
what I used to think was the solution.
But on antialiasing devices --- like Cairo will be --- it's harder because the
natural thing to do with coverage-based antialiasing would make some pixel lines
end up 75% white or something (50% white on 50% white on black).
I guess we need to talk to the Cairo guys to see how they think apps should
solve such issues in general.
Assignee | ||
Comment 13•20 years ago
|
||
There's a really good thread on the Cairo list about this *exact* issue.
http://lists.freedesktop.org/archives/cairo/2005-March/003342.html
My understanding from the thread is that coverage-based antialiasing will simply
not work for us unless we work really hard to identify all rendered rectangle
edges that might possible adjoin other rendered rectangle edges. That seems
unrealistic. Instead we should simply turn off Cairo's antialiasing. When the
resources are available, we can use supersampling --- i.e., render into a canvas
that's 4X x 4X the actual size, and then shrink it down, averaging pixel values.
Assignee | ||
Comment 14•20 years ago
|
||
In the meantime, we should fix this bug by ensuring that a) layout makes the
actual block boxes adjoining and b) Gfx rounds correctly so that a pixel always
gets the color of the box its top-left corner is in.
Assignee | ||
Comment 15•20 years ago
|
||
I can reproduce the problem in attachement 1238, and the layout is fine (blocks
are adjoining). So the problem is in Gfx.
Assignee | ||
Comment 16•20 years ago
|
||
looks like the bug is really in nsTransform2D.
Assignee | ||
Comment 17•20 years ago
|
||
This is the logical fix ... just always transform points, and transform them
consistently. It fixes this bug and all the testcases referenced in
nsTransform2D bugs still seem to work. I don't know what dcone's logic was
supposed to achieve.
By "transforming points consistently" I mean that a transformed coordinate is
always rounded to the nearest integer. This has the effect that whenever we
paint, a pixel should be filled with the color that would be painted at the
pixel's center. E.g. when filling the rectangle x=0.3, y=0.3, w=0.4, h=0.4, it
contains the point (0.5, 0.5) so we fill the pixel at (0, 0). (The logic works
out like this: the rectangle is defined by (0.3, 0.3), (0.7, 0.7), which rounds
to (0, 0), (1, 1).)
Assignee | ||
Updated•20 years ago
|
Assignee: layout.block-and-inline → roc
Status: NEW → ASSIGNED
Attachment #178418 -
Flags: superreview?(dbaron)
Attachment #178418 -
Flags: review?(dbaron)
Comment 18•20 years ago
|
||
Perhaps dcone's logic was to achieve consistency of border widths or something
like that? (I haven't looked at it closely yet. I'll try to do so later
tonight or tomorrow.) That said, maybe we should be rounding border widths to
the nearest pixel in the style system for exactly that reason.
Assignee | ||
Comment 19•20 years ago
|
||
Yeah, with this approach, fractional coordinates can make rectangles with
nominally the same width be rendered with different widths. I don't think
there's any way to fix that that avoids gaps. I'd rather avoid gaps.
Comment 20•20 years ago
|
||
...and would you object to fixing what's probably the most obvious of those
problems by rounding border widths to the nearest pixel in the style system?
Assignee | ||
Comment 21•20 years ago
|
||
Rounding things in the style system is OK by me providing
-- people don't yell at us for being incorrect
-- it doesn't break real pages
-- we actually round to *device* pixels so that you can still get nice layouts
on high-res devices
Comment 22•20 years ago
|
||
Comment on attachment 178418 [details] [diff] [review]
fix
Actually, I suspect this was an attempt to solve the fact that there were
problems when we scrolled by something that wasn't an exact number of pixels
(and then repainted some things over what was blitted). The solution to that
problem (bug 16200) was to always scroll by exactly an integer number of pixels
so the bits blitted would correspond to what was repainted.
Attachment #178418 -
Flags: superreview?(dbaron)
Attachment #178418 -
Flags: superreview+
Attachment #178418 -
Flags: review?(dbaron)
Attachment #178418 -
Flags: review+
Comment 23•20 years ago
|
||
FWIW, I filed bug 287624 on the borders thing -- although it's worth noting that
I was misunderstanding things, and it really has no relation to what's being
fixed here.
Assignee | ||
Comment 24•20 years ago
|
||
checked in
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•