On 2012-10-09 01:08, Christophe wrote:
Hi Rodolfo,
While having a quick look at this patch, I noticed something that
looks plain wrong to me in the original code:
----- Rodolfo García Peñas <[email protected]> a écrit :
From a280e1aa0fe1e2500f6f687b7b195cbb2bca9ba2 Mon Sep 17 00:00:00
2001
From: =?UTF-8?q?"Rodolfo=20Garc=C3=ADa=20Pe=C3=B1as=20(kix)"?=
<[email protected]>
Date: Sat, 6 Oct 2012 18:18:43 +0200
Subject: [PATCH 2/3] wcore set vmask in one step
[...]
- vmask = CWBorderPixel | CWCursor | CWEventMask |
CWOverrideRedirect;
+ vmask = CWBorderPixel | CWCursor | CWEventMask |
CWOverrideRedirect | CWColormap;
attribs.override_redirect = True;
attribs.cursor = wCursor[WCUR_DEFAULT];
attribs.background_pixmap = None;
the attribs 'background_pixmap' and 'background_pixel' but the
corresponding flags (CWBackPixmap and CWBackPixel) are not set (but
that's not really a problem).
@@ -99,14 +98,13 @@ WCoreWindow *wCoreCreate(WCoreWindow *parent,
int x, int y, int width, int heigh
- vmask = CWBorderPixel | CWCursor | CWEventMask;
+ vmask = CWBorderPixel | CWCursor | CWEventMask | CWColormap;
attribs.cursor = wCursor[WCUR_DEFAULT];
attribs.background_pixmap = None;
attribs.background_pixel = parent->screen_ptr->black_pixel;
attribs.event_mask = KeyPressMask | KeyReleaseMask |
ButtonPressMask |
ButtonReleaseMask | ButtonMotionMask |
ExposureMask | EnterWindowMask | LeaveWindowMask;
- vmask |= CWColormap;
attribs.colormap = parent->screen_ptr->w_colormap;
This time it's a bit worse, because it uses flag 'CWBorderPixel' but
does not set the field 'border_pixel'. It also sets fields
'background_*' with no use as the corresponding flags are missing.
Probably no risk of crash, just a wrong border color on some (old)
display.
Regards,
Christophe.
Hi Christophe,
thanks for your review. I have a doubt about your comments.
In the original code:
1. Set vmask
2. Set attribs (and attribs flags)
3. vmask changed (|=)
But vmask and attribs (and the flags) are not used yet (only assigned).
So, we can do (the patch):
1. Set vmask (|= included)
2. Set attribs.
I cannot see the problem :-( I am missing something?
Thanks!
kix
--
||// //\\// Rodolfo "kix" Garcia
||\\// //\\ http://www.kix.es/
--
To unsubscribe, send mail to [email protected].
