Hello Hans,

On 29.09.2016 09:56, Hans de Goede wrote:
Hi,

On 28-09-16 16:54, Michael Thayer wrote:
Hello Hans,

On 28.09.2016 15:37, Hans de Goede wrote:
Hi Michael,

On 28-09-16 14:47, Michael Thayer wrote:
[...]
On 09/16/2016 06:52 PM, Michael Thayer wrote:
When the X server asks us to load a hardware cursor, that
request is always followed up by a request to show it if we
report success, or to hide it if we report failure.  Therefore
it makes no sense to suppress the request if the cursor is not
currently visible.
[...]
For the record, I looked at making show_cursor() return a boolean.  It
seemed feasible, and would allow for removing the first time hack in
modesetting too

Making show_cursor return an error (adding a show_cursor_check()) seems
to be the best solution to me, we actually had a patch submitted for
this, but it was deemed unnecessary. I guess it is necessary after all:

https://patchwork.freedesktop.org/patch/94495/

but there was a catch in that the cursor could theoretically be
visible but still hidden on all screens (with a strange screen layout),

Yes that is possible.

in which case show_cursor() would not be called until the cursor moved
onto a screen.

Ack.

 Making set_cursor_position() return a success value seemed a bit too
invasive.

But in the scenario you describe it is not the drivers
set_cursor_position()
which will get called (well not only) also the drivers' show_cursor will
get
called on the crtc where the cursor should now show, so just making
show_cursor
return an error should be enough.
[...]

Right, xf86_crtc_set_cursor_position() calls the driver's
show_cursor().  What I meant was that that means that this function
can now fail, and that failure needs to be propagated up and probably
handled in xf86CursorMoveCursor() by reverting to a software cursor
there.  The alternative is another first time-style hack (if we are
asked to show the cursor and it is not in range of any of the crtc-s
then flicker it).

Hmm, so 2 things:

1) We definitely do not want another first-time hack, so if we really
need to lets do the propagate error thing
2) Thinking about holes in crtc layouts again, I think the xrandr cursor
constraint code will warp the pointer to
the closest crtc then (pretty sure actually) so I think this is a non
issue. So just rebasing the patch I linked
(and dropping the first time hack) should be enough. And since we're
planning to do this for 1.20, I think we
should just try doing things that way, we've an entire cycle to catch
any issues then (which I do not believe
there will be)

I rebased Alexandre's patch (not sure if there is an official way to credit someone, I just did so in the commit message) as two new patches, one to xfree86 and one to modesetting (in which I also got rid of the first time hack), both slightly adjusted to be more like existing code. I rebased my software-back-to-hardware on top of that.

I did not follow all code paths in detail, but it does look to me as if what you said about the constraint code is what is intended (can someone who knows RandR confirm?), so that if it does not work then it is the constraint code which should be fixed.

I tested that switching to a software cursor happens as soon as drmmode_show_cursor() is called, that the test for hardware cursor support is repeated on every show or set call, and although I can't confirm visually (we only have a single hardware cursor, not one per crtc), I checked as best I could using break-points in gdb that showing and hiding cursors happened correctly on multiple screens. Feel free to suggest additional tests.

Patches coming up.

Regards,

Michael

Regards,

Hans





Regards,

Michael

--
Michael Thayer | VirtualBox engineer
ORACLE Deutschland B.V. & Co. KG | Werkstr. 24 | D-71384 Weinstadt

ORACLE Deutschland B.V. & Co. KG
Hauptverwaltung: Riesstraße 25, D-80992 München
Registergericht: Amtsgericht München, HRA 95603

Komplementärin: ORACLE Deutschland Verwaltung B.V.
Hertogswetering 163/167, 3543 AS Utrecht, Niederlande Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697
Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher
_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to