----- Rodolfo García Peñas (kix) <[email protected]> a écrit : > 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?
Hi! No, you're not missing anything! Your patch is 100% correct, the problem is in some code that you have not changed, I just saw it because it was visible through your patch. I'm sending a patch to fix it so you'll be able to judge by yourself. Best regards, Christophe. > > Thanks! > kix > > -- > ||// //\\// Rodolfo "kix" Garcia > ||\\// //\\ http://www.kix.es/ > > > -- > To unsubscribe, send mail to [email protected]. -- To unsubscribe, send mail to [email protected].
