Re: Scrollbars patch

[Harold L Hunt <huntharo at msu dot edu>]

Jehan <nahor@bravobrava.com> said:

> Harold L Hunt wrote:
> >>>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?
> >>
> > 
> > No.  I never got the original code to stop displaying scrollbars if the
> > -scrollbars parameter was passed.  It didn't matter how large the original
> > window was, nor did it matter how you resized the window.  I could not get
> > those scrollbars to go away.
> 
> Hmm, that's weird. I was even explicitly hiding the scrollbars with a 
> call to ShowScrollbars (not at creation but in WM_SIZE).
> 

I never saw anything in the code that looked like it would actually result in
a call to ShowScrollbars.  What I mean is, I didn't see a way that the logic
values would be such that the call would actually happen.

One question, are you using an auto-hide taskbar?  If not, you might want to
test your original patch when you use auto-hide.  We trim the work area
differently when auto-hide is enabled, which could have been causing a problem
with the original code.

> 
> >>>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, ...)
> >>
> > 
> > I've included a tarball with the missing files above.
> 
> Thanks, but I got them already and that why I get this error.
> hw/xwin/libXwin.a(winconfig.o)(.text+0x2f6):winconfig.c: undefined 
> reference to `xf86openConfigFile'
> hw/xwin/libXwin.a(winconfig.o)(.text+0x355):winconfig.c: undefined 
> reference to `xf86readConfigFile'
> hw/xwin/libXwin.a(winconfig.o)(.text+0x371):winconfig.c: undefined 
> reference to `xf86closeConfigFile'
> 
> So I have winconfig.c... Ok, found my problem. I didn't patch Imakefile. 
> I also found an error in the Imakefile.diff for the last test release on 
> the website. You define XWINPARSELIB but you use XWINPARSERLIB (parse vs 
> parseR).
> 

Yeah, I caught that problem in the Xserver/Imakefile when I went to do a build
on another system that hadn't already built the parser.

> 
> > We can't make ptMaxSize larger than our visual size, because we could have an
> > 800x600 visual on a 1024x768 display... we would end up with several thousand
> > blank pixels if we really maximized the window.  That is the weirdness that I
> > was talking about.
> 
> I'm not saying to make it larger than the visual. I think you should set 
> to the size of the visual. So when some one maximize the window, it will 
> get the visual and no more. The maximum size of the window should 
> 800x600 (+decoration if any) and that's it.
> 

That sounds like what I am having it do right now.  But maximizing a window to
be 800x600 on a 1024x768 display is kinda funny, because Windows positions the
window such that the top and left borders are off the screen, but the right
and bottom borders still display because our window isn't large enough to
cover the whole desktop in this case.

On a side note, I am not currently disallowing moving the window when
maximized, which seems to be the normal thing to do.  But do we disallow
moving the window when we maximize an 800x600 window on a 1024x786 display?  
Seems like that would just confuse users.  Maybe immediately after maximizing
such windows, we should tell Windows that our window is not maximized, so that
the ``restore'' button will switch back to a ``maximize'' button, even though
we are in the maximized position and at maximum size.

What do you think?

> 
> >>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.
> >>
> > 
> > I was assuming that you had intended for scrollbars to be allowed only when
> > not fullscreen and with decorations.  I know you didn't intend for scrollbars
> > to be displayed in fullscreen mode because you didn't modify
> > winCreateBoundingWindowFullScreen.  I don't see any reason why we should
> > disallow scrollbars when there are no window decorations.  I'll readd that
> > functionality in the near future.
> 
> Since if fullscreen mode we change the resolution of the monitor, there 
> is no need for scrollbars. That's why I didn't modify 
> winCreateBoundingWindowFullScreen. I actually modify InitOuput to ignore 
> -scrollbars if -fullscreen was set.
> But checking for fFullscreen in WM_SIZE isn't necessary then because 
> fScrollbars will be FALSE.
> And if one day we allow a different size for the monitor than for the 
> visual (something like XWin -screen 0 1600 1200 -fullscreen 1280 1024), 
> it would be better to just check fScrollbars and be fullscreen agnostic.
> 

Ah... except if we change the resolution with the Shadow GDI engine in
fullscreen (``-engine 1'').  Shadow GDI essentially displays a topmost window
without decoration.  We can't change the Windows display resolution/depth when
running the Shadow GDI engine, so the user gets whatever resolution/depth they
are currently running Windows at.  Thus, if they change the Windows resolution
they will shrink the size available to the Shadow GDI engine... in which case
we would need to display scrollbars.  I think that the current behavior is to
crash or do some undefined things :)

> In the case of nodecoration, nothing prevents one to have something like:
> 	XWin -scrollbars -screen 0 1600 1200
> on his 640x480 monitor. Then we must have scrollbars or we have to 
> reduce the size of the visual.
> 

I don't understand.  Did you mean to pass -nodecoration as well?

> 
> >>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 */
> >>
> > 
> > 
> > Excellent.  I implemented just such a cleanup.
> > 
> > Then, I went further.  We don't even need to call SetScrollInfo in
> > winCreateBoundingWindowWindowed.  Rather, we just set nMin and nMax in our
> > calls to SetScrollInfo in our processing of WM_SIZE.  The overhead for doing
> > this in each WM_SIZE is negligible, and the resulting cleanup is appreciable.
> >  I have simply #if 0'd the cleaned-up code in winCreateBoundingWindowWindowed,
> > just so we have that code, published in a diff somewhere, to fall back on if
> > we ever find a reason to.
> 
> Ok. I'm atually 50-50 between the two solutions. It's kind of weird to 
> set nMax over and over when by definition it's something constant over 
> the XFree session.
> 

I am 50/50 as well, but I think that eliminating code duplication is a good
idea, regardless of the negligible performance hit, since we won't have to
worry about forgetting to update one of the blocks of similar code.

> 
> >>j5) in the case of WM_VSCROLL, SB_BOTTOM should actually be nMax-nPage+1
> >>(same for WM_HSCROLL and SB_RIGHT)
> >>
> > 
> > 
> > Charles Petzold doesn't seem to think so in his book.  I think he was relying
> > on Windows to trim the scroll position to be the maximum allowed position. 
> > However, I went ahead and changed this so that others are not confused.
> 
> Yeah, MSDN is quite confusing on that too. If you don't set nPage or if 
> you don't have nPage (Win3.1 and earlier), nMax was the largest value 
> you could have. MSDN gives an example where they say that if one has 260 
> lines to display but can only show 16 of them at a time, nMax should be 
> 244 (with nMin=1). But they don't say that if you set nPage to 16, then 
> nMax must be 260.
> However, they still say later on in the same article that
> 	MaxScrollPos = MaxRangeValue - (PageSize - 1)
> 
> Also, SetScrollInfo says:
> 	nPos member must specify a value between nMin and nMax - max(nPage – 1, 0)
> 

Yup, those are the same examples and articles that confused me.  The MSDN
Library is about as clear as the Boston Harbor (circa 1985) sometimes :)

> 
> >>>Ah... that's enough for today.  I'm tired.
> >>
> >>Welcome to the club, Good night! ;p
> > 
> > What do you know, it is the next day and I am still tired! :)
> 
> It was hard this early morning but now I'm ok. But I feel that tonight 
> I'm going to crash again.
> 
> 
> 	Jehan
> 

I am mostly tired from my run last night.  Phew.

Harold