RandR bugs

[Alan Hourihane]

On Thu, 2005-09-29 at 12:33 +0100, Alan Hourihane wrote:
> On Thu, 2005-09-29 at 13:16 +0200, Thomas Winischhofer wrote:
> > -----BEGIN PGP SIGNED MESSAGE-----
> > Hash: SHA1
> > 
> > Alan Hourihane wrote:
> > > On Thu, 2005-09-29 at 12:40 +0200, Thomas Winischhofer wrote:
> > >>>>3) This is a more conceptual issue:
> > >>>>
> > >>>>The sis driver allows switching output devices during server-runtime.
> > >>>>However, not all modes are supported for all available output devices.
> > >>>>Under normal circumstances, the driver's mode validation takes care of this.
> > >>>>
> > >>>>But:
> > >>>>
> > >>>>RandR does not allow setting a configuration, be it display size, be it
> > >>>>rotation/reflection, without (re-)setting the display mode. This is
> > >>>>suboptimal.
> > >>>>
> > >>>>For example, if I have a 1280x800 virtual screen and a current display
> > >>>>mode of 1024x768 (eg because I switched from LCD to TV, and TV does not
> > >>>>support 1280x800), setting the RandR config just in order to change the
> > >>>>rotation will cause RandR to try to switch the display mode to 1280x800
> > >>>>(because that is the desired screen size). This will fail (due to the
> > >>>>driver validating the display mode).
> > >>>>
> > >>>>At this point everything goes havoc due to the bugs mentioned in 1) and
> > >>>>2) above.
> > >>>>
> > >>>>However, even if the failures of xf86SwitchMode() are handled properly,
> > >>>>the whole RandR request will fail.
> > >>>>
> > >>>>So, I propose the following to solve this: RandR should only touch the
> > >>>>display mode if it is too large for the desired screen size. Otherwise
> > >>>>the display mode should be left untouched.
> > >>>>
> > >>>>Comments?
> > >>>
> > >>>
> > >>>Sounds reasonable. Have you got a patch ? That's much easier to comment
> > >>>on. It might be reasonable to open a bug on bugzilla for this so
> > >>>reasonable patches can be made and reviewed.
> > >>>
> > >>>Also, I'm not sure RRFunc is needed at all. It seems to me that
> > >>>xf86SwitchMode() is good enough to let the driver handle the necessary
> > >>>switch.
> > >>>
> > >>>The only thing that is missing is a call that the driver can make to
> > >>>obtain the current rotation mode (i.e. randrp->rotation). Removing the
> > >>>need completely for RRFunc.
> > >>
> > >>
> > >>I somewhat like the idea of the DriverFunc (as it is called in HEAD).
> > >>And it's already used for Egbert's method to eventually run the server
> > >>without being root.
> > > 
> > > 
> > > Mmm, I've not heard about that.
> > 
> > 
> > See the fbdev and sisusb drivers and xf86Init.c for this. Both drivers
> > allow running the server as non-root.
> 
> I'll take a look. Thanks for the pointer.
> 
> > >>And using the driver's switchmode function does not work around the
> > >>issue neither. Basically, I don't see what stuff like screen size or
> > >>rotation or reflection have to do with the display mode. It is not
> > >>RandR's business to mess with the display mode, IMHO.
> > > 
> > > 
> > > RandR doesn't mess with the display mode at all. It just validates the
> > > calling data against the current modepool.
> > >
> > > We already use xf86SwitchMode(), as RandR allows us to switch to
> > > different resolutions. 
> > 
> > 
> > What is "resolution" in that case? This sounds very much like "display
> > resolution", ie "display mode".
> 
> Right.
> 
> > But I question just that: Is it really the "*Resize* and *Rotate*"
> > extension's job to set the display mode/rate?
> 
> It's resize and rotate with respect to the drivers validated modes from
> it's own modepool. 
> 
> > It completely ignores that eventually the user wants a different display
> > mode than screen size...
> 
> I'm not sure what you mean here, but when you do 'xrandr -q' it lists
> valid display modes to switch to. You can use 'xrandr -s ...' to switch
> to a different mode which is pretty much the same as using CTRL+ALT and
> +/- to switch, but 'xrandr -s ..' let's you jump straight to the mode in
> question.

One thing to clarify here. Is that the CTRL+ALT method keeps the virtual
size, whereas the randr method actually changes the logical screen size.

Using xdpyinfo will show that that actually happens.

Alan.