Re: fullscreen windows, magic values & incomplete backends (was: Re: [Libreoffice-commits] .: vcl/inc vcl/source)

On Wednesday 28 of November 2012, Thorsten Behrens wrote:
> Libreoffice Gerrit user wrote:
> > commit 8bf500c365e3cf24086a672f63b0b5d5b60ff4a9
> > Author: Lubo?? Lu????k <l.lunak@suse.cz>
> > Date:   Mon Nov 26 14:54:34 2012 +0100
> >
> >     make presentation spanning all screens work (bnc#758138)
> >
> >     This reverts an API "improvement" from
> > 6ee5dfa150d408179e20a5525ff7ec46649e0e58 that tried to use -1 as the
> > current screen/display but failed. The check for nDisplayScreen being in
> > range broke all-screens (which is represented as last_screen+1 value,
> > which is broken in its own way, as e.g. the screen count can change). The
> > same way -1 as the default value is broken, as everywhere else invalid
> > invalid screen number means spanning all screens. The extra overloads
> > without a default value should take care of that.
> >
> >     Change-Id: Ie118038eacacebc007c25860732d5904ad0de2b9
>
> On having a closer look, I don't think this description is entirely
> accurate.  Mac, generic unix, and Windows don't seem to know
> anything about "default display", rather they tend to use screen #0
> - but your fix for that part looks much saner to me.

 That can be solved by saying that the default screen[*] there is #0 :).

> For the AllScreens case though, sal backends tend to use any
> arbitrary out-of-range index, so probably picking -1 as the magic
> value & documenting that in wrkwin.hxx sounds good to me.

 It's possible I'm just confused by this sentence, but to be sure, let me 
explain my comment in different words:

 The old commit I removed changed the ShowFullScreenMode() argument to 
accept -1 as "I don't care", in practice meaning "current screen". In 
addition to being wrong technically (as the added if() changed the 
all-screens value to current-screen), this was also wrong because everywhere 
else invalid values (and thus also -1) mean (span-)all-screens. So I went 
with removing all that code and using the overloads the get a similar 
current-screen effect if not explicitly specified.

 Using last_screen+1 as all-screens is also somewhat broken, because the 
number of screens can easily increase and thus the meaning of the value 
(saved in Impress' config I assume) can change. I pondered changing that to 
e.g. -1, but as I wasn't entirely sure of all the consequences of that change 
in practice it should be rather rare, I didn't do anything with this part.

 If this is in line with what you meant, then ok.

[*] The display vs screen distinction is mostly rather confusing, especially 
when people either don't know the difference or try to make them appear as 
the same. Let me just say 'screen' the whole time here.

-- 
 Lubos Lunak
 l.lunak@suse.cz