On Fri, 2008-11-28 at 18:29 +0100, Matthias Hopf wrote: This looks good to me, I only have a few minor comments about the style, nothing substantial below.
Thanks for getting this together. > + Note that changes to the screen size might invalidate panning > + parameters. In these cases panning might be silently disabled, or the > + panning parameters are updated automatically as necessary. The exact > + behavior of the implementation is undefined. If the panning parameters > + do not conflict with new screen size, panning remains enabled > + unchanged. I don't see a need to leave this undefined; let's specify what screen size changes do to panning and then implement it. Or vice-versa. And, no, I don't care what the defined behaviour is (it could be as simple as 'panning is disabled on any screen size change'). > + > ┌─── > RRGetScreenResources > window: WINDOW > @@ -938,6 +945,12 @@ dynamic changes in the display environment. > then re-enabling the CRTC at the new configuration to avoid an > invalid intermediate configuration. > > + Note that changes to the CRTC might invalidate panning parameters. In > + these cases panning might be silently disabled, or the panning > + parameters are updated automatically as necessary. The exact behavior > + of the implementation is undefined. If the panning parameters do not > + conflict with new CRTC parameters, panning remains enabled unchanged. > + Same here -- don't leave this undefined. > When this request succeeds, 'status' contains Success and the > requested changes to configuration will have been made. > > @@ -1059,6 +1072,103 @@ This request returns the pending and current > transforms for the specified > CRTC. The pending transform will be the same as the current transform if no > new pending transform has been set since the last call to RRSetCrtcConfig. > > +┌─── > + RRGetPanning > + crtc: CRTC > + config-timestamp: TIMESTAMP > + ▶ > + status: RRCONFIGSTATUS > + timestamp: TIMESTAMP > + left, top, width, height: CARD16 > + track_left, track_top, track_width, track_height: CARD16 > + border_left, border_top, border_right, border_bottom: INT16 > +└─── > + > + Errors: Crtc > + > + Version 1.3 adds panning support again. If multiple crtcs are active > + the panning behavior can be defined per crtc individually. > + RRGetPanning returns information about the currently set panning > + configuration for the specified crtc. > + > + If 'config-timestamp' does not match the current configuration > + timestamp (as returned by RRGetScreenResources), 'status' is set to > + InvalidConfigTime and the remaining reply data is empty. Otherwise, > + 'status' is set to Success. just eliminate the config-timestamp stuff, it isn't useful now that we have stable resource IDs etc. Note that the rest of the RandR protocol doesn't look at config-timestamp anymore as it only causes trouble. > + If 'timestamp' is less than the time when the configuration was last > + successfully set, the request is ignored and InvalidTime returned in > + status. > + > + If 'config-timestamp' is not equal to when the CRTC's configuration > + last changed, the request is ignored and InvalidConfigTime returned in > + status. This could occur if the CRTC changed since you last made a > + RRGetCrtcInfo request, perhaps by setting a different mode. Rather > + than allowing an incorrect call to be executed based on stale data, > + the server will ignore the request. I'd say all of this timestamp stuff can be eliminated. > + 'left', 'top', 'width', and 'height' contain the total panning area > + for this CRTC. 'width' has to be larger than the CRTC's width, and > + 'left'+'width' must be within the screen size, else a Match error > + results. Equivalent restrictions for the height exist. The exception > + is 'width' == 'height' == 0 which indicates that panning should be > + disabled. I'd say that width >= crtc_width, the same for height. That way, you can disable panning in one dimension while leaving it enabled in the other. Also, if width == crtc_width and height == crtc_height, then panning is disabled entirely. You could make width == 0 mean to set width to the crtc_width (like ClearArea does). > + 'track_left', 'track_top', 'track_width', and 'track_height' contain > + the pointer area for which the panning region is updated. For normal > + use cases it should enclose the panning area minus borders, and is > + typically set to either the panning area minus borders, or to the > + total screen size. If set to the total screen size, the CRTC will pan > + in the remaining axis even if the pointer is outside the panning area > + on a different CRTC. So, if the pointer is within this space, then the crtc is moved as far as possible within the panning region to try and make the cursor visible? > + 'border_left', 'border_top', 'border_right', and 'border_bottom' > + define the distances from the CRTC borders that will activate panning > + if the pointer hits them. If the borders are 0, the screen will pan > + when the pointer hits the CRTC borders (behavior of pre-RandR Xserver > + panning). If the borders are positive, the screen will pan when the > + pointer gets close to the CRTC borders, if they are negative, the > + screen will only pan when the pointer is already way past the CRTC > + borders. Negative values might confuse users and are discouraged. > + border_left + border_right has to be lower or equal than the CRTC's > + width, else a Match error results. An equivalent restriction for the > + height exists. Is there some reason to use borders instead of a separate rectangle here? Also, a couple of ascii-art pictures here would help a huge amount. I had to read these several times to get any idea of what they all do, and I'm still not sure I understand. > + This request sets the panning parameters. As soon as panning is > + enabled, the CRTC position can change with every pointer move. > + RRCrtcChangeNotify events are sent to the clients requesting those. > + > + Note that changes to the CRTC or screen might invalidate panning > + parameters. In these cases panning might be silently disabled, or the > + panning parameters are updated automatically as necessary. The exact > + behavior of the implementation is undefined. If the panning parameters > + do not conflict with new CRTC parameters or screen size, panning > + remains enabled unchanged. As above, we need well defined behaviour. -- [EMAIL PROTECTED]
signature.asc
Description: This is a digitally signed message part
_______________________________________________ xorg mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/xorg
