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

1 pixel position errors..

VERIFIED DUPLICATE of bug 63336

Status

Core Graveyard
GFX
P3
normal
VERIFIED DUPLICATE of bug 63336
18 years ago
9 years ago

People

(Reporter: dbaron, Assigned: dcone (gone))

Tracking

({verifyme})

Trunk
verifyme

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: XP part of fix attached. Still needs simple gfx change on Win and Mac.)

Attachments

(9 attachments)

(Reporter)

Description

18 years ago
DESCRIPTION:  There seem to be some 1 pixel positioning errors when painting or
when moving pieces of the screen around (i.e., scrolling).  These can cause 2
effects:
 * jitter when mousing over a link causes a repaint
 * slightly stretched (one line appears twice) text when scrolling

I will attach a screenshot and steps to reproduce the first.  I can't reliably
reproduce the second but I may attach a screenshot at some point in the future.

STEPS TO REPRODUCE:
 * Load http://www.fas.harvard.edu/~dbaron/
 * Scroll down one or two lines using the arrow keys (I think the errors may be
in the opposite direction when using the mouse - I'm not sure)
 * Hover over one of the links near the bottom of the screen

ACTUAL RESULTS:
 * Link moves up or down by 1 pixel  (see attached screenshot, where the link
currently being hovered over - the highlighted one - is one pixel lower than the
link next to it)

EXPECTED RESULTS:
 * Link should repaint at exact same position

DOES NOT WORK CORRECTLY ON:
 * Linux, apprunner, 1999-10-11-08-M11
 * Linux, apprunner, 1999-09-19
 * Linux, apprunner, 1999-09-14

UNABLE TO REPLICATE ON (note that the fonts appear smaller in all three of
these, so it wasn't necessarily working correctly, but I may just have been
unable to replicate it using the techniques mentioned):
 * Linux, apprunner, 1999-08-30
 * Linux, apprunner, 1999-09-05
 * Windows, apprunner, M10

ADDITIONAL INFORMATION:
 * in the screenshot, the 1px positioning error is not very bothersome - I don't
notice it, but I do notice the movement to get there, and it gives the browser
an WinMSIE-like feel.  The errors with stretched text are probably a bit more
serious since they can make text difficult to read, and are likely the same
problem.
(Reporter)

Comment 1

18 years ago
Created attachment 2109 [details]
screenshot of hovered link moved down by 1 pixel
(Reporter)

Comment 2

18 years ago
I will attach a screenshot showing the second problem.  The problem is in the
line "flashes that illuminate the motion", exactly at the point that was at the
bottom of the scrolling area when the page loaded.  I loaded the page and then
hit the down arrow a few times.  I can also get it to happen by causing the
window to fully repaint (by switching virtual screens and back again) and then
scrolling with the down arrow; I can also trigger it at the top of the screen by
doing the same and scrolling up.

So basically, I seem to be able to reproduce this *somewhat* reliably by
positioning text in the middle of the edge of the screen, causing a full
repaint, and then scrolling.
(Reporter)

Comment 3

18 years ago
Created attachment 2110 [details]
screenshot of duplication of 1 line of pixels
(Reporter)

Comment 4

18 years ago
BTW, I am unable to replicate the 2d problem when using the mouse to scroll, but
I can replicate the 1st problem.  However, the errors seem to be in the same
direction - parts of the text are too high - in this next (and hopefully last)
screenshot you can see that the upper part of the line "W3C, TR, Lists,
Archives" in the lower right is stretched so the characters are too tall - look
particularly at the number 3.  When I hover, the top parts of the letters move
down to where they should be.
(Reporter)

Comment 5

18 years ago
Created attachment 2111 [details]
screenshot of stretched text that is corrected on hover
(Reporter)

Comment 6

18 years ago
Another bug that seems related:  If I load
http://www.cira.colostate.edu/Special/CurrWx/g8full4.htm and then scroll to the
bottom with the arrow, and keep holding the arrow, mozilla will (depending on
the size of the window) either:

 * scroll the page up one pixel at a time, smudging what was the bottom row
gradually across the whole page.

 * scroll the page down one pixel at a time, doing the reverse smudging.

Screenshots to be attached.

Updated

18 years ago
Status: NEW → ASSIGNED
(Reporter)

Comment 7

18 years ago
The second problem is not fixed in the 1999-10-23-08 build.  I expected it might
have been because of dcone's checkin.  I haven't tried the first yet.
(Reporter)

Comment 8

18 years ago
The first isn't fixed either.
(Reporter)

Comment 9

18 years ago
The problems here may be caused by the fact that I set mozilla to my real screen
resolution, 91dpi, using the line:

user_pref("browser.screen_resolution", 91);

in prefs.js.

I think the problem could be that p2t is not always rounded.  Here are some
sample startup lines when fiddling with this pref:

GFX: dpi=60 t2p=0.0416667 p2t=24 depth=24                  N
GFX: dpi=72 t2p=0.05 p2t=20 depth=24                       N
GFX: dpi=75 t2p=0.0520833 p2t=19.2 depth=24                Y!!
GFX: dpi=91 t2p=0.0631944 p2t=15.8242 depth=24             Y
GFX: dpi=96 t2p=0.0666667 p2t=15 depth=24                  N
GFX: dpi=100 t2p=0.0666667 p2t=15 depth=24                 N
GFX: dpi=120 t2p=0.0666667 p2t=15 depth=24      (why???)
GFX: dpi=144 t2p=0.1 p2t=10 depth=24
GFX: dpi=150 t2p=0.104167 p2t=9.6 depth=24                 Y

(The marginal Y or N is whether I could see the bug described first in this
report.)

Perhaps p2t should always be rounded to an integer?
(Reporter)

Comment 10

18 years ago
Created attachment 2508 [details] [diff] [review]
proposed patch
(Reporter)

Comment 11

18 years ago
I think this patch should make the "undocumented feature" of the
browser.screen_resolution pref work as well as things do with the default
(hopefully).
(Reporter)

Comment 12

18 years ago
I expect this bug could also be a problem on Windows, where some video drivers
allow you to change to something other than "Small Fonts" (96dpi) or "Large
Fonts" (120dpi).  It could also be a problem on xlib, etc., and should be
checked...

Updated

18 years ago
Assignee: beard → pavlov
Status: ASSIGNED → NEW

Updated

18 years ago
Status: NEW → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 13

18 years ago
looks good.  checked in patch.
(Reporter)

Comment 14

18 years ago
This bug could be a problem on other platforms... in particular, see:

http://lxr.mozilla.org/seamonkey/source/gfx/src/windows/nsDeviceContextWin.cpp#100
http://lxr.mozilla.org/seamonkey/source/gfx/src/mac/nsDeviceContextMac.cpp#66
http://lxr.mozilla.org/seamonkey/source/gfx/src/xlib/nsDeviceContextXlib.cpp#129
http://lxr.mozilla.org/seamonkey/source/gfx/src/photon/nsDeviceContextPh.cpp#163
http://lxr.mozilla.org/seamonkey/source/gfx/src/motif/nsDeviceContextMotif.cpp#104

Platform notes:
Win and Mac - get info from OS, therefore bug might show if weird setting in OS
 (can only be done with some video drivers on Win; may not be possible at all
 on Mac - I'm not sure)
Photon - uses browser.screen_resolution hack, otherwise hardcoded to 96.  Same
 fix could probably be useful there
XLib - works just like GTK used to.  Fix could probably be useful there too.

However, it's not clear it's necessary.  If one can't trigger the bug on XLib
and Photon by setting browser.screen_resolution to 91, or on Windows by using a
weird logical resolution, then perhaps there's a bug elsewhere that setting p2t
to an integer is covering up (well, that's probably the case anyway, but it
could be GTK-specific).
(Reporter)

Updated

18 years ago
Status: RESOLVED → REOPENED
OS: Linux → All
Hardware: PC → All
(Reporter)

Updated

18 years ago
Resolution: FIXED → ---
(Reporter)

Comment 15

18 years ago
I observed shifting problems on Windows mozilla 1999-10-30-09-M11 when I set my
resolution to 91dpi.  They were slightly different from the ones I saw on
Linux, probably because repaints happen differently.  I'm reopening the bug and
marking it All/All.  Since I expect finding the cause of all these problems
would be difficult, it would probably be best to force the p2t on all platforms
to be an integer (which is *very* easy to do :-).  See comments above...

To set your Windows logical resolution to 91dpi, do the following:
 * Start Menu | Settings | Control Panel
 * choose "Display"
 * choose "Settings" tab
 * in the "Font Size" select, choose "Other..."
 * A dialog box comes up, where you can drag a ruler.  Drag the ruler until it
says "91 pixels per inch".  (Well, you have to stop dragging to get what it
says to change, so it's a pain...)

This may only be possible with some video drivers.
(Reporter)

Comment 16

18 years ago
I was thinking about this a bit, and there's a theoretical reason why this
should happen if p2t is not an integer.  The cause is the use of movement of
already-painted sections of screen.  If you move a bit of screen, that
transforms the coordinate system by a round number of pixels.  However, if p2t
isn't an integer, this isn't a round number of twips.  This means that when you
transform the coordinate system, and then do the rounding over again, you're off
by a bit.

The default scrolling increment is 240 twips
(see http://lxr.mozilla.org/seamonkey/source/view/src/nsScrollingView.cpp#360 ),
which at my 91dpi (where p2t = 15.8242) converts to 15 1/6 px.  When this is
rounded to 15px, it converts back to 237.3 twips.  This means (if you transform
the coordinate system by the requested 240 twips instead of the actual 237) that
about 3/16 of the things under the transformation will now round to a different
pixel boundary than the other 13/16.  This explains very nicely the fact that
1/6-1/8 of the links move after I scroll down in the test page I'm about to
attach.  It also correctly explains that the links that move move back in the
direction from which they came.

This means the number of places where this happens could be improved by
transforming the coordinate system by the actual value of pixels moved (237
twips) rather than by the number suggested (240 twips).  However, this would
still be incorrect by 0.3 twips.  This would lead to roughly 0.3/15 = 1/50 of
the links moving when they're hovered over.

There's no way to get around that problem other than to force p2t to be a round
number.  So I think that should be done on all platforms.

However, if my analysis is correct, there's another bug, which is that you're
adjusting the coordinate system by the scroll requested (240 twips) rather than
the actual one (237 twips).  I haven't actually checked the code to see if this
is the case, but I think it's pretty likely because of the observed movement
patterns.  This bug wouldn't show up at p2t=15 or p2t=12 (which probably account
for most of the use of Mozilla) but it would show up at some other logical
resolutions.  So there's probably a second bug that needs to be fixed here to
prevent this from happening in all cases.

(However, this doesn't explain why I saw it more at 75dpi, so I'm not fully
satisfied with this explanation.  Maybe it's wrong...)
(Reporter)

Comment 17

18 years ago
Actually, the observed rate of link movement is probably closer to 1/50 *per
arrow down*.  I had just been hitting the down arrow three or four times.

So, the p2t fix may fix the bug completely after all.  The bug I described
earlier may not really exist...
(Reporter)

Comment 18

18 years ago
Created attachment 2517 [details]
link movement test
(Reporter)

Comment 19

18 years ago
Ignore my last comment.  The second bug does exist.  I tested apprunner with my
own patch at dpi=108  -->  p2t=13  -->  real scroll offset=234twips rather than
240twips.  Almost half the links moved, as expected if the second bug does
exist.
(Reporter)

Comment 20

18 years ago
Created attachment 2518 [details] [diff] [review]
patch to fix the second part of the problem
(Reporter)

Comment 21

18 years ago
This patch, along with my other one, fixes all the problems at 108dpi for me.
Note that I changed newpos{X,Y} from unsigned to signed, but I don't think that
matters because nscoord's are signed.  (Should they be declared as nscoord
instead?)

There's still the problem of keeping p2t a round integer on all platforms...

Updated

18 years ago
Assignee: pavlov → kmcclusk
Status: REOPENED → NEW

Comment 22

18 years ago
i've done this on linux (gtk).  your turn for windows
(Reporter)

Comment 23

18 years ago
Don't miss the XP part of the patch ("patch to fix the second part of the
problem")... that's actually a larger effect.
(Reporter)

Comment 24

18 years ago
I'm actually beginning to wonder if anything other than the second patch (XP) is
really needed.  It might be OK without p2t being an integer.  I'd have to test
with the second and not the first...
(Reporter)

Comment 25

18 years ago
OK - I tried building with my second patch (the XP one) and not my first (the
one pavlov already checked in for GTK).  Both patches are needed.  However, the
second alone fixes most of the problem.  (The first alone happened to fix it all
at *my* resolution only because my new dpi rounded to p2t=16, and 240 is
divisible by 16, so I couldn't hit the problem by scrolling with arrow keys.)
(Reporter)

Updated

18 years ago
Whiteboard: XP part of fix attached. Still needs simple gfx change on Win and Mac.
(Reporter)

Updated

18 years ago
Target Milestone: M12
(Reporter)

Comment 26

18 years ago
Marking M12 since there's a patch attached (although that doesn't completely fix
the problem on Windows (and Mac?) yet, but that bit's really easy).

Updated

18 years ago
Assignee: kmcclusk → dcone
Reassigning to Don.
(Assignee)

Updated

18 years ago
Status: NEW → ASSIGNED
(Assignee)

Updated

18 years ago
Target Milestone: M12 → M13
(Reporter)

Comment 28

18 years ago
I'll need to investigate whether this fix still works for GFX scrollbars.  In
particular, I'm a little suspicious of the line
scrolledView->SetPosition(-aX, -aY);
in nsScrollPortView::ScrollTo.  However, that's only after looking at it for 2
minutes.  Depending on what scrolledView is, things might still be OK... (but
that might also be a 'better' place to fix the bug)...
(Reporter)

Comment 29

18 years ago
I should make sure this is fixed for all scrolling and not just scrolling of the
entire document...
(Assignee)

Updated

18 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago18 years ago
Resolution: --- → FIXED
(Assignee)

Comment 30

18 years ago
I don't see the problem anylonger..
(Reporter)

Comment 31

18 years ago
I do, in Linux apprunner 2000-01-04-09.  Set dpi to 108, load attached link
movement test, click in window, hit down arrow once, hover over links.  They
move. Reopening.

If you don't want to deal with this, you can assign it to me.  Note that there
are already patches attached to fix the bug, but they may not be complete.  I'll
try to investigate more soon.
(Reporter)

Updated

18 years ago
Status: RESOLVED → REOPENED
(Reporter)

Comment 32

18 years ago
Really reopening...
(Reporter)

Updated

18 years ago
Resolution: FIXED → ---
(Reporter)

Comment 33

18 years ago
And it's mathematically impossible that it's completely fixed on windows... see
the comments above.
(Assignee)

Comment 34

18 years ago
Can you tell me what exactly you are seeing that is wrong.  I have some fixes
for the transformation stuff in my build I have not checked in yet... could this
have fixed the problem.  When I compare the screen shots with my build I really
don't see the problem.  Also, what line are you saying is mathematically
impossible to fix...
(Reporter)

Comment 35

18 years ago
First, there may be a debug/opt difference here (I have no idea how).  I see the
bug in the mozilla.org build from 2000-01-06-13-M13, but not in my build
compiled from this morning's (2000-01-06) tree closure.  Could the debug build
be doing a full repaint instead of moving bits around the screen?  That could
explain it.

Second, without rounding p2t to an integer (which is only done so far on Linux,
and not Windows), it's mathematically impossible to fix this in all cases.  With
the scrolling fixes, its occurrence is greatly reduced, but it can't be fixed
completely.  See the math in my previous comments.

Third, what kind of a transformation fix do you have?  I don't think a
transformation fix would fix this.  The correction to the scrolling code I made
(second patch, above) fixes what is definitely a bug, and I'm not sure how a
transformation fix could affect that.

I suspect what you're seeing is some debug/opt difference in repainting, unless
the mozilla.org build was based on different source than mine this morning (and
even so, what would have fixed this?).
(Reporter)

Comment 36

18 years ago
Actually, it seems fixed in my mozilla.org opt build too if I switch to GFX
scrollbars. (I meant to do that when testing, but forgot.  I don't use them
because of bug 20185.)  But now I wonder if that's because it's really fixed or
if it's being hidden by a painting bug.
(Reporter)

Comment 37

18 years ago
...and it's fixed in my build because my patch is still in my tree.  Oops...

This seems fixed for GFX scrollbars, although I suspect it still happens in the
border cases on Windows that I mentioned before.  (That is, the GFX scrollbar
code probably works the way the native code would have, incorporating my second
patch.  But, my first patch has still only been applied to Linux.)  I'd also
like to do a bit more testing of it (in things that scroll other than main
windows).
(Reporter)

Comment 38

18 years ago
...and since Native scrollbars are still an option (and also may be used/copied
by someone in the future), it wouldn't be crazy to apply the patch for them,
would it?
(Reporter)

Comment 39

18 years ago
It's actually not fixed at the correct level for GFX scrollbars.  When scrolling
with arrows/keys, it seems OK.  However, when scrolling to named anchors
(ScrollFrameIntoView, I think), there are still serious problems.  For example,
at 108dpi, click on some of the named anchor links in
http://www.fas.harvard.edu/~dbaron/links/Software , hover over links, and you
will see the problem.  (I'm not exactly sure why there's no need to scroll after
clicking the anchor...)  My patch doesn't seem to fix it, either (but it does
change the behavior a bit).

I'll try to look into this again sometime...
(Assignee)

Updated

18 years ago
Target Milestone: M13 → M14
(Reporter)

Comment 40

18 years ago
I have a new fix that I think fixes the problem for both GFX (changes to
nsScrollPortView.cpp) and Native (changes to nsScrollingView.cpp, or maybe those
help both...) scrollbars.  Could you look at it and see if it's OK?  If it is, I
could check it in if you review it.

I'll attach it to this bug.
(Reporter)

Comment 41

18 years ago
Created attachment 4179 [details] [diff] [review]
new fix for the second part of the problem
(Reporter)

Comment 42

18 years ago
Actually, there's still a little problem with that fix, if I click on a named
anchor that scrolls a frame into view within a page of the end, because the
pixel rounding was done before the values were clamped.  Since I'm not sure
whether those checks to see if the scrollbar widgets are not null are ever
intended to fail in normal use, I moved the clamping outside of the scrollbar
checks and before the pixel rounding, and that fixes the problem.

New patch on its way...
(Reporter)

Comment 43

18 years ago
Created attachment 4180 [details] [diff] [review]
new fix, yet again
(Reporter)

Comment 44

18 years ago
Created attachment 4185 [details] [diff] [review]
fourth fix for the second (main) part of the bug
(Reporter)

Comment 45

18 years ago
The above new patch fixes the "if there's no change, don't do anything" bit for
the GFX scrollbar code, and adds such a bit for the native scrollbar code.
(Assignee)

Comment 46

18 years ago
You have the fix for this.  It looks fine on my Windows machine.  Can you check 
one more time, and if there is still a problem put in your fix that I reviewed 
last week.  
Assignee: dcone → dbaron
Status: REOPENED → NEW
(Reporter)

Comment 47

18 years ago
I'll try and do that early next week.  (There's a chance I could this weekend,
but it's low, I think.)
(Reporter)

Comment 48

18 years ago
I still see the problem in the 2000-01-30-09-M14 build from mozilla.org, with
both GFX and native scrollbars.  Therefore, I'll check the fix in (which fixes
most, but not all, of the bug) tonight or tomorrow.

There's a problem with jitter of list bullets in
http://www.fas.harvard.edu/~dbaron/links/Internet that this fix will cover up -
it seems the bullets are being moved around after the initial paint - I'm not
sure why (the second position is correct, but this fix makes the first one
correct too, I think).  I should investigate this...
(Reporter)

Comment 49

18 years ago
I checked in the fix (r=dcone).  I don't think this bug is completely fixed,
because some small changes to Windows (and Mac) GFX code may be necessary.  I'm
holding on to it for the time being, and I'll figure out who to assign it to...

Someone also needs to investigate:
 * the list bullet problem (see previous comment)
 * whether transform fixes could fix this
(Reporter)

Comment 50

18 years ago
Don - I'm giving this bug back to you because it's not completely fixed, but I'm
not sure if the rest of the fix will be necessary if you fix it via the
transforms.  If you think you make a transform change that fixes it, I'd be
happy to help investigate and help undo the other changes.

However, if the transform changes don't fix it, there is a minor GFX change that
needs to be made on Windows, like the one made before on Unix/GTK (see above).

In either case, it's not high priority, so I'm marking as M20.  I'll stick the
list bullets thing on my permanent list of things to investigate as time
approaches infinity (i.e., possibly never).  Unless someone else cares, that
is...
Assignee: dbaron → dcone
Target Milestone: M14 → M20
(Assignee)

Comment 51

18 years ago
I can not see this problem.. I keep trying on Windows 98, NT and Mac.. I can not 
see it.  Is this fixed?
Status: NEW → ASSIGNED
(Reporter)

Comment 52

18 years ago
You should be able to see it about 1/50 of of the time (or so) if your logical
resolution is set to something that doesn't translate into an integral p2t (that
is, a logical resolution x where 1440/x is not an integer).  For example, set
your Windows logical res to 91dpi (or perhaps 93dpi would work better) (this can
only be done with some video drivers), and try my "link movement test"
attachment.  Probably one or two of the links will move, although you may need
to hit arrow-down more than once to see the effect at all.

That is, assuming you didn't fix this in the transforms...
(Assignee)

Comment 53

18 years ago
This works for me on NT and Windows 98.  I tried a variety of resolutions.. all 
work.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago18 years ago
Resolution: --- → FIXED

Updated

18 years ago
Keywords: verifyme

Comment 54

17 years ago
Marking verified in the May 30th builds.
Status: RESOLVED → VERIFIED
(Reporter)

Comment 55

17 years ago
Reopening.  I still see this problem if I:

 * load http://www.w3.org/TR/SVG/
 * use PgDn to go to the bottom
 * hit PgUp once
 * hover over the ToC links

I'm running at 126 dpi, I think.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
(Reporter)

Comment 56

17 years ago
My window was at 973x1117 when I saw the problem.
(Reporter)

Comment 57

17 years ago
I still see problems in
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=2517 .  That's a far
clearer test.
(Assignee)

Comment 58

17 years ago
I am dupping this to a roundoff bug.. so I can keep track of the problems we 
have using twips..
This is a problem because we are using twips.  When we map to pixels.. there 
will be roundoff errors.  These errors can not be solved until all starting and 
ending locations of any graphics that goto the screen are put onto pixel 
boundaries..in twips.  

*** This bug has been marked as a duplicate of 63336 ***
Status: REOPENED → RESOLVED
Last Resolved: 18 years ago17 years ago
Resolution: --- → DUPLICATE
Summary: 1 pixel position errors → 1 pixel position errors..
(Reporter)

Updated

16 years ago
Blocks: 134942

Updated

15 years ago
No longer blocks: 134942

Comment 59

15 years ago
v
Status: RESOLVED → VERIFIED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.