Re: [PATCH] Fix bugs 3550, 3845 (win mousewheel bugs)

From: Andrew Dunbar (hippietrail@yahoo.com)
Date: Mon Feb 17 2003 - 20:37:33 EST

  • Next message: Tomas Frydrych: "Re: layout patch - transform handling needed"

     --- Johnny Lee <typo_pl@hotmail.com> wrote:
    > The patches against HEAD and STABLE are attached.
    > The actual non-whitespace changes aren't very big.
    >
    > I had to indent a bunch of code to make it look good
    > which made the diffs bigger than they really are.

    Hi Johnny. Please try to resist the temptation to do
    this when patching code. It makes it very difficult
    to find the real changes with standard cvs & diff
    tools.
    If it helps to change indenting etc to find the bug
    then do the same fix to some code that hasn't been
    reindented to make the actual patch.
    If the code is so badly indented that it's really
    worth fixing, this should be a seperate commit to
    cvs, clearly noting that no code is being changed.

    Just a tip - keep up the good work!

    Andrew Dunbar.

    > Bug 3550:
    >
    > When the document is >60 pages, the vertical
    > scrollbar range > 16-bits.
    > But the windows message that the UI uses to handle
    > mousewheel msgs only
    > allows 16-bits of info to be passed to the window
    > proc.
    >
    > To get around this, the scrollbar code currently
    > scales this value up as
    > appropriate to get from the 16-bit pos to the actual
    > pos.
    >
    > Except in the mousewheel case, where the code was
    > sending the actual desired
    > Y-coords. The window proc receives the message and
    > proceeds to scale the
    > value up as it usually does. So the mousewheel ends
    > up moving 2x as much. It
    > snowballs as you get further down the file.
    >
    > When there's less than ~60 pages, the vertical
    > scrollbar range fits into
    > 16-bits so the scale factor is set to 1 which means
    > no change occurs to the
    > scroll pos due to scaling.
    >
    > Fix is to scale the value down before sending the
    > scrollbar msg, since the
    > scrollbar code will scale the msg back up. this is
    > the minimal change to fix
    > the bug. we lose some lower bits of info, but the
    > code currently lives with
    > that Windows 16-bit limitation.
    >
    >
    > Bug 3845:
    >
    > When you change the mousewheel to scroll down "one
    > screen" at a time in the
    > Mouse control panel, Windows will now return
    > WHEEL_PAGESCROLL (== UINT_MAX
    > == 0xFFFFFFFF) as the number of lines to scroll in
    > one mouse wheel move.
    >
    > We end up going in the opposite direction when the
    > mouse wheel is moved
    > since UINT_MAX == -1 in signed math.
    >
    > Fix is to look out for WHEEL_PAGESCROLL and treat
    > the mouse wheel move as a
    > page scroll up or down as appropriate.
    >
    >
    >
    >
    _________________________________________________________________
    > The new MSN 8: advanced junk mail protection and 2
    > months FREE*
    > http://join.msn.com/?page=features/junkmail
    > > --- ap_win32frameimpl.cpp.old Wed Feb 05 19:15:59
    > 2003
    > +++ ap_Win32FrameImpl.cpp Mon Feb 17 06:04:22 2003
    > @@ -947,31 +947,43 @@
    > {
    > // Get delta
    > const int iDelta = (short) HIWORD(wParam);
    > + const int cWheelLines = _getMouseWheelLines()();
    >
    > - // Calculate the movement offset to an integer
    > resolution
    > - const int iMove = (iDelta *
    > _getMouseWheelLines()) / WHEEL_DELTA;
    > -
    > - // Get current scroll position
    > - SCROLLINFO si = { 0 };
    > -
    > - si.cbSize = sizeof(si);
    > - si.fMask = SIF_ALL;
    > - fImpl->_getVerticalScrollInfo(&si);
    > -
    > - // Clip new position to limits
    > - int iNewPos = si.nPos - (((iMove)?iMove:1) *
    > SCROLL_LINE_SIZE);
    > - if (iNewPos > si.nMax) iNewPos = si.nMax;
    > - if (iNewPos < si.nMin) iNewPos = si.nMin;
    > -
    > - if (iNewPos != si.nPos)
    > - {
    > - // If position has changed set new position
    > - SendMessage(hwnd,
    > - WM_VSCROLL,
    > - MAKELONG(SB_THUMBPOSITION, iNewPos),
    > - NULL);
    > - }
    > -
    > + if (WHEEL_PAGESCROLL == cWheelLines)
    > + {
    > + WORD wDir = (iDelta < 0) ? SB_PAGEDOWN :
    > SB_PAGEUP;
    > + SendMessage(hwnd,
    > + WM_VSCROLL,
    > + MAKELONG(wDir, 0),
    > + NULL);
    > + }
    > + else
    > + {
    > + // Calculate the movement offset to an integer
    > resolution
    > + const int iMove = (iDelta * cWheelLines) /
    > WHEEL_DELTA;
    > +
    > + // Get current scroll position
    > + SCROLLINFO si = { 0 };
    > +
    > + si.cbSize = sizeof(si);
    > + si.fMask = SIF_ALL;
    > + fImpl->_getVerticalScrollInfo(&si);
    > +
    > + // Clip new position to limits
    > + int iNewPos = si.nPos - (((iMove)?iMove:1) *
    > SCROLL_LINE_SIZE);
    > + if (iNewPos > si.nMax) iNewPos = si.nMax;
    > + if (iNewPos < si.nMin) iNewPos = si.nMin;
    > +
    > + if (iNewPos != si.nPos)
    > + {
    > + // If position has changed set new position
    > + iNewPos >>= fImpl->m_vScale;
    > + SendMessage(hwnd,
    > + WM_VSCROLL,
    > + MAKELONG(SB_THUMBPOSITION, iNewPos),
    > + NULL);
    > + }
    > + }
    > return 0;
    > }
    >
    >
    > > --- ap_Win32Frame.cpp.old Fri Nov 29 09:18:28 2002
    > +++ ap_win32frame.cpp Mon Feb 17 06:22:02 2003
    > @@ -1026,30 +1026,43 @@
    > {
    > // Get delta
    > const int iDelta = (short) HIWORD(wParam);
    > + const int cWheelLines = GetMouseWheelLines();
    >
    > - // Calculate the movement offset to an integer
    > resolution
    > - const int iMove = (iDelta *
    > GetMouseWheelLines()) / WHEEL_DELTA;
    > + if (WHEEL_PAGESCROLL == cWheelLines)
    > + {
    > + WORD wDir = (iDelta < 0) ? SB_PAGEDOWN :
    > SB_PAGEUP;
    > + SendMessage(hwnd,
    > + WM_VSCROLL,
    > + MAKELONG(wDir, 0),
    > + NULL);
    > + }
    > + else
    > + {
    > + // Calculate the movement offset to an integer
    > resolution
    > + const int iMove = (iDelta * cWheelLines) /
    > WHEEL_DELTA;
    >
    > - // Get current scroll position
    > - SCROLLINFO si = { 0 };
    > + // Get current scroll position
    > + SCROLLINFO si = { 0 };
    >
    > - si.cbSize = sizeof(si);
    > - si.fMask = SIF_ALL;
    > - f->_getVerticalScrollInfo(&si);
    > + si.cbSize = sizeof(si);
    > + si.fMask = SIF_ALL;
    > + f->_getVerticalScrollInfo(&si);
    >
    > - // Clip new position to limits
    > - int iNewPos = si.nPos - (iMove *
    > SCROLL_LINE_SIZE);
    > - if (iNewPos > si.nMax) iNewPos = si.nMax;
    > - if (iNewPos < si.nMin) iNewPos = si.nMin;
    > + // Clip new position to limits
    > + int iNewPos = si.nPos - (iMove *
    > SCROLL_LINE_SIZE);
    > + if (iNewPos > si.nMax) iNewPos = si.nMax;
    > + if (iNewPos < si.nMin) iNewPos = si.nMin;
    >
    > - if (iNewPos != si.nPos)
    > - {
    > - // If position has changed set new position
    > - SendMessage(hwnd,
    > - WM_VSCROLL,
    > - MAKELONG(SB_THUMBPOSITION, iNewPos),
    > - NULL);
    > - }
    > + if (iNewPos != si.nPos)
    > + {
    > + // If position has changed set new position
    > + iNewPos >>= f->m_vScale;
    > + SendMessage(hwnd,
    > + WM_VSCROLL,
    > + MAKELONG(SB_THUMBPOSITION, iNewPos),
    > + NULL);
    > + }
    > + }
    > }
    > return 0;
    >
    >
    >

    =====
    http://linguaphile.sourceforge.net/cgi-bin/translator.pl http://www.abisource.com

    __________________________________________________
    Do You Yahoo!?
    Everything you'll ever need on one web page
    from News and Sport to Email and Music Charts
    http://uk.my.yahoo.com



    This archive was generated by hypermail 2.1.4 : Mon Feb 17 2003 - 20:42:13 EST