Re: [PATCH] Remove XAA
Hello, > I love this. It invalidates some of my outstanding patches in the best > possible way. But... could you explain (preferably in the commit > message) what evidence you have that "it hasn't worked"? I thought > people were still using XAA. Indeed, I have just now tested XAA vs EXA on the Lemote Yeeloong[1] (siliconmotion). In gtkperf, XAA is good 20% faster than EXA here. With the removal of XAA, what do you guys suggest us to do? "Staying on an old version of Xorg forever" doesn't count as a solution. Thanks. [1] "The Lemote Yeeloong is the most free software / open source friendly portable computer ever produced. All of the software that runs on it - even the BIOS (the software that runs before your operating system loads) is open for anyone who would like to improve or replace it. All of the features of the hardware are usable without any binary blobs - binaries that make it impossible for the community to improve upon or make innovative use of hardware, such as wifi, as is the case with the vast majority of computers." http://www.amazon.com/Screen-Lemote-Yeeloong-8089_B-Netbook/dp/B005SYBJYE/ -- With respect, Roman ~~~ "Stallman had a printer, with code he could not see. So he began to tinker, and set the software free." 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
[PATCH] fixesproto v6: Pointer barrier thresholds
From: Christopher James Halse Rogers --- This is prompted by a request from the Ubuntu DX team to conditionally restrict pointer motion at a screen edge. The Unity launcher appears on a screen edge, triggered by pointer proximity, and the objective is to have a form of edge resistance so that it is easy to summon the launcher on the border of two displays while still allowing determined pointer motion through. There doesn't seem to be a good way to implement this client-side. Pointer barriers seem the obvious mechanism, but turning them on and off runs into roundtrip latency, and there's some useful information that the server has that's not easily available to a client. This is the second go at a protocol. The first simply added a velocity-gated pointer barrier request. In testing this suboptimal behaviour - getting the pointer past the barrier required too vigourous a motion, and it was desired that the barrier would also respond to a period of gentler motion in addition to being transparent to sufficiently fast movement. I'm aware that this is becoming a bit elaborate. I'd like some guidance - I don't want to make something that's pointlessly generic, but it should be useful outside of Unity's specific usecase. fixesproto.txt | 118 1 files changed, 118 insertions(+), 0 deletions(-) diff --git a/fixesproto.txt b/fixesproto.txt index 5903ac9..16a7aae 100644 --- a/fixesproto.txt +++ b/fixesproto.txt @@ -650,6 +650,124 @@ DestroyPointerBarrier Errors: Barrier +* XFIXES VERSION 6 OR BETTER *** + +13. Pointer Barriers Expansion + +This update extends pointer barriers to optionally allow the pointer through +when a threshold is reached. This can be useful for desktop environments that +wish to use a large region of the screen, such as an entire edge, to provide a +casual target while allowing determined movement to pass through. + +13.1 Types + + BarrierThreshold: XID + BarrierThresholdKind: {None, Distance, Velocity, HitCount} + BarrierEvent: {Hit, ThresholdExceeded} + +13.2 Errors + + BarrierThreshold + +13.3 Events + +BarrierNotify + + subtype:BarrierEvent + window: WINDOW + timestamp: Timestamp + barrier:BARRIER + threshold: BARRIERTHRESHOLD or None + value: CARD32 + x, y: INT16 + +13.4 Requests + +SelectBarrierInput + + window: WINDOW + event-mask: SETofBarrierEvent + + + This request directs barrier events to the named window. Subtype + indicates the trigger of the event, which is Hit when the barrier has + prevented pointer movement and ThresholdExceeded when the barrier has + been hit but has not prevented pointer movement due to the threshold + being exceeded. + + Barrier is the barrier on which the event was triggered, which is + associated with threshold or None if no threshold is associated. + If threshold is not None, value contains the current value of the + property the threshold measures against. + + (x,y) contain the coordinates of the pointer after restriction by any + applicable barriers. + + In the case of multiple overlapping barriers an event is sent for each. + +CreateBarrierThreshold + + threshold: BARRIERTHRESHOLD + type: BarrierThresholdKind + value: CARD32 + + This request creates a new barrier threshold. This threshold + specifies a condition under which a pointer barrier will allow a + pointer to pass through unimpeded. + + Type can be None, in which case the threshold is never triggered. + + Type can be Distance, in which case the threshold will trigger if the + user attempts to move the pointer a cumulative distance in px greater + than value. The distance is not automatically reset, so once the + threshold has been triggered it will remain triggered. + + Type can be Velocity, in which case the threshold will trigger if the + instantaneous velocity of the pointer, perpendicular to the barrier, + in px/sec is greater than value. + + Type can be HitCount, in which case the threshold will trigger once the + barrier has been hit value times. The count is not automatically + reset, so once the threshold has been triggered it will remain + triggered. + + If type is not one of these values, BadValue is generated. + + Errors: IDChoice, Value + +AddBarrierThresholdOr +AddBarrierThresholdAnd + + threshold: BARRIERTHRESHOLD + type:
Re: [PATCH 1.12] A coding style for the server
On Thu, 19 Jan 2012 15:49:06 +1100, Daniel Stone wrote: > Actually, non-cuddling else was slightly in the majority -- this was > just based on a quick couple of greps, as was the decision to cuddle > if/for/while statements with their opening greps. For full > disclosure, I prefer it this way, but was pleasantly surprised to see > the numbers agree with my preference. If the numbers didn't agree, I > would've sucked it up and gone the other way. I looked at some older X server code (X11R1) and found that it used this form: if () { } else { } almost exclusively, except for do, which was done as: do { } while(); Even switch shows a marked preference for this form: switch () { case ..: } However, about 1/3 of the switch cases use switch () { instead. What I can't figure out is how to get indent to format declarations the way the original X server mi code did: static void miComputeClips (pParent, pScreen, universe) register WindowPtr pParent; register ScreenPtr pScreen; register RegionPtr universe; { RegionPtr childUniverse; register WindowPtr pChild; int oldVis; BoxPtr borderSize; All of the names are nicely lined up. Here's another longer one (the code is completely broken, of course, as it doesn't follow X pixelization rules, but...): void miFillSppPoly(dst, pgc, count, ptsIn, xTrans, yTrans) DrawablePtr dst; GCPtr pgc; int count; /* number of points */ SppPointPtr ptsIn; /* the points */ int xTrans, yTrans; /* Translate each point by this */ { double xl, xr, /* x vals of left and right edges */ ml, /* left edge slope */ mr, /* right edge slope */ dy, /* delta y */ i; /* loop counter */ int y, /* current scanline */ j, imin, /* index of vertex with smallest y */ ymin, /* y-extents of polygon */ ymax, *width, *FirstWidth,/* output buffer */ *Marked;/* set if this vertex has been used */ register intleft, right,/* indices to first endpoints */ nextleft, nextright; /* indices to second endpoints */ DDXPointPtr ptsOut, FirstPoint; /* output buffer */ double ceil(); -- keith.pack...@intel.com pgpNkBCOxqzRw.pgp 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] Remove XAA
On Thu, Jan 19, 2012 at 03:43:39PM +1100, Daniel Stone wrote: > On 19 January 2012 04:03, Jamey Sharp wrote: > > I love this. It invalidates some of my outstanding patches in the best > > possible way. But... could you explain (preferably in the commit > > message) what evidence you have that "it hasn't worked"? I thought > > people were still using XAA. > > Ah, fair point. I guess it wasn't _completely_ non-functional as > such, but almost entirely so; the only time it'd be useful is if you > were playing xtank in a non-composited environment. Here's the > updated commit message: Much better, thanks! Makes sense to me. I seem to recall Nvidia was loading the XAA module because it was the only entry point they could use to get at some symbol, but they don't actually use the acceleration architecture, because that would be crazy. If I have that right, I imagine we'll get some discussion about that when Aaron notices this thread. I haven't reviewed the patch, but I approve in principle. Is that an acked-by? Feel free to add one, or not. Jamey signature.asc Description: Digital 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 1.12] A coding style for the server
Hi, On 19 January 2012 10:25, Guillem Jover wrote: > On Wed, 2012-01-18 at 14:02:01 +1100, Daniel Stone wrote: >> So, the commit in my coding-style xserver branch is simply the result of >> running this: >> indent -linux -bad -bap -blf -bli0 -br -brs -cbi0 -cdw -nce -cs -i4 -hnl >> -l80 -lc80 -lp -nbbo -nbc -nbfda -nprs -npcs -npsl -saf -sai -saw -nut >> across *.c and *.h in the X server tree. No more, no less. The build >> seems to continue to work just fine. Don't be scared by the -linux; >> it just provides a set of reasonable default options. Pretty much >> everything else was just done by hand to match what we seem to generally >> do in the server. > > I had the impression from skimming over xorg code base that most of it > was using -psl? Yeah, that's my rough impression too; I'll add that for v2. (Even though it is a pretty lame alternative to just using cscope.) >> It's obviously too big to attach to this mail, so I haven't done that. > > It seems indent is not smart enough and produces some space damage. It > does not seem to be able to handle some unary pointer operators correctly > and thinks they are binary ones appending spaces after them. It also > seems to confuse function pointers with casts placing spaces before the > opening parenthesis. So any such patch might need manual review. I have to admit that I didn't read the entire patch ... > Also, defining a global coding style does not mean the code has to switch > immediately, it could be switched as code gets modfied, which avoids the > «git blame» barrier already mentioned on the thread if that's considered > to be an issue. > > In any case a global coding style would be extremely nice. Hmm, at some point you will have to have a flag day though, to switch everything that hasn't yet been fixed. I don't think it really buys us much. Cheers, Daniel ___ 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 1.12] A coding style for the server
Hi, On 19 January 2012 06:42, Chase Douglas wrote: > Part of the problem of a coding style is the ongoing maintenance. The > kernel handles this through the check_patch script. This gives rise to > the following questions: > > 1. Should we just go with linux style, so we can use check_patch without > any modifications? No - the main problem is that the kernel uses tabs with ts=8 rather than four-space indents. While I'm not opposed to it on principle, it only works for them because they're so aggressive about keeping functions small and using early return/break-of-control rather than deeply nested ifs. Our code would be utterly, utterly unreadable if formatted to ts=8 while retaining the wrapping at 80 characters. > 2. If not, will we fork check_patch or make our own? > > 3. Who will enforce style? I believe if we have a script we can add a > commit hook to the git server, but that may be too heavy-handed. Yeah - maybe it's just something we should try to catch during the review process, rather than enforce? > 4. If no one will enforce style, do we want to periodically run indent > to fix things. Maybe once per cycle after the merge window closes (as > you suggest here, but I begged for mercy against this time :)? Heh, sure. :) Cheers, Daniel ___ 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 1.12] A coding style for the server
Hi, On 19 January 2012 02:08, James Cloos wrote: > A consistent coding style is a Good Thing™. > > Trevor’s comments should be addressed. > > The final indent command should be documented not only in the commit log > but also in a README file. That should help encourage those who submit > patches based on the tar releases also to maintain the resulting coding > style. > > Emacs and vim cookies matching the chosen style would be most useful. Good point, I'll fix all of the above up for a next commit. Cheers, Daniel ___ 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 1.12] A coding style for the server
Hi, On 19 January 2012 01:11, Trevor Woerner wrote: > On Tue, Jan 17, 2012 at 10:02 PM, Daniel Stone wrote: >> indent -linux -bad -bap -blf -bli0 -br -brs -cbi0 -cdw -nce -cs -i4 -hnl >> -l80 -lc80 -lp -nbbo -nbc -nbfda -nprs -npcs -npsl -saf -sai -saw -nut > > About a dozen of the options you specify are already included in the > -linux option. As such your list only needs to include: > > -linux -bad -blf -bli0 -cbi0 -cdw -nce -cs -i4 -lc80 -nbbo -nbc -nbfda -nut > > In other words the following are already included in -linux: > > -bap -br -brs -hnl -l80 -lp -nprs -npcs -npsl -saf -sai -saw Good looking out, thanks. > From reading the man page it appears the -nbbo (no break after > boolean) option conflicts with the -hnl (honour newlines) option such > that -hnl cancels -nbbo, therefore -nbbo is ignored. Ah! I think just removing -hnl and retaining -nbbo is the right thing to do here. > Personally I don't like it when a developer submits a patch that > intermixes code changes with formatting changes. As such I'm not fond > of the -lp (continue at parenthesis) option because it has the > side-effect of encouraging this behaviour. If, for example, a given > function has 5 arguments and the code is refactored to change the name > of the function such that the new name is a different length from the > previous name, the patch will then contain the changed line plus 4 > lines of formatting changes to keep the arguments "lined up". Nod, it is unfortunate. I don't think it's too much of a problem though, as if you're changing the names of several functions in one go, you should perhaps take it as a hint to further split your patch. :) > You mentioned that cuddling the else slightly outnumbered not cuddling > (was there a vote that I missed, or are you basing this on an > examination of the existing code?). However you then provide the -nce > (don't cuddle else) option. Personally I prefer -nce, but if you'd > like to go with the majority then -ce should be used instead. Actually, non-cuddling else was slightly in the majority -- this was just based on a quick couple of greps, as was the decision to cuddle if/for/while statements with their opening greps. For full disclosure, I prefer it this way, but was pleasantly surprised to see the numbers agree with my preference. If the numbers didn't agree, I would've sucked it up and gone the other way. Cheers, Daniel ___ 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] Remove XAA
Hi, On 19 January 2012 04:03, Jamey Sharp wrote: > I love this. It invalidates some of my outstanding patches in the best > possible way. But... could you explain (preferably in the commit > message) what evidence you have that "it hasn't worked"? I thought > people were still using XAA. Ah, fair point. I guess it wasn't _completely_ non-functional as such, but almost entirely so; the only time it'd be useful is if you were playing xtank in a non-composited environment. Here's the updated commit message: commit eb422cd841f188a42520492dea01fef960372ed5 Author: Daniel Stone Date: Wed Jan 18 15:01:09 2012 +1100 Remove XAA Commit 0c6987df in June 2008 disabled XAA offscreen pixmaps per default, as they were broken, leaving XAA only able to accelerate operations directly on the screen pixmap and nowhere else, eliminating acceleration for basically every modern toolkit, and any composited environment. So, it hasn't worked for almost four years. No-one's even come close to fixing it. RIP. Signed-off-by: Daniel Stone Reviewed-by: Dave Airlie Reviewed-by: Alex Deucher Cheers, Daniel ___ 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
[PATCH 2/2] Don't set X and Y valuators for indirect touch events
For expediency, it made sense to always have the X and Y axes set for direct touch device event propagation. The last X and Y values are stored internally. However, indirect device touch event propagation does not depend on the touch's X and Y values. Thus, we don't need to set the values for every indirect touch event. On top of this, the previous X and Y values aren't stored for indirect touches, so without this change the axes get erroneously set to 0. Signed-off-by: Chase Douglas --- dix/getevents.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/dix/getevents.c b/dix/getevents.c index 3e37910..546b5a8 100644 --- a/dix/getevents.c +++ b/dix/getevents.c @@ -1842,7 +1842,7 @@ GetTouchEvents(InternalEvent *events, DeviceIntPtr dev, uint32_t ddx_touchid, default: return 0; } -if (!(flags & TOUCH_CLIENT_ID)) +if (t->mode == XIDirectTouch && !(flags & TOUCH_CLIENT_ID)) { if (!valuator_mask_isset(&mask, 0)) valuator_mask_set_double(&mask, 0, valuator_mask_get_double(touchpoint.ti->valuators, 0)); -- 1.7.8.3 ___ 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
[PATCH 1/2] Treat all touch event valuators as absolute
An indirect touch device, such as a multitouch touchpad, has relative X and Y axes internally. These axes are in screen coordinates. However, the cooresponding axes for touch events are in absolute device coordinates. Signed-off-by: Chase Douglas --- dix/getevents.c | 12 +--- 1 files changed, 9 insertions(+), 3 deletions(-) diff --git a/dix/getevents.c b/dix/getevents.c index 1547059..3e37910 100644 --- a/dix/getevents.c +++ b/dix/getevents.c @@ -222,17 +222,23 @@ set_valuators(DeviceIntPtr dev, DeviceEvent* event, ValuatorMask *mask) int i; /* Set the data to the previous value for unset absolute axes. The values - * may be used when sent as part of an XI 1.x valuator event. */ + * may be used when sent as part of an XI 1.x valuator event. + * + * All touch event valuators are absolute, even if the corresponding pointer + * valuator is relative. This is the case for indirect touch devices for the + * X and Y axes. */ for (i = 0; i < valuator_mask_size(mask); i++) { if (valuator_mask_isset(mask, i)) { SetBit(event->valuators.mask, i); -if (valuator_get_mode(dev, i) == Absolute) +if (IsTouchEvent((InternalEvent *)event) || +valuator_get_mode(dev, i) == Absolute) SetBit(event->valuators.mode, i); event->valuators.data[i] = valuator_mask_get_double(mask, i); } -else if (valuator_get_mode(dev, i) == Absolute) +else if (IsTouchEvent((InternalEvent *)event) || + valuator_get_mode(dev, i) == Absolute) event->valuators.data[i] = dev->valuator->axisVal[i]; } } -- 1.7.8.3 ___ 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
[PATCH 2/2] Only update pointer motion data for pointer emulated touch events
Signed-off-by: Chase Douglas --- dix/getevents.c |8 ++-- 1 files changed, 6 insertions(+), 2 deletions(-) diff --git a/dix/getevents.c b/dix/getevents.c index 2946b16..1547059 100644 --- a/dix/getevents.c +++ b/dix/getevents.c @@ -1878,9 +1878,13 @@ GetTouchEvents(InternalEvent *events, DeviceIntPtr dev, uint32_t ddx_touchid, &devx, &devy, &screenx, &screeny); /* see fill_pointer_events for coordinate systems */ -updateHistory(dev, &mask, ms); +if (emulate_pointer) +updateHistory(dev, &mask, ms); + clipValuators(dev, &mask); -storeLastValuators(dev, &mask, 0, 1, devx, devy); + +if (emulate_pointer) +storeLastValuators(dev, &mask, 0, 1, devx, devy); event->root = scr->root->drawable.id; -- 1.7.8.3 ___ 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
[PATCH 1/2] Only scale direct device touch coordinates
Indirect touch devices provide valuator values in pure device coordinates. They also don't need to be fixed up for screen crossings. Signed-off-by: Chase Douglas --- dix/getevents.c |7 ++- 1 files changed, 6 insertions(+), 1 deletions(-) diff --git a/dix/getevents.c b/dix/getevents.c index d0014e6..2946b16 100644 --- a/dix/getevents.c +++ b/dix/getevents.c @@ -1867,7 +1867,12 @@ GetTouchEvents(InternalEvent *events, DeviceIntPtr dev, uint32_t ddx_touchid, if (need_rawevent) set_raw_valuators(raw, &mask, raw->valuators.data); -scr = scale_to_desktop(dev, &mask, &devx, &devy, &screenx, &screeny); +/* Indirect device touch coordinates are not used for cursor positioning. + * They are merely informational, and are provided in device coordinates. + * The device sprite is used for positioning instead, and it is already + * scaled. */ +if (t->mode == XIDirectTouch) +scr = scale_to_desktop(dev, &mask, &devx, &devy, &screenx, &screeny); if (emulate_pointer) scr = positionSprite(dev, Absolute, &mask, &devx, &devy, &screenx, &screeny); -- 1.7.8.3 ___ 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 modular] Per-component configure options
On 12-01-16 06:22 PM, Trevor Woerner wrote: > @@ -1009,9 +1017,15 @@ process_module_file() { > continue > fi > > - module=`echo $line | cut -d'/' -f1` > - component=`echo $line | cut -d'/' -f2` > - build $module $component > + module=`echo $line | cut -d' ' -f1 | cut -d'/' -f1` > + component=`echo $line | cut -d' ' -f1 | cut -d'/' -f2` > + confopts_check=`echo $line | cut -d' ' -f2-` What is the role of the dash following f2? There were none before. > + if [ "$module/$component" = "$confopts_check" ]; then > + confopts="" > + else > + confopts="$confopts_check" > + fi > + build $module "$component" "$confopts" > done <"$MODFILE" Are all the new quotes around "$component" required? There can be no spaces in the lines, e.g. app/xclock. The seperator is a space and a '/'. Many double quotes have been added throughout the script. ___ 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 1.12] A coding style for the server
On Wed, 18 Jan 2012 20:42:43 +0100, Chase Douglas wrote: > 1. Should we just go with linux style, so we can use check_patch without > any modifications? > > 2. If not, will we fork check_patch or make our own? I'd be good with either plan. I can also run check_patch on anything getting added to the server, with suitable nag mails for patches which don't pass. > 3. Who will enforce style? I believe if we have a script we can add a > commit hook to the git server, but that may be too heavy-handed. It wouldn't be hard for me to make that part of the regular patch merging process > 4. If no one will enforce style, do we want to periodically run indent > to fix things. Maybe once per cycle after the merge window closes (as > you suggest here, but I begged for mercy against this time :)? No, once will be painful enough. After that, all new patches would be required to follow the formatting conventions or not get accepted. Keeping the code correct at all times makes it easier to fix new code -- just re-indent all changed files. -- keith.pack...@intel.com pgpcvnfg4naSW.pgp 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 xf86-input-evdev 1/2] Re-indent: put '{' on new line
On Thu, Jan 19, 2012 at 01:23:19AM +0100, Chase Douglas wrote: > On 01/19/2012 01:21 AM, Peter Hutterer wrote: > > On Thu, Jan 12, 2012 at 04:00:32PM +0100, Chase Douglas wrote: > >> Signed-off-by: Chase Douglas > > > > did you run ident or something to get this? it doesn't apply cleanly but if > > it's just a ident line it'd be easy enough to reproduce. > > I started with master, so I don't know why it doesn't apply cleanly... I applied out-of-order, the force x/y patches went in since and they mess things up. my fault, sorry. for large-scale code formatting patches, it's usually a good idea to halt development for a bit so everyone can rebase. sorry, been preempted so I didn't get to this earlier. > I did it with a vim replace. Multiline statements are manually fixed. > > > also, given daniels' patch on the xserver, it might be a good idea to just > > use the same ident option as the server. > > Yeah, that's not a bad idea. However, that's still in flux. Would it be > better to skip this patch for now and refresh the original "copy > valuators" patch with the few trivial fixes? yeah, let's get the actual fix in and worry about ident later. I'll review the last patch for code only, then we can merge it once it applies again. Cheers, Peter ___ 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 xf86-input-evdev 1/2] Re-indent: put '{' on new line
On 01/19/2012 01:21 AM, Peter Hutterer wrote: > On Thu, Jan 12, 2012 at 04:00:32PM +0100, Chase Douglas wrote: >> Signed-off-by: Chase Douglas > > did you run ident or something to get this? it doesn't apply cleanly but if > it's just a ident line it'd be easy enough to reproduce. I started with master, so I don't know why it doesn't apply cleanly... I did it with a vim replace. Multiline statements are manually fixed. > also, given daniels' patch on the xserver, it might be a good idea to just > use the same ident option as the server. Yeah, that's not a bad idea. However, that's still in flux. Would it be better to skip this patch for now and refresh the original "copy valuators" patch with the few trivial fixes? -- Chase ___ 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 xf86-input-evdev 1/2] Re-indent: put '{' on new line
On Thu, Jan 12, 2012 at 04:00:32PM +0100, Chase Douglas wrote: > Signed-off-by: Chase Douglas did you run ident or something to get this? it doesn't apply cleanly but if it's just a ident line it'd be easy enough to reproduce. also, given daniels' patch on the xserver, it might be a good idea to just use the same ident option as the server. Cheers, Peter > --- > src/apple.c|9 +- > src/draglock.c | 57 +++--- > src/emuMB.c| 15 ++- > src/emuThird.c |3 +- > src/emuWheel.c | 54 ++--- > src/evdev.c| 339 > +--- > 6 files changed, 318 insertions(+), 159 deletions(-) > > diff --git a/src/apple.c b/src/apple.c > index 8e00a84..a6e72ef 100644 > --- a/src/apple.c > +++ b/src/apple.c > @@ -87,7 +87,8 @@ struct product_table > { > unsigned int vendor; > unsigned int product; > -} apple_keyboard_table[] = { > +} apple_keyboard_table[] = > +{ > { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_MINI_ANSI}, > { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_MINI_ISO}, > { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_MINI_JIS}, > @@ -220,7 +221,8 @@ static void set_fkeymode_property(InputInfoPtr pInfo, > enum fkeymode fkeymode) > return; > } > > -if (!prop_fkeymode) { > +if (!prop_fkeymode) > +{ > init = TRUE; > prop_fkeymode = MakeAtom(EVDEV_PROP_FUNCTION_KEYS, > strlen(EVDEV_PROP_FUNCTION_KEYS), TRUE); > } > @@ -251,7 +253,8 @@ EvdevAppleGetProperty (DeviceIntPtr dev, Atom property) > enum fkeymode fkeymode; > > fkeymode = get_fnmode(); > -if (fkeymode != pEvdev->fkeymode) { > +if (fkeymode != pEvdev->fkeymode) > +{ > /* set internal copy first, so we don't write to the file in > * SetProperty handler */ > pEvdev->fkeymode = fkeymode; > diff --git a/src/draglock.c b/src/draglock.c > index ac9d9c0..8156da8 100644 > --- a/src/draglock.c > +++ b/src/draglock.c > @@ -69,52 +69,64 @@ EvdevDragLockPreInit(InputInfoPtr pInfo) > next_num = option_string; > > /* Loop until we hit the end of our option string */ > -while (next_num != NULL) { > +while (next_num != NULL) > +{ > lock_button = 0; > meta_button = strtol(next_num, &end_str, 10); > > /* check to see if we found anything */ > -if (next_num != end_str) { > +if (next_num != end_str) > +{ > /* setup for the next number */ > next_num = end_str; > -} else { > +} else > +{ > /* we have nothing more to parse, drop out of the loop */ > next_num = NULL; > } > > /* Check for a button to lock if we have a meta button */ > -if (meta_button != 0 && next_num != NULL ) { > +if (meta_button != 0 && next_num != NULL ) > +{ > lock_button = strtol(next_num, &end_str, 10); > > /* check to see if we found anything */ > -if (next_num != end_str) { > +if (next_num != end_str) > +{ > /* setup for the next number */ > next_num = end_str; > -} else { > +} else > +{ > /* we have nothing more to parse, drop out of the loop */ > next_num = NULL; > } > } > > /* Ok, let the user know what we found on this look */ > -if (meta_button != 0) { > -if (lock_button == 0) { > -if (!pairs) { > +if (meta_button != 0) > +{ > +if (lock_button == 0) > +{ > +if (!pairs) > +{ > /* We only have a meta button */ > pEvdev->dragLock.meta = meta_button; > > xf86IDrvMsg(pInfo, X_CONFIG, "DragLockButtons : " > "%i as meta\n", meta_button); > -} else { > +} else > +{ > xf86IDrvMsg(pInfo, X_ERROR, "DragLockButtons : " > "Incomplete pair specifying button pairs > %s\n", > option_string); > } > -} else { > +} else > +{ > > /* Do bounds checking to make sure we don't crash */ > if ((meta_button <= EVDEV_MAXBUTTONS) && (meta_button >= 0 ) > && > -(lock_button <= EVDEV_MAXBUTTONS) && (lock_button >= 0)) > { > +(lock_button <= EVDEV_MAXBUTTONS) && (lock_button >= 0)) > +{ > > xf86IDrvMsg(pInfo, X_CONFIG, > "DragLockButtons : %i -> %i\n", > @@ -122,7 +134,8 @@ EvdevDragLockPreInit(InputInfoPtr pInfo) > > pEvdev->dragLock.lock_p
Re: [PATCH 1.12] A coding style for the server
Hi! On Wed, 2012-01-18 at 14:02:01 +1100, Daniel Stone wrote: > Most of the code we broadly agrees on most things. Some things were a > little more split; for instance, cuddling else was only just outnumbered > by non-cuddling else, and also braces cuddling if outnumbered > non-cuddling. > So, the commit in my coding-style xserver branch is simply the result of > running this: > indent -linux -bad -bap -blf -bli0 -br -brs -cbi0 -cdw -nce -cs -i4 -hnl > -l80 -lc80 -lp -nbbo -nbc -nbfda -nprs -npcs -npsl -saf -sai -saw -nut > across *.c and *.h in the X server tree. No more, no less. The build > seems to continue to work just fine. Don't be scared by the -linux; > it just provides a set of reasonable default options. Pretty much > everything else was just done by hand to match what we seem to generally > do in the server. I had the impression from skimming over xorg code base that most of it was using -psl? > It's obviously too big to attach to this mail, so I haven't done that. It seems indent is not smart enough and produces some space damage. It does not seem to be able to handle some unary pointer operators correctly and thinks they are binary ones appending spaces after them. It also seems to confuse function pointers with casts placing spaces before the opening parenthesis. So any such patch might need manual review. Also, defining a global coding style does not mean the code has to switch immediately, it could be switched as code gets modfied, which avoids the «git blame» barrier already mentioned on the thread if that's considered to be an issue. In any case a global coding style would be extremely nice. regards, guillem ___ 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:xload] Solaris: use getloadavg from libc instead of kstats
I forgot to mention, this assumes that on BSD's, the autoconf check for getloadavg will work and can safely replace the older check for "defined(BSD) && (BSD >= 199306)": +# getloadavg: 4.3BSD-Reno& later, glibc 2.2& later, Solaris 7& later +# BSD& GNU libc use, Solaris requires +AC_CHECK_FUNCS([getloadavg], [AC_CHECK_HEADERS([sys/loadavg.h])]) -#if defined(BSD)&& (BSD>= 199306) +#if defined(HAVE_GETLOADAVG) There's a lot more ancient cruft that could probably also be purged, but this was the bit that had a recently uncovered bug that broke us. -- -Alan Coopersmith-alan.coopersm...@oracle.com Oracle Solaris Platform Engineering: X Window System ___ 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
[PATCH:xload] Solaris: use getloadavg from libc instead of kstats
The simpler interface (based on the BSD function) has been in libc since Solaris 7, and avoids datasize bugs like the previous fix, so might as well use it. Purge all the other ancient Solaris & SunOS support variants as well. This does mean if you want to keep running xload on a Sun OS version from before 1998 you will need to use a branch of xload from before 2012 (such as the one included in those old releases). Signed-off-by: Alan Coopersmith --- configure.ac |6 ++- get_load.c | 110 +- 2 files changed, 13 insertions(+), 103 deletions(-) diff --git a/configure.ac b/configure.ac index 7c94abf..c33871c 100644 --- a/configure.ac +++ b/configure.ac @@ -28,6 +28,7 @@ AC_INIT([xload], [1.1.0], AC_CONFIG_MACRO_DIR([m4]) AC_CONFIG_SRCDIR([Makefile.am]) AC_CONFIG_HEADERS([config.h]) +AC_USE_SYSTEM_EXTENSIONS # Initialize Automake AM_INIT_AUTOMAKE([foreign dist-bzip2]) @@ -62,8 +63,9 @@ AM_CONDITIONAL(USE_GETTEXT, test "x$USE_GETTEXT" = "xyes") ### How to check load average on various OS'es: -# Solaris: libkstat -AC_CHECK_LIB([kstat], [kstat_open]) +# getloadavg: 4.3BSD-Reno & later, glibc 2.2 & later, Solaris 7 & later +# BSD & GNU libc use , Solaris requires +AC_CHECK_FUNCS([getloadavg], [AC_CHECK_HEADERS([sys/loadavg.h])]) # Checks for pkg-config packages PKG_CHECK_MODULES(XLOAD, xaw7 xmu xt x11 [xproto >= 7.0.17]) diff --git a/get_load.c b/get_load.c index 185a7fb..001bd4b 100644 --- a/get_load.c +++ b/get_load.c @@ -118,20 +118,6 @@ void GetLoadPoint( #include #endif -#ifdef sun -#include -#if !defined(HAVE_CONFIG_H) && defined(SVR4) -# define HAVE_LIBKSTAT 1 -#endif -#ifdef HAVE_LIBKSTAT -# include -# include -#elif defined(i386) && !defined(SVR4) -#include -#defineKVM_ROUTINES -#endif /* i386 */ -#endif - #ifdef CSRG_BASED #include #endif @@ -290,51 +276,6 @@ XtPointer call_data; /* pointer to (double) return value */ return; } #else /* not (SYSV && i386) */ -#ifdef KVM_ROUTINES -/* - * Sun 386i Code - abstracted to see the wood for the trees - */ - -static struct nlist nl[2]; -static kvm_t *kd; - -void -InitLoadPoint()/* Sun 386i version */ -{ -kd = kvm_open("/vmunix", NULL, NULL, O_RDONLY, "Load Widget"); -if (kd == (kvm_t *)0) { - xload_error("cannot get access to kernel address space", ""); -} - -nl[0].n_name = "avenrun"; -nl[1].n_name = NULL; - -if (kvm_nlist(kd, nl) != 0) { - xload_error("cannot get name list", ""); -} - -if (nl[0].n_value == 0) { - xload_error("Cannot find address for avenrun in the kernel\n", ""); -} -} - -/* ARGSUSED */ -void -GetLoadPoint( w, closure, call_data ) /* Sun 386i version */ -Widget w; /* unused */ -XtPointer closure; /* unused */ -XtPointer call_data; /* pointer to (double) return value */ -{ -double *loadavg = (double *)call_data; -long temp; - -if (kvm_read(kd, nl[0].n_value, (char *)&temp, sizeof (temp)) != - sizeof (temp)) { - xload_error("Kernel read error", ""); -} -*loadavg = (double)temp/FSCALE; -} -#else /* not KVM_ROUTINES */ #if defined(linux) || (defined(__FreeBSD_kernel__) && defined(__GLIBC__)) @@ -595,8 +536,11 @@ void GetLoadPoint( } #else /* not __bsdi__ */ -#if defined(BSD) && (BSD >= 199306) +#if defined(HAVE_GETLOADAVG) #include +#ifdef HAVE_SYS_LOADAVG_H +#include/* Solaris definition of getloadavg */ +#endif void InitLoadPoint() { @@ -613,37 +557,7 @@ void GetLoadPoint(w, closure, call_data) xload_error("couldn't obtain load average", ""); } -#else /* not BSD >= 199306 */ -#if defined(sun) && defined(HAVE_LIBKSTAT) - -static kstat_t *ksp; -static kstat_ctl_t *kc; - -void -InitLoadPoint(void) -{ - if ((kc = kstat_open()) == NULL) - xload_error("kstat_open failed:", strerror(errno)); - - if ((ksp = kstat_lookup(kc, "unix", 0, "system_misc")) == NULL) - xload_error("kstat_lookup failed:", strerror(errno)); -} - -void -GetLoadPoint(Widget w, XtPointer closure, XtPointer call_data) -{ - kstat_named_t *vp; - double *loadavg = (double *)call_data; - - if (kstat_read(kc, ksp, NULL) == -1) - xload_error("kstat_read failed:", strerror(errno)); - - if ((vp = kstat_data_lookup(ksp, "avenrun_1min")) == NULL) - xload_error("kstat_data_lookup failed:", strerror(errno)); - - *loadavg = (double)vp->value.ui32 / FSCALE; -} -#else /* not Solaris */ +#else /* not HAVE_GETLOADAVG */ #ifndef KMEM_FILE #define KMEM_FILE "/dev/kmem" @@ -692,10 +606,6 @@ GetLoadPoint(Widget w, XtPointer closure, XtPointer call_data) #endif #endif /* MOTOROLA */ -#if defined(sun) && defined(SVR4) -#define KERNEL_FILE "/kernel/unix" -#endif - #ifdef sgi #if (OSMAJORVERSION > 4) #defi
Re: [PATCH modular] Per-component configure options
On Wed, Jan 18, 2012 at 4:53 PM, Peter Hutterer wrote: > also, while it is overkill I heartily recommend asciidoc as man > page source since I suspect few people will install it and thus have it in > their manpath - asciidoc at least makes the man page human-readable without > special tools. "Overkill" would be: abandoning build.sh altogether and implementing the build procedure as a layer on top of yocto (this should help those who need to cross-compile) _then_ writing a man page (in asciidoc, of course :-) ___ 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 libXi] Force class alignment to a multiple of sizeof(XID).
On Tue, Jan 17, 2012 at 09:26:14PM +0100, Matthieu Herrb wrote: > From: Peter Hutterer > > Calculate length field to a multiples of sizeof(XID). XIDs are typedefs > to ulong and thus may be 8 bytes on some platforms. This can trigger a > SIGBUS if a class ends up not being 8-aligned (e.g. after XAxisInfo). > > Reported-by: Nicolai Stange > Signed-off-by: Peter Hutterer > Signed-off-by: Matthieu Herrb 15feb92..07ced7b master -> master thanks. Cheers, Peter > --- > src/XListDev.c | 27 +++ > 1 files changed, 19 insertions(+), 8 deletions(-) > > diff --git a/src/XListDev.c b/src/XListDev.c > index 6a16da4..6b91238 100644 > --- a/src/XListDev.c > +++ b/src/XListDev.c > @@ -61,6 +61,17 @@ SOFTWARE. > #include > #include "XIint.h" > > +/* Calculate length field to a multiples of sizeof(XID). XIDs are typedefs > + * to ulong and thus may be 8 bytes on some platforms. This can trigger a > + * SIGBUS if a class ends up not being 8-aligned (e.g. after XAxisInfo). > + */ > +static int pad_to_xid(int base_size) > +{ > +int padsize = sizeof(XID); > + > +return ((base_size + padsize - 1)/padsize) * padsize; > +} > + > static int > SizeClassInfo(xAnyClassPtr *any, int num_classes) > { > @@ -69,18 +80,18 @@ SizeClassInfo(xAnyClassPtr *any, int num_classes) > for (j = 0; j < num_classes; j++) { > switch ((*any)->class) { > case KeyClass: > -size += sizeof(XKeyInfo); > +size += pad_to_xid(sizeof(XKeyInfo)); > break; > case ButtonClass: > -size += sizeof(XButtonInfo); > +size += pad_to_xid(sizeof(XButtonInfo)); > break; > case ValuatorClass: > { > xValuatorInfoPtr v; > > v = (xValuatorInfoPtr) *any; > -size += sizeof(XValuatorInfo) + > -(v->num_axes * sizeof(XAxisInfo)); > +size += pad_to_xid(sizeof(XValuatorInfo) + > +(v->num_axes * sizeof(XAxisInfo))); > break; > } > default: > @@ -105,7 +116,7 @@ ParseClassInfo(xAnyClassPtr *any, XAnyClassPtr *Any, int > num_classes) > xKeyInfoPtr k = (xKeyInfoPtr) *any; > > K->class = KeyClass; > -K->length = sizeof(XKeyInfo); > +K->length = pad_to_xid(sizeof(XKeyInfo)); > K->min_keycode = k->min_keycode; > K->max_keycode = k->max_keycode; > K->num_keys = k->num_keys; > @@ -117,7 +128,7 @@ ParseClassInfo(xAnyClassPtr *any, XAnyClassPtr *Any, int > num_classes) > xButtonInfoPtr b = (xButtonInfoPtr) *any; > > B->class = ButtonClass; > -B->length = sizeof(XButtonInfo); > +B->length = pad_to_xid(sizeof(XButtonInfo)); > B->num_buttons = b->num_buttons; > break; > } > @@ -129,8 +140,8 @@ ParseClassInfo(xAnyClassPtr *any, XAnyClassPtr *Any, int > num_classes) > xAxisInfoPtr a; > > V->class = ValuatorClass; > -V->length = sizeof(XValuatorInfo) + > -(v->num_axes * sizeof(XAxisInfo)); > +V->length = pad_to_xid(sizeof(XValuatorInfo) + > +(v->num_axes * sizeof(XAxisInfo))); > V->num_axes = v->num_axes; > V->motion_buffer = v->motion_buffer_size; > V->mode = v->mode; > -- > 1.7.6 > > ___ > 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 > ___ 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 modular] Per-component configure options
On Wed, Jan 18, 2012 at 02:32:48PM -0500, Gaetan Nadon wrote: > On 12-01-16 06:22 PM, Trevor Woerner wrote: > > From: Trevor Woerner > > > > Allow a user to specify per-component configure options by > > providing them in the --modfile file. Any text remaining on > > a line following a given module/component is assumed to be > > options which will be passed to the configuration script. > How will anyone know that this feature even exists? > It does not show up in the --help as it is not a command line option or > an env var. One day we will need a man page :-) fwiw, if we do that, please let's rename the script first into a xorg-build or somesuch. also, while it is overkill I heartily recommend asciidoc as man page source since I suspect few people will install it and thus have it in their manpath - asciidoc at least makes the man page human-readable without special tools. otoh, since it is just a shell script to build from git, you could also prefix the output with a For a manual on build.sh, see http://wiki.x.org/wiki/build-script and use that wiki page as manual. Cheers, Peter > We already have --confflags option (and CONFFLAGS env var). I suppose > the per module conf option will be in addition to the (global) confflags. > > > > Signed-off-by: Trevor Woerner > > --- > > build.sh | 40 +++- > > 1 files changed, 27 insertions(+), 13 deletions(-) > > > > diff --git a/build.sh b/build.sh > > index b01d652..5c34eb7 100755 > > --- a/build.sh > > +++ b/build.sh > > @@ -148,11 +148,13 @@ failed() { > > # arguments: > > # $1 - module > > # $2 - component > > +# $3 - configuration options > > # returns: > > # (irrelevant) > > module_title() { > > module=$1 > > component=$2 > > +confopts="$3" > > # preconds > > if [ X"$module" = X ]; then > > return > > @@ -161,6 +163,7 @@ module_title() { > > echo "" > > echo > > "==" > > echo "== Processing module/component: \"$module/$component\"" > > +echo "==configuration options: \"$confopts\"" > Should it not also show the $CONFFLAGS option? They were not shown > before, but now it would be misleading to only show the per module > option. A popular one is --without-fop. > > } > > > > checkfortars() { > > @@ -341,7 +344,8 @@ clone() { > > # perform processing of each module/component > > # arguments: > > # $1 - module > > -# $2 - component (optional) > > +# $2 - component > > +# $3 - configure options > > # returns: > > # 0 - good > > # 1 - bad > > @@ -349,29 +353,30 @@ process() { > > needs_config=0 > > > > module=$1 > > -component=$2 > > +component="$2" > > +confopts="$3" > > # preconds > > if [ X"$module" = X ]; then > > echo "process() required first argument is missing" > > return 1 > > fi > > > > -module_title $module $component > > +module_title $module "$component" "$confopts" > > > > SRCDIR="" > > CONFCMD="" > > -if [ -f $module/$component/autogen.sh ]; then > > +if [ -f $module/"$component"/autogen.sh ]; then > > SRCDIR="$module/$component" > > CONFCMD="autogen.sh" > > elif [ X"$CLONE" != X ]; then > > -clone $module $component > > +clone $module "$component" > > if [ $? -eq 0 ]; then > > SRCDIR="$module/$component" > > CONFCMD="autogen.sh" > > fi > > needs_config=1 > > else > > -checkfortars $module $component > > +checkfortars $module "$component" > > CONFCMD="configure" > > fi > > > > @@ -451,7 +456,8 @@ process() { > > ${CPP:+CPP="$CPP"} \ > > ${CPPFLAGS:+CPPFLAGS="$CPPFLAGS"} \ > > ${CFLAGS:+CFLAGS="$CFLAGS"} \ > > - ${LDFLAGS:+LDFLAGS="$LDFLAGS"} > > + ${LDFLAGS:+LDFLAGS="$LDFLAGS"} \ > > + $confopts > Perhaps it should be following CONFFLAGS to alert the reader. > Would it need ${} like CONFFLAGS? > > if [ $? -ne 0 ]; then > > failed ${CONFCMD} $module $component > > cd $old_pwd > > @@ -536,13 +542,15 @@ process() { > > # LISTONLY, RESUME, NOQUIT, and BUILD_ONE > > # arguments: > > # $1 - module > > -# $2 - component (optional) > > +# $2 - component > > +# $3 - configure options > > # returns: > > # 0 - good > > # 1 - bad > > build() { > > module=$1 > > -component=$2 > > +component="$2" > > +confopts="$3" > > if [ X"$LISTONLY" != X ]; then > > echo "$module/$component" > > return 0 > > @@ -558,7 +566,7 @@ build() { > > fi > > fi > > > > -process $module $component > > +process $module "$component" "$confopts" > > if [ $? -ne 0 ]; then > > echo "build.sh: error processing module/component: > > \"$module/$component\"" > > if [ X"$NOQUIT" = X ]; then > > @@ -1009,9 +1017,15 @@ process_module_file() { > > continue
Re: [PATCH 1.12] A coding style for the server
The biggest (only?) downside is that any one-shot an/or on-going reformatting of _existing_ code confounds future git-annotate and git-bisect :-( ___ 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 modular] Per-component configure options
Hi Alan, Thanks for the review. (Please note that I'm purposefully being overly descriptive to help others on the list who might be new to the build.sh script) On Wed, Jan 18, 2012 at 4:09 PM, Alan Coopersmith wrote: > However, I wasn't quite clear on how to use it - perhaps because > I've failed to pay attention long enough that I didn't remember > what "the --modfile file" referred to (nor have I had time to > look into this patch really, given other priorities recently > eating most of my time). By default build.sh will run through its internal list of all the modules of which it is aware and build all of them in the correct dependency order. Currently that list contains 246 modules on a Linux x86-ish machine. If you only want to build one specific module you can specify it with the "-o" option. But if you want to build a random subset of all the modules (which we assume is a common enough occurrence) it's best to provide a "--modfile " option. Currently is a plain text file which contains a list of each module you want built, one per line. At the time I was implementing the --modfile option we had discussed whether or not build.sh should be smart enough to sort the items in itself based on its internal notion of the correct dependency order, but it was decided that if a user were using the --modfile option they would be confused if build.sh built the modules in an order different than the order specified in the . Therefore build.sh builds the modules specified in in the given order. To help a user create a build.sh provides an "-L" option; if you run "build.sh -L" it will print an in-build-dependency-order list of all the modules of which it is aware (based on the machine's architecture). This output can be a good place to start for creating a custom . With this patch I'm proposing an extension to the --modfile mechanism such that each line of the file still lists each of the modules you want built, but any remaining text on a given line (after a space) lists the options you would like passed to that module's configuration. So whereas previously my looked like this: ... lib/libXxf86vm lib/libpciaccess pixman/ mesa/drm mesa/mesa data/bitmaps app/appres ... It now looks like this: ... lib/libXxf86vm lib/libpciaccess pixman/ mesa/drm --enable-nouveau-experimental-api mesa/mesa data/bitmaps app/appres ... I'm curious to know what you are using as your custom build infrastructure (if you are at liberty to say). Best regards, Trevor ___ 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 modular] Per-component configure options
Thanks for the review Gaetan! I had completely forgot about the CONFFLAGS option. I'll start again and integrate the two (or could this be considered a replacement?). I'll also provide more usage documentation. ___ 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 modular] Per-component configure options
On 01/18/12 08:14 AM, Trevor Woerner wrote: I'm surprised nobody seems interested in this patch, I would have thought a general mechanism to provide per-component configuration options would have been a feature for which people would have been waiting; especially the distribution maintainers. As a distrubution maintainer, our distro package builds use our custom infrastructure, not build.sh. For build.sh, this seems like a nice idea, to avoid the configure warnings I get from passing flags like --disable-gallium as global flags even though only one module built by build.sh can handle it. However, I wasn't quite clear on how to use it - perhaps because I've failed to pay attention long enough that I didn't remember what "the --modfile file" referred to (nor have I had time to look into this patch really, given other priorities recently eating most of my time). -- -Alan Coopersmith-alan.coopersm...@oracle.com Oracle Solaris Platform Engineering: X Window System ___ 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 1.12] A coding style for the server
On 01/18/2012 11:59 AM, Alan Coopersmith wrote: On 01/18/12 07:08 AM, James Cloos wrote: A consistent coding style is a Good Thing™. Yes. The final indent command should be documented not only in the commit log but also in a README file. That should help encourage those who submit patches based on the tar releases also to maintain the resulting coding style. Emacs and vim cookies matching the chosen style would be most useful. Putting the style in the wiki, with links to the emacs, vim, indent, etc. rules would be good to allow it to be shared across all our modules, and then we could just stick a link to the CodingStyle wiki page in the module README's. For the lazy: http://www.x.org/wiki/CodingStyle http://www.x.org/wiki/CodingStyle";>CodingStyle ___ 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 1.12] A coding style for the server
On 01/18/2012 04:02 AM, Daniel Stone wrote: > Hi all, > So since the list has been fairly quiet and uncontroversial since the > Ctrl-Alt-Backspace argument, I thought I might throw two comedy patches > out there and see if they stuck. > > No but seriously though, it's both a) utterly ridiculous and b) faintly > stupid, that we still don't actually have a coding style. I honestly -- > honestly[0] -- don't care what the coding style is. I just care that we > have one. > > Most of the code we broadly agrees on most things. Some things were a > little more split; for instance, cuddling else was only just outnumbered > by non-cuddling else, and also braces cuddling if outnumbered > non-cuddling. > > Probably the largest change is that all tabs were expanded to eight > spaces. The only argument I've ever heard in favour of having a tab > hardcoded to eight spaces and mixing tabs and spaces is that a) emacs > does that sometimes, and b) it makes compilation faster because you have > fewer bytes to lex. I honestly don't have much time for either > argument, and it does provably make editing code harder. > > So, the commit in my coding-style xserver branch is simply the result of > running this: > indent -linux -bad -bap -blf -bli0 -br -brs -cbi0 -cdw -nce -cs -i4 -hnl > -l80 -lc80 -lp -nbbo -nbc -nbfda -nprs -npcs -npsl -saf -sai -saw -nut > across *.c and *.h in the X server tree. No more, no less. The build > seems to continue to work just fine. Don't be scared by the -linux; > it just provides a set of reasonable default options. Pretty much > everything else was just done by hand to match what we seem to generally > do in the server. Part of the problem of a coding style is the ongoing maintenance. The kernel handles this through the check_patch script. This gives rise to the following questions: 1. Should we just go with linux style, so we can use check_patch without any modifications? 2. If not, will we fork check_patch or make our own? 3. Who will enforce style? I believe if we have a script we can add a commit hook to the git server, but that may be too heavy-handed. 4. If no one will enforce style, do we want to periodically run indent to fix things. Maybe once per cycle after the merge window closes (as you suggest here, but I begged for mercy against this time :)? -- Chase ___ 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 1.12] A coding style for the server
On 01/18/2012 04:02 AM, Daniel Stone wrote: > I'd like to see this merged for 1.12, despite the merge window having > shut, so patches for 1.13's master are more likely to apply to 1.12. > It's arguably not the best time to do it, but there is no good time. As a downstream that is doing a one-off franken-server, it would be very annoying for this change to be applied directly to 1.12. A bit of background: Ubuntu has been shipping a prototype implementation of XInput multitouch since 11.04. The next release is a long-term support (LTS) release. We did not want to have conflicting XInput implementations wrt upstream for an LTS, so we decided we must have the 1.12 input implementation. However, this being an LTS cycle, our feature freeze deadline is earlier than normal. On top of that, we must have all the proprietary binary drivers for the server by feature freeze. Unfortunately, this isn't possible for 1.12. So we are going to be shipping a franken-server 1.11 + 1.12 input backport. Perhaps surprisingly, no issues have been found at all in testing, and we are about to push it into the main archive at the end of this week or early next. We are only one downstream, so I don't feel we should have veto power over this change. However, it really would make our lives much harder if this were applied to 1.12, so please have mercy on us :). All that said, I really would like a formal coding style as well. I would be all for applying this to 1.13. -- Chase ___ 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 modular] Per-component configure options
On 12-01-16 06:22 PM, Trevor Woerner wrote: > From: Trevor Woerner > > Allow a user to specify per-component configure options by > providing them in the --modfile file. Any text remaining on > a line following a given module/component is assumed to be > options which will be passed to the configuration script. How will anyone know that this feature even exists? It does not show up in the --help as it is not a command line option or an env var. One day we will need a man page :-) We already have --confflags option (and CONFFLAGS env var). I suppose the per module conf option will be in addition to the (global) confflags. > Signed-off-by: Trevor Woerner > --- > build.sh | 40 +++- > 1 files changed, 27 insertions(+), 13 deletions(-) > > diff --git a/build.sh b/build.sh > index b01d652..5c34eb7 100755 > --- a/build.sh > +++ b/build.sh > @@ -148,11 +148,13 @@ failed() { > # arguments: > # $1 - module > # $2 - component > +# $3 - configuration options > # returns: > # (irrelevant) > module_title() { > module=$1 > component=$2 > +confopts="$3" > # preconds > if [ X"$module" = X ]; then > return > @@ -161,6 +163,7 @@ module_title() { > echo "" > echo > "==" > echo "== Processing module/component: \"$module/$component\"" > +echo "==configuration options: \"$confopts\"" Should it not also show the $CONFFLAGS option? They were not shown before, but now it would be misleading to only show the per module option. A popular one is --without-fop. > } > > checkfortars() { > @@ -341,7 +344,8 @@ clone() { > # perform processing of each module/component > # arguments: > # $1 - module > -# $2 - component (optional) > +# $2 - component > +# $3 - configure options > # returns: > # 0 - good > # 1 - bad > @@ -349,29 +353,30 @@ process() { > needs_config=0 > > module=$1 > -component=$2 > +component="$2" > +confopts="$3" > # preconds > if [ X"$module" = X ]; then > echo "process() required first argument is missing" > return 1 > fi > > -module_title $module $component > +module_title $module "$component" "$confopts" > > SRCDIR="" > CONFCMD="" > -if [ -f $module/$component/autogen.sh ]; then > +if [ -f $module/"$component"/autogen.sh ]; then > SRCDIR="$module/$component" > CONFCMD="autogen.sh" > elif [ X"$CLONE" != X ]; then > -clone $module $component > +clone $module "$component" > if [ $? -eq 0 ]; then > SRCDIR="$module/$component" > CONFCMD="autogen.sh" > fi > needs_config=1 > else > -checkfortars $module $component > +checkfortars $module "$component" > CONFCMD="configure" > fi > > @@ -451,7 +456,8 @@ process() { > ${CPP:+CPP="$CPP"} \ > ${CPPFLAGS:+CPPFLAGS="$CPPFLAGS"} \ > ${CFLAGS:+CFLAGS="$CFLAGS"} \ > - ${LDFLAGS:+LDFLAGS="$LDFLAGS"} > + ${LDFLAGS:+LDFLAGS="$LDFLAGS"} \ > + $confopts Perhaps it should be following CONFFLAGS to alert the reader. Would it need ${} like CONFFLAGS? > if [ $? -ne 0 ]; then > failed ${CONFCMD} $module $component > cd $old_pwd > @@ -536,13 +542,15 @@ process() { > # LISTONLY, RESUME, NOQUIT, and BUILD_ONE > # arguments: > # $1 - module > -# $2 - component (optional) > +# $2 - component > +# $3 - configure options > # returns: > # 0 - good > # 1 - bad > build() { > module=$1 > -component=$2 > +component="$2" > +confopts="$3" > if [ X"$LISTONLY" != X ]; then > echo "$module/$component" > return 0 > @@ -558,7 +566,7 @@ build() { > fi > fi > > -process $module $component > +process $module "$component" "$confopts" > if [ $? -ne 0 ]; then > echo "build.sh: error processing module/component: > \"$module/$component\"" > if [ X"$NOQUIT" = X ]; then > @@ -1009,9 +1017,15 @@ process_module_file() { > continue > fi > > - module=`echo $line | cut -d'/' -f1` > - component=`echo $line | cut -d'/' -f2` > - build $module $component > + module=`echo $line | cut -d' ' -f1 | cut -d'/' -f1` > + component=`echo $line | cut -d' ' -f1 | cut -d'/' -f2` > + confopts_check=`echo $line | cut -d' ' -f2-` > + if [ "$module/$component" = "$confopts_check" ]; then > + confopts="" > + else > + confopts="$confopts_check" > + fi > + build $module "$component" "$confopts" > done <"$MODFILE" > > return 0 ___ 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 1.12] A coding style for the server
On 01/17/12 07:02 PM, Daniel Stone wrote: I'd like to see this merged for 1.12, despite the merge window having shut, so patches for 1.13's master are more likely to apply to 1.12. It's arguably not the best time to do it, but there is no good time. Makes sense to me - no matter when we do it, at some point there will be a boundary across which patches have to be reformatted to apply cleanly, so doing it as the current stable series is not getting as many backports as when it first came out, but we're getting ready to release a new stable branch seems like a good time. I would like to just have one big boundary to cross though, so hope we can work out the few style points that have been raised quickly, before pulling this in, but anything to make the XKB code look less alien and utterly unlike anything else in the server (or pretty much anything else) seems like a win. For the general idea: Acked-by: Alan Coopersmith -- -Alan Coopersmith-alan.coopersm...@oracle.com Oracle Solaris Platform Engineering: X Window System ___ 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 1.12] A coding style for the server
On 01/18/12 07:08 AM, James Cloos wrote: A consistent coding style is a Good Thing™. Yes. The final indent command should be documented not only in the commit log but also in a README file. That should help encourage those who submit patches based on the tar releases also to maintain the resulting coding style. Emacs and vim cookies matching the chosen style would be most useful. Putting the style in the wiki, with links to the emacs, vim, indent, etc. rules would be good to allow it to be shared across all our modules, and then we could just stick a link to the CodingStyle wiki page in the module README's. -- -Alan Coopersmith-alan.coopersm...@oracle.com Oracle Solaris Platform Engineering: X Window System ___ 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 modular] Per-component configure options
On 12-01-18 11:14 AM, Trevor Woerner wrote: > I'm surprised nobody seems interested in this patch, I would have > thought a general mechanism to provide per-component configuration > options would have been a feature for which people would have been > waiting; especially the distribution maintainers. Sorry, I missed this patch. I'll have a look at it. > > I was trying to perform a build from the git repositories on a clean > installation of a Ubuntu 10.04.3 (LTS) machine and found that I > couldn't build mesa/mesa successfully without supplying the > "--enable-nouveau-experimental-api" configuration option when building I have never been able to build mesa "out-of-the-box". It always needed some options to not build certain areas. I noticed from a poster using build.sh that he uses --confflags with all options. The modules that do not recognise an option simply issues a warning. This cannot replace the feature you propose but comes in support of it. > mesa/drm. On my openSuSE 12.1 machine (which I normally use) the build > of mesa/mesa gets contaminated by the system headers (which I hadn't > noticed at the time) which are provided by the libdrm-devel package. > The Ubuntu 10.04.3 machine doesn't have this header in /usr/include so > the build fails. In any case a git build shouldn't be contaminated in > this way. This same scenario happened to me with the xserver. No -I were provided for a config test and the system installed headers were used for those who had the development headers installed. I fixed this. > > Ideally it would be great to perform a sysroot-style build at some > point to find out what other modules are being contaminated by system > include files, but that will have to wait until I find the motivation > and/or time. Be aware that pkg-config has a default search path that will pick-up .pc files from the system installed ones. To ensure that you are not picking-up any headers or libs, export PKG_CONFIG_LIBDIR=/dev/null. Building from a "server" distro with no X installed is a sure thing. > ___ > 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 > ___ 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 1.12] A coding style for the server
James Cloos (18/01/2012): > The final indent command should be documented not only in the commit log > but also in a README file. That should help encourage those who submit > patches based on the tar releases also to maintain the resulting coding > style. > > Emacs and vim cookies matching the chosen style would be most useful. Oh yeah. Mraw, KiBi. signature.asc Description: Digital 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] Remove XAA
On Wed, Jan 18, 2012 at 03:05:41PM +1100, Daniel Stone wrote: > commit 3811d1b85b71c1f8bd0b3c8269e0f7d388a51c2b > Author: Daniel Stone > Date: Wed Jan 18 15:01:09 2012 +1100 > > Remove XAA > > It hasn't worked for over four years. No-one's even come close to > fixing it. I love this. It invalidates some of my outstanding patches in the best possible way. But... could you explain (preferably in the commit message) what evidence you have that "it hasn't worked"? I thought people were still using XAA. Jamey signature.asc Description: Digital 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] Remove XAA
On Tue, Jan 17, 2012 at 11:05 PM, Daniel Stone wrote: > Hi, > The following commit is available in the 'xaa' branch of > git://people.freedesktop.org/~daniels/xserver - I think the commit > message explains it fairly well. > > I've got patches to intel, ati, mach64 and mga to fix their builds in > the absence of XAA; a lot of other drivers[0] still depend on XAA, > however. Given that the drivers right now are broken, I'm proposing to > leave this patch until 1.13, so we at least have some time to fix the > drivers and get them deployed. > > commit 3811d1b85b71c1f8bd0b3c8269e0f7d388a51c2b > Author: Daniel Stone > Date: Wed Jan 18 15:01:09 2012 +1100 > > Remove XAA > > It hasn't worked for over four years. No-one's even come close to > fixing it. > > RIP. > > Signed-off-by: Daniel Stone Finally! Reviewed-by: Alex Deucher > > I swear this is my last semi-troll-but-actually-serious commit. > > Cheers, > Daniel > > [0]: apm ark ast chips cirrus cyrix geode glide glint i128 i740 impact > imstt neomagic newport nsc nv r128 radeonhd rendition s3 s3virge > savage siliconmotion sis sisusb sunffb tdfx tga trident tseng > vermilion via voodoo xgi xgixp > ___ > 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 ___ 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 modular] Per-component configure options
I'm surprised nobody seems interested in this patch, I would have thought a general mechanism to provide per-component configuration options would have been a feature for which people would have been waiting; especially the distribution maintainers. I was trying to perform a build from the git repositories on a clean installation of a Ubuntu 10.04.3 (LTS) machine and found that I couldn't build mesa/mesa successfully without supplying the "--enable-nouveau-experimental-api" configuration option when building mesa/drm. On my openSuSE 12.1 machine (which I normally use) the build of mesa/mesa gets contaminated by the system headers (which I hadn't noticed at the time) which are provided by the libdrm-devel package. The Ubuntu 10.04.3 machine doesn't have this header in /usr/include so the build fails. In any case a git build shouldn't be contaminated in this way. Ideally it would be great to perform a sysroot-style build at some point to find out what other modules are being contaminated by system include files, but that will have to wait until I find the motivation and/or time. ___ 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 1.12] A coding style for the server
On Tue, Jan 17, 2012 at 10:02 PM, Daniel Stone wrote: > I honestly -- > honestly[0] -- don't care what the coding style is. I just care that we > have one. PS I forgot to add that I agree with this statement 100% and think this is a good conversation to be having during the lull. ___ 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 1.12] A coding style for the server
A consistent coding style is a Good Thing™. Trevor’s comments should be addressed. The final indent command should be documented not only in the commit log but also in a README file. That should help encourage those who submit patches based on the tar releases also to maintain the resulting coding style. Emacs and vim cookies matching the chosen style would be most useful. -JimC -- James Cloos OpenPGP: 1024D/ED7DAEA6 ___ 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 1.12] A coding style for the server
On Tue, Jan 17, 2012 at 10:02 PM, Daniel Stone wrote: > indent -linux -bad -bap -blf -bli0 -br -brs -cbi0 -cdw -nce -cs -i4 -hnl > -l80 -lc80 -lp -nbbo -nbc -nbfda -nprs -npcs -npsl -saf -sai -saw -nut About a dozen of the options you specify are already included in the -linux option. As such your list only needs to include: -linux -bad -blf -bli0 -cbi0 -cdw -nce -cs -i4 -lc80 -nbbo -nbc -nbfda -nut In other words the following are already included in -linux: -bap -br -brs -hnl -l80 -lp -nprs -npcs -npsl -saf -sai -saw >From reading the man page it appears the -nbbo (no break after boolean) option conflicts with the -hnl (honour newlines) option such that -hnl cancels -nbbo, therefore -nbbo is ignored. Personally I don't like it when a developer submits a patch that intermixes code changes with formatting changes. As such I'm not fond of the -lp (continue at parenthesis) option because it has the side-effect of encouraging this behaviour. If, for example, a given function has 5 arguments and the code is refactored to change the name of the function such that the new name is a different length from the previous name, the patch will then contain the changed line plus 4 lines of formatting changes to keep the arguments "lined up". You mentioned that cuddling the else slightly outnumbered not cuddling (was there a vote that I missed, or are you basing this on an examination of the existing code?). However you then provide the -nce (don't cuddle else) option. Personally I prefer -nce, but if you'd like to go with the majority then -ce should be used instead. Best regards, Trevor ___ 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: transset
Hello, I merged the code from transset-df, did a lot of cleanup and fixed memory leaks. So I guess that should be fine now, but if anyone is willing to review, that would be great of course. What should I do to get it included into xorg/app/? Thanks. Regards, -- Arnaud Fontaine ___ 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