[PATCH] fixes: Pointer barriers

[Soeren Sandmann]

Adam Jackson <ajax at redhat.com> writes:

> This is pretty much a 'version 0' kind of thing, it looks right to me
> but reviews strongly needed.

A conceptual question I have is: What is supposed to happen on
diagonal movement? A possibility is to decompose such movement into a
horizontal and a vertical component, which is what it looks like this
patch is going for. That has the effect that in this situation:

      *    
       \   
	\  |
	 \ |
	  \|
	   |
	   |\
	   | \
	   |  \
	   |   \
                \
                 *

The cursor movement will be allowed even though geometrically, the
cursor is crossing a barrier. Also, in this situation:


        *    |
         \   |
          \  |
           \ |
            \ 
      ------ \
              \
               *

the movement will not be allowed. To me, both seem wrong.

The algorithm I'd suggest using is this:

    - Write the mouse travel path as 

      	    t -> (ox, oy) * (1 - t) + t * (nx, ny),     t  in  [0,1]

    - Compute the attempted travel directions like the code does now

    - For each barrier, if there is a t such that the path intersects
      the barrier, then add (t, barrier) to a list.

    - Sort that list from least to greatest t.

    - For each (t, barrier) in sorted order, if the barrier does not
      allow travel in one of the attempted directions, confine the
      cursor according to that barrier and stop processing.

But it's probably the sort of thing that would have to be tried out to
be sure it works.


Comments on the code:

In CursorConstrainHarder():

- It looks to me like the barriers will contrain even if the pointer
  is outside the barrier's extent: E.g., in this situation:

     	     *------------*  cursor

		    |
		    |
		    |
		    | barrier

   the cursor will still be constrained by the barrier because this
   code:

	if (b->x1 == b->x2) {
	   if ((ox < b->x1 && *x > b->x1 && !(d & BarrierPositiveX)) ||
	       (ox > b->x1 && *x < b->x1 && !(d & BarrierNegativeX)))
	   {
		...;
           }

   will trigger. Some check against the y coordinates seem to be
   necessary - the exact nature of those checks depends on how the
   conceptual issue described above is resolved.

- It seems there is some redundancy in if clauses. For example, if the
  code detects a positive movement across a vertical barrier, it
  confines if that movement is not both allowed and attempted. But we
  already know that it *was* attempted because otherwise 

        (ox < b->x1 && *x > b->x1) 

  would not have been true. Hence, it is sufficient to check against
  the allowed directions, and there is no need to compute the
  attempted directions. (But this will be moot if barriers for
  diagonal moves are implemented).

- In general, when checking against a barrier, the inequalities should
  be < and >= (and not < and > ) if a barrier is to be considered
  infinitely thin and residing between pixels.

- In this code:

     if ((stuff->x1 != stuff->x2) || (stuff->y1 != stuff->y2))
     	return BadValue.

  I think the "||" should be "&&". It may also be worthwhile checking
  that the barrier is not a single point since if the diagonal
  algorithm is used, there could be divide-by-zero issues.

  Ie., 

    if (stuff->x1 != stuff->x2 && stuff->y1 != stuff->y2)
        return BadValue;

    if (stuff->x1 == stuff->x2 && stuff->y1 == stuff->y2)
        return BadValue;

- This comment:

  /* This sure does need fixing */

  is true of course.

  Probably the list of devices should be stored per-barrier, and then
  when a device is removed, it must be removed from all barriers. The
  alternative would be to store a list of barriers in each
  device. That would be slightly more efficient, but also probably
  more complicated a barrier can apply to all devices, so it would
  involve hooking device creation.


Soren