[PATCH 1/1] Bug 1300: SDL_GetWindowPosition / SDL_SetWindowPosition do not work from the same origin

Stefanos_A · October 20, 2013, 3:26pm

The following patch queries the border size of the window inside the
ConfigureNotify event, and adjusts the window coordinates accordingly
before passing them to the SDL event queue.

HG changeset patch

User Stefanos Apostolopoulos <@Stefanos_A>

Date 1382282623 -7200

Sun Oct 20 17:23:43 2013 +0200

Node ID 909b0d7fe4dd0af23585e750bd3396a0b53d2a6b

Parent 2f2f0b3b4702482cb47a98904fb1822080e60266

Fix bug 1300 by querying current border size in ConfigureNotify, and
adjusting window coordinates accordingly.

diff -r 2f2f0b3b4702 -r 909b0d7fe4dd src/video/x11/SDL_x11events.c
— a/src/video/x11/SDL_x11events.c Sat Oct 19 01:29:23 2013 -0700
+++ b/src/video/x11/SDL_x11events.c Sun Oct 20 17:23:43 2013 +0200
@@ -519,10 +519,32 @@
xevent.xconfigure.x, xevent.xconfigure.y,
xevent.xconfigure.width, xevent.xconfigure.height);
#endif

```
       long border_left = 0;
```
```
       long border_right = 0;
```
```
       long border_top = 0;
```
```
       long border_bottom = 0;
```
```
       if (data->xwindow) {
```

           Atom _net_frame_extents = XInternAtom(display,

“_NET_FRAME_EXTENTS”, 0);

```
           Atom type;
```
```
           int format;
```

           unsigned long nitems, bytes_after;

```
           unsigned char *property;
```

           XGetWindowProperty(display, data->xwindow,

               _net_frame_extents, 0, 16, 0,

               XA_CARDINAL, &type, &format,

               &nitems, &bytes_after, &property);+

           border_left = ((long*)property)[0];

           border_right = ((long*)property)[1];

           border_top = ((long*)property)[2];

           border_bottom = ((long*)property)[3];

```
       }
```

       if (xevent.xconfigure.x != data->last_xconfigure.x ||
           xevent.xconfigure.y != data->last_xconfigure.y) {
           SDL_SendWindowEvent(data->window, SDL_WINDOWEVENT_MOVED,

                               xevent.xconfigure.x, xevent.xconfigure.y);

                               xevent.xconfigure.x - border_left,

                               xevent.xconfigure.y - border_top);
       }
       if (xevent.xconfigure.width != data->last_xconfigure.width ||
           xevent.xconfigure.height != data->last_xconfigure.height) {

slouken · October 20, 2013, 4:54pm

Thanks for attaching the patch to the bug. It looks good!On Sun, Oct 20, 2013 at 8:26 AM, Stefanos A. wrote:

The following patch queries the border size of the window inside the
ConfigureNotify event, and adjusts the window coordinates accordingly
before passing them to the SDL event queue.

HG changeset patch

User Stefanos Apostolopoulos

Date 1382282623 -7200

Sun Oct 20 17:23:43 2013 +0200

Node ID 909b0d7fe4dd0af23585e750bd3396a0b53d2a6b

Parent 2f2f0b3b4702482cb47a98904fb1822080e60266

Fix bug 1300 by querying current border size in ConfigureNotify, and
adjusting window coordinates accordingly.

diff -r 2f2f0b3b4702 -r 909b0d7fe4dd src/video/x11/SDL_x11events.c
— a/src/video/x11/SDL_x11events.c Sat Oct 19 01:29:23 2013 -0700
+++ b/src/video/x11/SDL_x11events.c Sun Oct 20 17:23:43 2013 +0200
@@ -519,10 +519,32 @@
xevent.xconfigure.x, xevent.xconfigure.y,
xevent.xconfigure.width, xevent.xconfigure.height);
#endif
       long border_left = 0;
       long border_right = 0;
       long border_top = 0;
       long border_bottom = 0;
       if (data->xwindow) {
           Atom _net_frame_extents = XInternAtom(display,
“_NET_FRAME_EXTENTS”, 0);
           Atom type;
           int format;
           unsigned long nitems, bytes_after;
           unsigned char *property;
           XGetWindowProperty(display, data->xwindow,
               _net_frame_extents, 0, 16, 0,
               XA_CARDINAL, &type, &format,
               &nitems, &bytes_after, &property);
           border_left = ((long*)property)[0];
           border_right = ((long*)property)[1];
           border_top = ((long*)property)[2];
           border_bottom = ((long*)property)[3];
       }
       if (xevent.xconfigure.x != data->last_xconfigure.x ||
           xevent.xconfigure.y != data->last_xconfigure.y) {
           SDL_SendWindowEvent(data->window, SDL_WINDOWEVENT_MOVED,
                               xevent.xconfigure.x,
xevent.xconfigure.y);
                               xevent.xconfigure.x - border_left,
                               xevent.xconfigure.y - border_top);
       }
       if (xevent.xconfigure.width != data->last_xconfigure.width ||
           xevent.xconfigure.height != data->last_xconfigure.height)
{

SDL mailing list
SDL at lists.libsdl.org
http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org

Stefanos_A · October 21, 2013, 12:25pm

Can someone with a recent kde/kwin installation please give this a
try? I have verified that this works as expected on gnome 3 and unity,
but my arch+kde vm just went bust during an update and I cannot
currently test there.

2013/10/20 Sam Lantinga :> Thanks for attaching the patch to the bug. It looks good!

On Sun, Oct 20, 2013 at 8:26 AM, Stefanos A. <@Stefanos_A> wrote:
The following patch queries the border size of the window inside the
ConfigureNotify event, and adjusts the window coordinates accordingly
before passing them to the SDL event queue.

HG changeset patch

User Stefanos Apostolopoulos <@Stefanos_A>

Date 1382282623 -7200

Sun Oct 20 17:23:43 2013 +0200

Node ID 909b0d7fe4dd0af23585e750bd3396a0b53d2a6b

Parent 2f2f0b3b4702482cb47a98904fb1822080e60266

Fix bug 1300 by querying current border size in ConfigureNotify, and
adjusting window coordinates accordingly.

diff -r 2f2f0b3b4702 -r 909b0d7fe4dd src/video/x11/SDL_x11events.c
— a/src/video/x11/SDL_x11events.c Sat Oct 19 01:29:23 2013 -0700
+++ b/src/video/x11/SDL_x11events.c Sun Oct 20 17:23:43 2013 +0200
@@ -519,10 +519,32 @@
xevent.xconfigure.x, xevent.xconfigure.y,
xevent.xconfigure.width, xevent.xconfigure.height);
#endif
       long border_left = 0;
       long border_right = 0;
       long border_top = 0;
       long border_bottom = 0;
       if (data->xwindow) {
           Atom _net_frame_extents = XInternAtom(display,
“_NET_FRAME_EXTENTS”, 0);
           Atom type;
           int format;
           unsigned long nitems, bytes_after;
           unsigned char *property;
           XGetWindowProperty(display, data->xwindow,
               _net_frame_extents, 0, 16, 0,
               XA_CARDINAL, &type, &format,
               &nitems, &bytes_after, &property);
           border_left = ((long*)property)[0];
           border_right = ((long*)property)[1];
           border_top = ((long*)property)[2];
           border_bottom = ((long*)property)[3];
       }
       if (xevent.xconfigure.x != data->last_xconfigure.x ||
           xevent.xconfigure.y != data->last_xconfigure.y) {
           SDL_SendWindowEvent(data->window, SDL_WINDOWEVENT_MOVED,
                               xevent.xconfigure.x,
xevent.xconfigure.y);
                               xevent.xconfigure.x - border_left,
                               xevent.xconfigure.y - border_top);
       }
       if (xevent.xconfigure.width != data->last_xconfigure.width ||
           xevent.xconfigure.height != data->last_xconfigure.height)
{

SDL mailing list
SDL at lists.libsdl.org
http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org
SDL mailing list
SDL at lists.libsdl.org
http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org

Gabriele_Greco · October 21, 2013, 1:00pm

It seems to me that this patch cause a memory leak since it misses an
XFree() to free the data allocated in property by XGetWindowProperty, at
least the X11 documentation I found here reports that XFree() is needed:

http://tronche.com/gui/x/xlib/window-information/XGetWindowProperty.html

Maybe that some error checking around XGetWindowProperty and an XFree
should do the job:

if (XGetWindowProperty(display, data->xwindow,
_net_frame_extents, 0, 16, 0,
XA_CARDINAL, &type, &format,
&nitems, &bytes_after, &property) == Success) {
border_left = ((long*)property)[0];
border_right = ((long*)property)[1];
border_top = ((long*)property)[2];
border_bottom = ((long*)property)[3];
XFree(property);
}

BTW: I’m not an expert of the X11 API :)On Mon, Oct 21, 2013 at 2:25 PM, Stefanos A. wrote:

Can someone with a recent kde/kwin installation please give this a
try? I have verified that this works as expected on gnome 3 and unity,
but my arch+kde vm just went bust during an update and I cannot
currently test there.

2013/10/20 Sam Lantinga :
Thanks for attaching the patch to the bug. It looks good!

On Sun, Oct 20, 2013 at 8:26 AM, Stefanos A. wrote:
The following patch queries the border size of the window inside the
ConfigureNotify event, and adjusts the window coordinates accordingly
before passing them to the SDL event queue.

HG changeset patch

User Stefanos Apostolopoulos

Date 1382282623 -7200

Sun Oct 20 17:23:43 2013 +0200

Node ID 909b0d7fe4dd0af23585e750bd3396a0b53d2a6b

Parent 2f2f0b3b4702482cb47a98904fb1822080e60266

Fix bug 1300 by querying current border size in ConfigureNotify, and
adjusting window coordinates accordingly.

diff -r 2f2f0b3b4702 -r 909b0d7fe4dd src/video/x11/SDL_x11events.c
— a/src/video/x11/SDL_x11events.c Sat Oct 19 01:29:23 2013 -0700
+++ b/src/video/x11/SDL_x11events.c Sun Oct 20 17:23:43 2013 +0200
@@ -519,10 +519,32 @@
xevent.xconfigure.x, xevent.xconfigure.y,
xevent.xconfigure.width, xevent.xconfigure.height);
#endif
       long border_left = 0;
       long border_right = 0;
       long border_top = 0;
       long border_bottom = 0;
       if (data->xwindow) {
           Atom _net_frame_extents = XInternAtom(display,
“_NET_FRAME_EXTENTS”, 0);
           Atom type;
           int format;
           unsigned long nitems, bytes_after;
           unsigned char *property;
           XGetWindowProperty(display, data->xwindow,
               _net_frame_extents, 0, 16, 0,
               XA_CARDINAL, &type, &format,
               &nitems, &bytes_after, &property);
           border_left = ((long*)property)[0];
           border_right = ((long*)property)[1];
           border_top = ((long*)property)[2];
           border_bottom = ((long*)property)[3];
       }
       if (xevent.xconfigure.x != data->last_xconfigure.x ||
           xevent.xconfigure.y != data->last_xconfigure.y) {
           SDL_SendWindowEvent(data->window,
SDL_WINDOWEVENT_MOVED,
                               xevent.xconfigure.x,
xevent.xconfigure.y);
                               xevent.xconfigure.x - border_left,
                               xevent.xconfigure.y - border_top);
       }
       if (xevent.xconfigure.width != data->last_xconfigure.width
||
             xevent.xconfigure.height !=
data->last_xconfigure.height)

{

SDL mailing list
SDL at lists.libsdl.org
http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org

SDL mailing list
SDL at lists.libsdl.org
http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org

SDL mailing list
SDL at lists.libsdl.org
http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org

–
Ing. Gabriele Greco, DARTS Engineering
Tel: +39-0100980150 Fax: +39-0100980184
s-mail: Piazza Della Vittoria 9/3 - 16121 GENOVA (ITALY)

Stefanos_A · October 21, 2013, 3:46pm

My understanding is that XInternAtom returns an existing atom if the
last parameter is 0 (i.e. only allocates on first call), but the
documentation here is quite cryptic. There is one more call to
XInternAtom in this file and it doesn’t call XFree either, so I based
my patch on that.

If XFree is required, then the atom should be created/freed on
SDL_Init/Quit respectively.

You are right about error checking, I will respin a patch shortly.

2013/10/21 Gabriele Greco <gabriele.greco at darts.it>:> It seems to me that this patch cause a memory leak since it misses an

XFree() to free the data allocated in property by XGetWindowProperty, at
least the X11 documentation I found here reports that XFree() is needed:

http://tronche.com/gui/x/xlib/window-information/XGetWindowProperty.html

Maybe that some error checking around XGetWindowProperty and an XFree should
do the job:

if (XGetWindowProperty(display, data->xwindow,
_net_frame_extents, 0, 16, 0,
XA_CARDINAL, &type, &format,
&nitems, &bytes_after, &property) == Success) {
border_left = ((long*)property)[0];
border_right = ((long*)property)[1];
border_top = ((long*)property)[2];
border_bottom = ((long*)property)[3];
XFree(property);
}

BTW: I’m not an expert of the X11 API

On Mon, Oct 21, 2013 at 2:25 PM, Stefanos A. <@Stefanos_A> wrote:
Can someone with a recent kde/kwin installation please give this a
try? I have verified that this works as expected on gnome 3 and unity,
but my arch+kde vm just went bust during an update and I cannot
currently test there.

2013/10/20 Sam Lantinga :
Thanks for attaching the patch to the bug. It looks good!

On Sun, Oct 20, 2013 at 8:26 AM, Stefanos A. <@Stefanos_A> wrote:
The following patch queries the border size of the window inside the
ConfigureNotify event, and adjusts the window coordinates accordingly
before passing them to the SDL event queue.

HG changeset patch

User Stefanos Apostolopoulos <@Stefanos_A>

Date 1382282623 -7200

Sun Oct 20 17:23:43 2013 +0200

Node ID 909b0d7fe4dd0af23585e750bd3396a0b53d2a6b

Parent 2f2f0b3b4702482cb47a98904fb1822080e60266

Fix bug 1300 by querying current border size in ConfigureNotify, and
adjusting window coordinates accordingly.

diff -r 2f2f0b3b4702 -r 909b0d7fe4dd src/video/x11/SDL_x11events.c
— a/src/video/x11/SDL_x11events.c Sat Oct 19 01:29:23 2013 -0700
+++ b/src/video/x11/SDL_x11events.c Sun Oct 20 17:23:43 2013 +0200
@@ -519,10 +519,32 @@
xevent.xconfigure.x, xevent.xconfigure.y,
xevent.xconfigure.width, xevent.xconfigure.height);
#endif
       long border_left = 0;
       long border_right = 0;
       long border_top = 0;
       long border_bottom = 0;
       if (data->xwindow) {
           Atom _net_frame_extents = XInternAtom(display,
“_NET_FRAME_EXTENTS”, 0);
           Atom type;
           int format;
           unsigned long nitems, bytes_after;
           unsigned char *property;
           XGetWindowProperty(display, data->xwindow,
               _net_frame_extents, 0, 16, 0,
               XA_CARDINAL, &type, &format,
               &nitems, &bytes_after, &property);
           border_left = ((long*)property)[0];
           border_right = ((long*)property)[1];
           border_top = ((long*)property)[2];
           border_bottom = ((long*)property)[3];
       }
       if (xevent.xconfigure.x != data->last_xconfigure.x ||
           xevent.xconfigure.y != data->last_xconfigure.y) {
           SDL_SendWindowEvent(data->window,
SDL_WINDOWEVENT_MOVED,
                               xevent.xconfigure.x,
xevent.xconfigure.y);
                               xevent.xconfigure.x - border_left,
                               xevent.xconfigure.y - border_top);
       }
       if (xevent.xconfigure.width != data->last_xconfigure.width
||
xevent.xconfigure.height !=
data->last_xconfigure.height)
{

SDL mailing list
SDL at lists.libsdl.org
http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org
SDL mailing list
SDL at lists.libsdl.org
http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org
SDL mailing list
SDL at lists.libsdl.org
http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org
–
Ing. Gabriele Greco, DARTS Engineering
Tel: +39-0100980150 Fax: +39-0100980184
s-mail: Piazza Della Vittoria 9/3 - 16121 GENOVA (ITALY)

SDL mailing list
SDL at lists.libsdl.org
http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org