Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Wrong space between block level elements that have no margin

RESOLVED FIXED

Status

()

Core
Layout: Text
RESOLVED FIXED
15 years ago
12 years ago

People

(Reporter: masayuki, Assigned: roc)

Tracking

(Blocks: 1 bug)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

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

15 years ago
worksforme, linux build 2002-10-04-08....  This is probably an OS-dependent font
metrics issue...

Comment 2

15 years ago
The problem seems caused by accumulated calculation round-off difference. 
I could reproduce the bug. 
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

15 years ago
Priority: -- → P3
Target Milestone: --- → Future

Comment 3

14 years ago
.
Assignee: attinasi → block-and-inline
Component: Layout → Layout: Block & Inline
Priority: P3 → --
QA Contact: cpetersen0953 → ian
Target Milestone: Future → ---
Has this been seen on any non-Windows platforms?  I'm not sure why it's PC/All.
Depends on: 63336
Sorry, my mistake.
I change to All/All.

We could reproduce with...
Win98SE, WinMe, WinXP, MacOS9.2, FreeBSD.
Hardware: PC → All

Comment 6

14 years ago
*** Bug 218037 has been marked as a duplicate of this bug. ***

Comment 7

13 years ago
Created attachment 163919 [details] [diff] [review]
patch

The value of yTop and yBottom are adjusted to alignment of onePixel by trimming
the reminder divided with onePixel.

Updated

13 years ago
Component: Layout: Block and Inline → Accessibility APIs

Comment 8

13 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

13 years ago
Created attachment 164026 [details] [diff] [review]
patch
Attachment #163919 - Attachment is obsolete: true

Updated

13 years ago
Component: Accessibility APIs → Layout: Fonts and Text

Comment 10

13 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

13 years ago
Attachment #164026 - Flags: review?(dbaron)
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.
Attachment #164026 - Flags: review?(dbaron) → review-
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.
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.
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.
I can reproduce the problem in attachement 1238, and the layout is fine (blocks
are adjoining). So the problem is in Gfx.
looks like the bug is really in nsTransform2D.
Created attachment 178418 [details] [diff] [review]
fix

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: layout.block-and-inline → roc
Status: NEW → ASSIGNED
Attachment #178418 - Flags: superreview?(dbaron)
Attachment #178418 - Flags: review?(dbaron)
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.
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.
...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?
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 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+
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.
Blocks: 287774
checked in
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED

Updated

12 years ago
Blocks: 310761
You need to log in before you can comment on or make changes to this bug.