Re: Scrollbars patch

[Jehan <nahor at bravobrava dot com>]

Harold L Hunt wrote:
> Jehan,
>
> Implementing a scrollbar patch is quite a bit more complex than you had
> initially thought.
>
> I have done a ton of work on the patch and I've got things much more complete
> now.  I haven't got time to describe all the changes I made, but here is the
> patch against current CVS for you to look over:
>
> http://www.msu.edu/~huntharo/xwin/shadow/xwin-scrollbars-20020710.diff.bz2 (12
> KiB)
>
>
> Okay, I'll give a few highlights:
>
> 1) You were passing FALSE for the fRedraw flag to SetScrollInfo in almost
> every call that you made to the function.  This had multiple effects,
> including causing the thumb position to lag behind the actual scroll location,
> causing the thumb size to not update when you changed the page size, etc.
Actually, I set FALSE in all cases. If not, then I forgot :). If I did 
that, it because on my machine it was working well that way, so I didn't 
want to force Windows to paint the scrollbar twice.


> 2) The code for WM_VSCROLL and WM_HSCROLL made about 8 sets of calls each to
> the same functions with slightly different parameters.  I changed the code to
> calculate the parameters first, then make one set of calls to SetScrollInfo
> and ScrollWindowEx, etc.
I thought about doing something like that and then I forgot.
And I didn't look at the example in MSDN, I understood the message well 
enough and usually their examples suck if they compile at all!! (and 
this one isn't perfect either: they call UpdateWindow in one case but 
not the other, tsss!!!)


> 3) I added a flag for fUserGaveHeightAndWidth to indicate that the user
> explicity passed a height and width for a given screen.  Now when we are
> creating a default-sized window, with -scrollbars, we make the window as large
> as possible, and we shrink the underlying X visual to fit within the client
> area of the Windows window, without displaying the scrollbars.  We show the
> scrollbars if/when the user ever shrinks the window.
>
> 4) When a user does specify a visual size with -scrollbars, we make the
> initial window as large as possible and make the visual the same size as the
> specified size.  We show the scrollbars only if necessary (i.e. we hide them
> if the user passes -screen 0 800 600 -scrollbars on a 1024x768 display).
Hiding the scrollbars, that was already the case, wasn't it?


> 5) You no longer have to specify a width and height for a ``-screen scr_num
> [width height]'' parameter, which allows you to do:
> XWin -scrollbars -screen 0 -screen 1
>
> This would create two full-sized screens that are resizable but that will not
> initially display scrollbars.
This has nothing to do with the scrollbar, does it? But still a good 
feature :)


> 6) I added processing for WM_GETMINMAXINFO, in which we update the maximum
> tracking size for the window.  The processing in WM_SIZING attempted to do the
> same thing, but in actuality it never did anything because it would never see
> sizes larger than the initial window size.  The max tracking size is the
> largest size that the window is allowed to have when it is not maximized.  We
> let the user make the window large enough to display the whole visual, even is
> this means that the window will be larger than the current display (just
> think, they can move the window around to see the part that they are
> interested in... I'm not going to argue with someone that wants to do that). 
> This should allow multi-monitor users to create one huge window and stretch it
> across a couple of monitors, if they so desire.
I didn't know this message, or actually I forgot. That's more elegant 
for sure.


> 7) I added a check to make sure that specified screens are numbered
> consecutively from 0.  Screens do not have to be described in order, but there
> cannot be any gaps in the screen number sequence once all parameters have been
> processed.  This prevents a user from doing ``XWin -screen 1 -scrollbars'' and
> then wondering why the window does not have scrollbars (or resizing support).
>  This fixes an existing, but subtle, bug that no one seems to have stumbled
> across yet.
Same comment as 5). :@p


> 8) I added WM_MAXIMIZE to the window style when -scrollbars is passed.  This
> allows one to maximize the Cygwin/XFree86 window.  However, there are a few
> problems here... such as, what is a maximized 800x600 window on a 1024x768
> screen?  I dunno... try it, it is weird.
I wanted to but I can't compile for now (missing xf86openConfigFile,
xf86readConfigFile, xf86closeConfigFile at link timeand I don't have 
time to look at that yet)
But, a wild guess, isn't ptMaxSize in WM_GETMINMAXINFO for that? (MSDN, 
MINMAXINFO: ptMaxSize | when a window is maximized or resized, ...)


> 9) WarpCursor is messed up when you use mwm to switch to another virtual
> desktop inside of a Cygwin/XFree86 window that is smaller than the underlying
> visual and has scrollbars displayed.  In this case, WarpCursor will blast the
> cursor to the location on the Windows display where the X location should
> be... but that location may actually be scrolled off the Cygwin/XFree86
> window.  In those cases I would like to be able to scroll the warp destination
> into the Cygwin/XFree86 window, then warp accordingly.  Figuring out how to
> get the warp destination into the current scroll viewport may be difficult.
Is it?
	GetScrollInfo(...)
	if ((XY < nPos) || (XY > nPos+Page)) {
	  nPos = max(0, min(nMax-nPage+1, XY-nPage/2))
	  SetScrollInfo(...)
	  GetScrollInfo(...)
	  dwOffset = nPos;
	  ScrollWindowEx(...)
	}
if you rely on Windows to correct the scrolling you can simplify with
	nPos = XY-nPage/2
instead of the min/max thingy

So either I'm missing something, or it's easier than you thought.


j1) in WM_SIZE, you check for fDecoration and fFullscreen for breaking. 
This is not necessary maybe even dangerous, especially in the 
nodecoration case. With nodecoration, I guess we can still receive a 
WM_SIZE if we change the screen resolution via the Display control panel 
or if we change the size of the taskbar.
And the other hand, I don't thing we'll get any bug if we still do 
process WM_SIZE.
Well, let's say that, as a matter of preference, I usually manage 
unlikely cases if the code necessary to do it doesn't make my code 
harder to read.


j2) Just for detail and the pleasure of commenting (:p), you also have 
those lines
	/* Is the client area large enough to show the whole visual? */
	if (iWidth != s_pScreenInfo->dwWidth
	    || iHeight != s_pScreenInfo->dwHeight)
  if iWidth is bigger than s_pScreenInfo->dwWidth (same for height), 
isn't the screen "large enough to show the whole visual"?


j3) In your note in wincreatewnd.c, when you describe what is range and 
what is page, you say: "In other words, the page size is the size of the 
client area minus the space the scrollbars occupy". And as you said in 
winwndproc.c, the client area already takes into account the space 
occupied by the scrollbars.


j4) Still in wincreatewnd.c, for better clarity, I would write something 
like:
	if (!(pScreenInfo->fScrollbars && pScreenInfo->fUserGaveHeightAndWidth)) {
	  /* except if the user specified the size of the visual *and*
	     uses scrollbars, we don't want the visual bigger than
	     the window */
	  pScreenInfo->dwWidth = rcClient.right - rcClient.left;
	  pScreenInfo->dwHeight = rcClient.bottom - rcClient.top;
	}
	
	if (pScreenInfo->fScrollbars)
	  ...
	  si.nMax = pScreenInfo->dwWidth - 1;
	  SetScrollInfo (*phwnd, SB_HORZ, &si, FALSE);
	  ...
	  si.nMax = pScreenInfo->dwHeight - 1;
	  SetScrollInfo (*phwnd, SB_VERT, &si, FALSE);
	}	
	/* no else anymore */


j5) in the case of WM_VSCROLL, SB_BOTTOM should actually be nMax-nPage+1 
(same for WM_HSCROLL and SB_RIGHT)


j6) in WM_GETMINMAXINFO, you assume that we have decoration. Can't we 
receive this event if we resize the screen? (and same reason than in j1)


> Ah... that's enough for today.  I'm tired.
Welcome to the club, Good night! ;p