Re: [PATCH xwayland 2/2 v2] xwayland: Always update the wl_pointer cursor on pointer focus

2015-09-29 Thread Keith Packard
Jonas Ådahl  writes:

> As mentioned, a different approach would be to work-around the issue in
> xwayland, but I consider such a solution more hacky than this. Please let
> me know what you think.

All you need do in xwayland is smash pSpriteCursor to some other value,
like ((CursorPtr) (intptr_t) 1). That'll make the existing checks fail
and force the driver to be called with the new cursor.

No mipointer changes ABI/API needed, which means we could merge this for
1.18 (given the DIX patch which I've already reviewed).

-- 
-keith


signature.asc
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xwayland 2/2 v2] xwayland: Always update the wl_pointer cursor on pointer focus

2015-09-29 Thread Jonas Ådahl
On Tue, Sep 29, 2015 at 09:49:46AM -0700, Keith Packard wrote:
> Jonas Ådahl  writes:
> 
> > As mentioned, a different approach would be to work-around the issue in
> > xwayland, but I consider such a solution more hacky than this. Please let
> > me know what you think.
> 
> All you need do in xwayland is smash pSpriteCursor to some other value,
> like ((CursorPtr) (intptr_t) 1). That'll make the existing checks fail
> and force the driver to be called with the new cursor.
> 
> No mipointer changes ABI/API needed, which means we could merge this for
> 1.18 (given the DIX patch which I've already reviewed).

The added field is in the end of the struct, and for what I can
understand from Adam running abidiff, the only remaining ABI issue was
the fact that SpriteRec was put in the middle of another public struct
making it impossible to extend; this new patch does not modify
SpriteRec. It feels less hacky than adding a "fake" pointer
((intptr_t) 1)and the necessary checks all over mipointer.c.


Jonas

> 
> -- 
> -keith


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xwayland 2/2 v2] xwayland: Always update the wl_pointer cursor on pointer focus

2015-09-29 Thread Keith Packard
Jonas Ådahl  writes:

> On Tue, Sep 29, 2015 at 09:49:46AM -0700, Keith Packard wrote:
>> Jonas Ådahl  writes:
>> 
>> > As mentioned, a different approach would be to work-around the issue in
>> > xwayland, but I consider such a solution more hacky than this. Please let
>> > me know what you think.
>> 
>> All you need do in xwayland is smash pSpriteCursor to some other value,
>> like ((CursorPtr) (intptr_t) 1). That'll make the existing checks fail
>> and force the driver to be called with the new cursor.
>> 
>> No mipointer changes ABI/API needed, which means we could merge this for
>> 1.18 (given the DIX patch which I've already reviewed).
>
> The added field is in the end of the struct, and for what I can
> understand from Adam running abidiff, the only remaining ABI issue was
> the fact that SpriteRec was put in the middle of another public struct
> making it impossible to extend; this new patch does not modify
> SpriteRec. It feels less hacky than adding a "fake" pointer
> ((intptr_t) 1)and the necessary checks all over mipointer.c.

There wouldn't be any changes to mipointer.c.

-- 
-keith


signature.asc
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xwayland 2/2 v2] xwayland: Always update the wl_pointer cursor on pointer focus

2015-09-29 Thread Jonas Ådahl
On Tue, Sep 29, 2015 at 10:04:40PM -0700, Keith Packard wrote:
> Jonas Ådahl  writes:
> 
> > On Tue, Sep 29, 2015 at 09:49:46AM -0700, Keith Packard wrote:
> >> Jonas Ådahl  writes:
> >> 
> >> > As mentioned, a different approach would be to work-around the issue in
> >> > xwayland, but I consider such a solution more hacky than this. Please let
> >> > me know what you think.
> >> 
> >> All you need do in xwayland is smash pSpriteCursor to some other value,
> >> like ((CursorPtr) (intptr_t) 1). That'll make the existing checks fail
> >> and force the driver to be called with the new cursor.
> >> 
> >> No mipointer changes ABI/API needed, which means we could merge this for
> >> 1.18 (given the DIX patch which I've already reviewed).
> >
> > The added field is in the end of the struct, and for what I can
> > understand from Adam running abidiff, the only remaining ABI issue was
> > the fact that SpriteRec was put in the middle of another public struct
> > making it impossible to extend; this new patch does not modify
> > SpriteRec. It feels less hacky than adding a "fake" pointer
> > ((intptr_t) 1)and the necessary checks all over mipointer.c.
> 
> There wouldn't be any changes to mipointer.c.

Yes, otherwise we'd crash in miPointerUpdateSprite when trying to access
pCursor->bits. After fixing that, we'd have to patch up the sprite
callbacks in xwayland to deal with the not-a-cursor cursor. It feels
hacky, but anyway, I'll make such a patch anyway if thats what you
prefer.


Jonas

> 
> -- 
> -keith


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xwayland 2/2 v2] xwayland: Always update the wl_pointer cursor on pointer focus

2015-09-30 Thread Keith Packard
Jonas Ådahl  writes:

> Yes, otherwise we'd crash in miPointerUpdateSprite when trying to access
> pCursor->bits.

pSpriteCursor, not pCursor.

-- 
-keith


signature.asc
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xwayland 2/2 v2] xwayland: Always update the wl_pointer cursor on pointer focus

2015-09-30 Thread Jonas Ådahl
On Wed, Sep 30, 2015 at 07:43:50AM -0700, Keith Packard wrote:
> Jonas Ådahl  writes:
> 
> > Yes, otherwise we'd crash in miPointerUpdateSprite when trying to access
> > pCursor->bits.
> 
> pSpriteCursor, not pCursor.

Ugh, poking in mipointers struct directly, ok. I'll have to expose the
MIPOINTER() macro. Is it fine to just move it to mipointer.h, or should
I introduce some DeviceIntPtr -> miPointerPtr function (which would need
some moving around of typedefs as well)?


Jonas

> 
> -- 
> -keith


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel