152671 - One scan line is "chopped off" when scrolling in something other than 96DPI mode

Status: RESOLVED WONTFIX

(Core Graveyard :: GFX, defect, P1)

Description

[Kevin McCluskey (gone)]

This bug is a spin off from bug 80530

When running in 120DPI mode one line of pixels is frequently "chopped off" when
scrolling. Forcing a repaint by placing another window over the top of the
browser fixes the problem. Sometimes the missing line of pixels is in the center
of a text string causes the text to appear odd.

Comment 1

[Kevin McCluskey (gone)]

Attachment: testcase that demonstrates the bug

Testcase that demostrates the bug. Set your display to 120DPI then Scroll the
test case up and down and you will notice missing lines in the text "Layout".

Comment 2

[Kevin McCluskey (gone)]

Attachment: Fix bug by aligning (top leading) on pixel boundary

Comment 3

[Kevin McCluskey (gone)]

The off by one pixel errors are usually the result of calculations within layout
which place frames at (1/2) pixel locations. This happens frequently in 120DPI
because the twips per pixel value is 12 which is evenly divisible by 2. This
results in twip values that fall exactly on 1/2 pixel boundaries as opposed to
96DPI with a twips per pixel value of 15 which is not evenly divisible by 2. 

The patch I attached fixes the test case in the bug by aligning the top leading
on a pixel boundary instead of keeping it at 1/2 pixel.  There are other places
within layout where calculations of the form (value / 2) are done which may also
cause off by one pixel errors. @see nsHTMLReflowState.cpp, nsBlockReflowContext.cpp.

Comment 4

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

Comment on attachment 88321 [details] [diff] [review]
Fix bug by aligning (top leading) on pixel boundary

I don't see how this change alone will help anything.  There are tons of things
that don't align on pixel boundaries -- for example, CSS units specified in pt,
etc., and fixing just the leading (which is just an offset relative to another
position) won't really help if the block is already on a half-pixel boundary. 
It will only make the layout a little less accurate (and might help in one case
that you're looking at, but hurt in a different one).  Is there a way we could
make the rounding consistent instead?

Comment 5

[Kevin McCluskey (gone)]

"I don't see how this change alone will help anything..." 

It was not my intent that this would fix all of the off by one pixel errors. It
was meant to demonstrate that many of the off by one pixel errors probably are
the result of frames being positioned at half pixel locations.  I was looking
for the source of why I see so many frames at exactly 1/2 pixel offsets in
120DPI and It seems to be because we frequently divide twip values by 2 which
later are used as the basis for other calculations which result in frames at
fractional pixel offsets. 

The fundamental problem is floating point numbers can not accurately represent
all real numbers. Currently, the holes in the floating point representation
result in errors that cause off by one pixel errors.

consider the following example:

You have a frame positioned at y=5 pixels and it's height=10.5 pixels 

If the value of 5 is represented by 4.99999999 and your length is  more
accurately represented as 10.50000000 due to the inability of floating point
numbers to represent the exact real number values

If you add 0.500000 and truncate you get 5 and 11.  (the length is 6 pixels)

If you scroll it up one pixel and the representation is

3.999999 and 9.49999999 then you add 0.50000 and round you get 4 and 9. (the
length is now 5 pixels!!)

You could fix this example by not rounding up by adding .5 but adding some other
 constant instead. The problem is the constant you choose for rounding up will
depend on the input to the transformation matrix which is dependent on the DPI
setting and the typical fractional pixel components layout is storing. 

The reason we don't see as many off by one pixel errors in 96DPI is that
rounding  up by 0.5 works well because we can not accurately store 1/2 pixel
offsets in twips because the twip to pixel conversion factor is 15 in 96DPI
which is not evenly divisible by 2. 

Comment 6

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

If the transform has forced us to floating point, can't we just and 0.4999
instead of 0.5 so that we always round in a consistent direction?

Comment 7

[Kevin McCluskey (gone)]

This will not fix the general length issue:

Let me modify the original example:

You have a frame positioned at y=5.00.. pixels and it's height=10.25 pixels 
due to the page author setting an explicit height through CSS. 

(y + height + roundup constant) trunc(5.0 + 10.25 + 0.499..) = 15 

The author sees that the height of the element rendered as 11 pixels high.

If the author subsequently moves the frame to y=5.3 pixels through CSS
positioning he gets:

(y + height + roundup constant) trunc(5.3 + 10.25 + 0.4999..) = 16

(the length is 12 pixels high).

You can also come up with examples where the the y coordinate is mapped to a
slightly larger fractional pixel value because of round off errors accumulated
as a result of translating the transform which will also cause the (y + height +
roundup constant) to be mapped to an adjacent pixel.

Comment 8

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

> If the author subsequently moves the frame to y=5.3 pixels

That will never happen since we ensure (in scrollframe code) that all scroll
events are numbers of twips that convert exactly to a number of pixels (and we
also ensure that p2t is always an integer).  See bug 16200.

Comment 9

[Kenneth Herron]

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

Comment 10

[Kenneth Herron]

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

Comment 11

[Kenneth Herron]

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

Comment 12

[Kenneth Herron]

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

Comment 13

[Kenneth Herron]

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

Comment 14

[Ali Ebrahim]

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

Comment 15

[Mike Connor [:mconnor]]

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

Comment 16

[Gábor Stefanik]

Isn't this fixed now with the unit fix landed? (We now use app units instead of twips, so most probably it is.)

Comment 17

[Gordon P. Hemsley [:GPHemsley]]

This bug has been buried in the graveyard and has not been updated in over 5 years. It is probably safe to assume that it will never be fixed, so resolving as WONTFIX.

[Mass-change filter: graveyard-wontfix-2014-09-24]