Re: [PATCH] evdev: add 3x3 transformation matrix xinput property for multi-head handling
On Wed, 2010-04-21 at 07:56 +0200, ext Peter Korsgaard wrote: Oliver == Oliver McFadden oliver.mcfad...@nokia.com writes: Oliver Hi Peter, Oliver There is one minor bug in this patch, you forgot to xfree() Oliver str from xf86CheckStrOption when you're done with it; see my Oliver recent patch to evdev. Ahh, yes - Thanks. I'll respin the patch with this fixed and anything else you find. Oliver I'll apply the patch locally and do a full analysis after my Oliver coffee. :-) Great, thanks! Everything else looks fine; just the leaked storage issue. -- Oliver. ___ 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 util-macros] XORG_GIT_MODULE_VERSION: writes module version in xorg-git-version.h
On Mon, Apr 19, 2010 at 10:01:06PM -0400, Gaetan Nadon wrote: On Tue, 2010-04-20 at 10:54 +1000, Peter Hutterer wrote: On Mon, Apr 19, 2010 at 11:00:08PM +0200, Julien Cristau wrote: On Mon, Apr 19, 2010 at 14:52:55 -0400, Gaetan Nadon wrote: Generates the git module version according to the git describe HEAD If the git module has pending changes, it appends -dirty to the version tag Signed-off-by: Gaetan Nadon mems...@videotron.ca --- xorg-macros.m4.in | 18 ++ 1 files changed, 18 insertions(+), 0 deletions(-) diff --git a/xorg-macros.m4.in b/xorg-macros.m4.in index 916b472..efb2e53 100644 --- a/xorg-macros.m4.in +++ b/xorg-macros.m4.in @@ -1009,3 +1009,21 @@ mv \$(top_srcdir)/.INSTALL.tmp \$(top_srcdir)/INSTALL) \ echo 'util-macros \pkgdatadir\ from xorg-macros.pc not found: installing possibly empty INSTALL.' 2) AC_SUBST([INSTALL_CMD]) ]) # XORG_INSTALL + +# XORG_GIT_MODULE_VERSION() +# - +# Minimum version: 1.8.0 +# +# Generates the git module version according to the git describe HEAD +# If the git module has pending changes, it appends -dirty to the version tag +# +AC_DEFUN([XORG_GIT_MODULE_VERSION], [ +GIT_MODULE_VERSION_CMD=VER=\`GIT_DIR=\$(top_srcdir)/.git git describe HEAD 2/dev/null\`; \ +DVER=\`GIT_DIR=\$(top_srcdir)/.git git diff-index HEAD 2/dev/null\`; \ +OUTSTR=\\#undef XORG_GIT_VERSION\ ; \ +OUTFILE=\xorg-git-version.h\; \ +test -n \\$\$VER\ OUTSTR=\\#define XORG_GIT_VERSION \$\$VER\ test -n \\$\$DVER\ OUTSTR=\\#define XORG_GIT_VERSION \$\$VER-dirty\; \ +test -e \\$\$OUTFILE\ || echo \\$\$OUTSTR\ \\$\$OUTFILE\; \ +CONTENT=\`cat \$\$OUTFILE\` test \\$\$CONTENT\ = \\$\$OUTSTR\ || echo \$\$OUTSTR \$\$OUTFILE; +AC_SUBST([GIT_MODULE_VERSION_CMD]) +]) # XORG_GIT_MODULE_VERSION my eyes! As I said in reply to the initial patch by Peter I'd like a way to disable this, because packages might be built from a git tree, which might be unrelated to the xorg one, or outside of any git tree. What's the output like if not building from git? Does 'make GIT_MODULE_VERSION_CMD=:' work to disable it (I guess not, because anything trying to include xorg-git-version.h will be unhappy)? it simply sets the #undef and nothing will be printed to the log file. Is there any specific argument against _running_ the macro as long if it doesn't add anything to the logfile? Gaetan: I'm not a big fan of the -dirty either but I guess if others want it we can leave it in. Two questions though: If you remotely help someone debugging a problem, you would like to know if tag/commit is really the code he is running or if there are local changes. With the commit number you can cross-reference the master repo and detect unpsuhed local commits. With the -dirty (now called -with-uncommitted-changes) you now the code is tainted with local changes you know nothing about. It's just additional info. I really hope that when I help someone debugging they'd tell me beforehand if they have local changes. in which case the dirty doesn't help much anyway because you still don't know what the changes do. but yeah, doesn't matter either way, I'm fine with it. - If modules start using this macro, do they stillhave to add to DISTCLEANFILES and friends? - This macro only gets invoked on automake runs, right? So if I pull and just rebuild, would it update the git version? All options are available in the Makefile.am regarding the invocation of the macro (or your original script). I have not paid attention to that, it looks you had it covered. The problem domain is similar to ChangeLog. You need a trigger when a new commit is available, but the best we could do is a dist hook. And you may not be running off git. What use case was this feature designed for? If someone follows git and files a bug (or sends the log) it'll be easy to see which commit they have locally. So questions like do you have commit 1234deadbeef in your local repo aren't needed, reducing the turnaround time. One suggested a dependency on a git file that changes with the HEAD commit, don't recall what the objections were. I'll think about it some more. Acked-by: Peter Hutterer peter.hutte...@who-t.net for version 2 but please make sure that someone else acks it too, automake stuff gives me the creeps. would also be good checking through Matthias' script from radeonhd for any obvious use-cases that this patch doesn't cover. It certainly seems more complex. 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 1/2] Add Clickpad support
At Wed, 21 Apr 2010 14:16:41 +1000,
Peter Hutterer wrote:
Thank you for the patch. Just as a heads-up, I'd like to wait with this
until there's at least one kernel released with this support.
My comments to the patch are inline.
Thanks for review!
On Thu, Apr 15, 2010 at 06:12:10PM +0200, Takashi Iwai wrote:
This patch adds the support for Synaptics Clickpad devices.
It requires the change in Linux kernel synaptics input driver, found in
https://patchwork.kernel.org/patch/92435/
When the kernel driver sets only the left-button bit evbit, Clickpad
mode is activated. In this mode, the bottom touch area is used as
button emulations. Clicking at the bottom-left, bottom-center and
bottom-right zone corresponds to a left, center and right click.
There are two new parameters added. One is a parameter to specify
the size of the bottom button-area, and another is to toggle the touch-
sensing in the button area.
Signed-off-by: Takashi Iwai ti...@suse.de
---
include/synaptics-properties.h |6
man/synaptics.man | 21 +++
src/eventcomm.c| 15 ++
src/properties.c | 19 +
src/synaptics.c| 56
src/synapticsstr.h |4 +++
tools/synclient.c |2 +
7 files changed, 123 insertions(+), 0 deletions(-)
diff --git a/include/synaptics-properties.h b/include/synaptics-properties.h
index cf330d8..c77afd3 100644
--- a/include/synaptics-properties.h
+++ b/include/synaptics-properties.h
@@ -155,4 +155,10 @@
/* 32 bit, 4 values, left, right, top, bottom */
#define SYNAPTICS_PROP_AREA Synaptics Area
+/* 8 bit (0-100) */
+#define SYNAPTICS_PROP_TOUCH_BUTTON_AREA Synaptics Touch Button Area
+
+/* 8 bit (BOOL) */
+#define SYNAPTICS_PROP_TOUCH_BUTTON_SENSE Synaptics Touch Button Sense
+
#endif /* _SYNAPTICS_PROPERTIES_H_ */
diff --git a/man/synaptics.man b/man/synaptics.man
index 59fbaac..f92ea0c 100644
--- a/man/synaptics.man
+++ b/man/synaptics.man
@@ -509,6 +509,27 @@ Ignore movements, scrolling and tapping which take
place below this edge.
The option is disabled by default and can be enabled by setting the
AreaBottomEdge option to any integer value other than zero. Property:
Synaptics Area
.
+.TP
+.BI Option \*qTouchButtonArea\*q \*q integer \*q
+The percentage of touch-button area of Clickpad device.
+.
+The synaptics driver supports ClickZone mode, which emulates the buttons
+to click at the bottom of touchpad area. This option specifies how large
+is the area to be used as the button area. Passing zero to this disables
+the Clickpad behavior. The default value is 20.
where does the 0-100 come from? what unit is this? it's answered in the
header file but this should be in the man page as well.
Right.
also, I'd rather have this stored in device units and leave things like
this is 20% of the area to higher-level tools.
I don't mind in any way. Maybe this parameter is rarely changed.
+Property: Synaptics Touch Button Area
please make this so that the Synaptics Area is re-used instead of
introducing another area property. This should remove the need for this
property altogether and just need the enable/disable value in the other one.
Do you mean to extend Synaptics Area property?
I find it might be better to split from Synaptics Area, because this
is pretty specific to Clickpad mode while Synaptics Area is a more
generic one. But, mixing them up is technically no problem, of
course.
how do you get the default of 20? isn't this something that should be
auto-scaled from the device in case future devices have a different
resolution?
It looks same regardless of the device size. I checked several
clickpad devices in different sizes, and all of them show mostly same
button area, ca 20% of the touchpad size at the bottom.
+.TP
+.BI Option \*qTouchButtonSense\*q \*q integer \*q
+Disable the touch-sensing in the Clickpad button area.
+.
+Since the touchpad is often quite sensitive, the pointer would move
+slightly when you click via Clickpad, and user may miss the aimed
+position. When this option is set, the cursor movement in the
+Clickpad button area is suppressed, thus the point won't move while
+clicking the button. On the other hand, this reduces the available
+space on the touchpad. The default is on.
+Property: Synaptics Touch Button Sense
+.
.LP
A tap event happens when the finger is touched and released in a time
interval shorter than MaxTapTime, and the touch and release
diff --git a/src/eventcomm.c b/src/eventcomm.c
index d00d810..ca99f3a 100644
--- a/src/eventcomm.c
+++ b/src/eventcomm.c
@@ -166,6 +166,19 @@ event_query_info(LocalDevicePtr local)
}
}
+static void event_query_clickpad(LocalDevicePtr local)
+{
+
Re: [PATCH] evdev: add 3x3 transformation matrix xinput property for multi-head handling
Oliver == Oliver McFadden oliver.mcfad...@nokia.com writes: Hi, Oliver Everything else looks fine; just the leaked storage issue. Great, can I add your Acked-by then when I resend? -- Bye, Peter Korsgaard ___ 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] evdev: add 3x3 transformation matrix xinput property for multi-head handling
On Wed, 2010-04-21 at 08:30 +0200, ext Peter Korsgaard wrote: Oliver == Oliver McFadden oliver.mcfad...@nokia.com writes: Hi, Oliver Everything else looks fine; just the leaked storage issue. Great, can I add your Acked-by then when I resend? Yeah, no problem. -- Oliver. ___ 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
[PATCHv2] evdev: add 3x3 transformation matrix xinput property for multi-head handling
For absolute input devices (E.G. touchscreens) in multi-head setups, we
need a way to bind the device to an randr output. This adds the
infrastructure to evdev to allow us to do so.
The X server scales input coordinates to the dimensions of the shared
(total) frame buffer, so to restrict motion to an output we need to
scale/rotate/translate device coordinates to a subset of the frame buffer
before passing them on to the X server.
This is done here using a 3x3 transformation matrix, which is applied to
the device coordinates using homogeneous coordinates, E.G.:
[ c0 c1 c2 ] [ x ]
[ c3 c4 c5 ] * [ y ]
[ c6 c7 c8 ] * [ 1 ]
Notice: As input devices have varying input ranges, the coordinates are first
scaled to the [0..1] range for generality, and afterwards scaled back up.
E.G. for a dual head setup (using same resolution) next to each other, you
would want to scale the X coordinates of touchscreen connected to the
both heads by 50%, and translate (offset) the coordinates of the rightmost
head by 50%, or in matrix form:
left: right:
[ 0.5 0 0 ] [ 0.5 0 0.5 ]
[ 0 1 0 ] [ 0 1 0 ]
[ 0 0 1 ] [ 0 0 0 ]
Which can be done using xinput:
xinput set-prop left --type=float Evdev Coordinate Transformation Matrix \
0.5 0 0 0 1 0 0 0 1
xinput set-prop right --type=float Evdev Coordinate Transformation Matrix \
0.5 0 0.5 0 1 0 0 0 1
This is general enough to replace the various tweaks we have in evdev
(InvertX/Y, Calibration, SwapAxes), but I have left them for now.
Signed-off-by: Peter Korsgaard peter.korsga...@barco.com
Acked-by: Oliver McFadden oliver.mcfad...@nokia.com
---
Changes since V1:
- Add missing xfree() noticed by Oliver
include/evdev-properties.h |7 ++
man/evdev.man | 13
src/evdev.c| 139 +++-
src/evdev.h|2 +
4 files changed, 160 insertions(+), 1 deletions(-)
diff --git a/include/evdev-properties.h b/include/evdev-properties.h
index 7df2876..7417a5b 100644
--- a/include/evdev-properties.h
+++ b/include/evdev-properties.h
@@ -66,4 +66,11 @@
/* BOOL */
#define EVDEV_PROP_SWAP_AXES Evdev Axes Swap
+/* Coordinate transformation matrix */
+/* FLOAT, 9 values in row-major order, coordinates in 0..1 range: */
+/* [c0 c1 c2] [x] */
+/* [c5 c4 c5] * [y] */
+/* [c6 c7 c8] [1] */
+#define EVDEV_PROP_TRANSFORM Evdev Coordinate Transformation Matrix
+
#endif
diff --git a/man/evdev.man b/man/evdev.man
index 49ab12c..21d7aa4 100644
--- a/man/evdev.man
+++ b/man/evdev.man
@@ -161,6 +161,15 @@ originally reported by the kernel (e.g. touchscreens). The
scaling to the
custom coordinate system is done in-driver and the X server is unaware of
the transformation. Property: Evdev Axis Calibration.
.TP 7
+.BI Option \*qTransformationMatrix\*q \*q c0 c1 c2 c3 c4 c5 c6 c7 c8 \*q
+Applies the 3x3 transformation matrix defined by c0..c8 in row-major
+order to the device coordinates before passing them on to the X
+server. The coefficients are floating point values, and the
+coordinates are scaled to the 0..1 range before transformed. This
+feature can be used to restrict absolute devices (e.g. touchscreens)
+to specific outputs in multi-head setups. Property: Evdev Coordinate
+Transformation Matrix.
+.TP 7
.B Option \*qMode\*q \*qRelative\*q\fP|\fP\*qAbsolute\*q
Sets the mode of the device if device has absolute axes.
The default value for touchpads is relative, for other absolute.
@@ -196,6 +205,10 @@ driver.
4 32-bit values, order min-x, max-x, min-y, max-y or 0 values to disable
in-driver axis calibration.
.TP 7
+.BI Evdev Coordinate Transformation Matrix
+9 floating point values, defining the coordinate transformation matrix
+in row-major order.
+.TP 7
.BI Evdev Axis Inversion
2 boolean values (8 bit, 0 or 1), order X, Y. 1 inverts the axis.
.TP 7
diff --git a/src/evdev.c b/src/evdev.c
index 0678edf..31da4bf 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -32,6 +32,8 @@
#endif
#include xorg-server.h
+#include math.h
+
#include X11/keysym.h
#include X11/extensions/XI.h
@@ -119,6 +121,7 @@ static Atom prop_calibration = 0;
static Atom prop_swap = 0;
static Atom prop_axis_label = 0;
static Atom prop_btn_label = 0;
+static Atom prop_transform = 0;
#endif
/* All devices the evdev driver has allocated and knows about.
@@ -347,6 +350,35 @@ EvdevQueueButtonClicks(InputInfoPtr pInfo, int button, int
count)
}
}
+static int
+EvdevClip(InputInfoPtr pInfo, float f, int axis)
+{
+EvdevPtr pEvdev = pInfo-private;
+int v;
+
+v = lroundf(f);
+
+if (v pEvdev-absinfo[axis].maximum)
+v = pEvdev-absinfo[axis].maximum;
+else if (v pEvdev-absinfo[axis].minimum)
+v = pEvdev-absinfo[axis].minimum;
+
+return v;
+}
+
+static void
+EvdevTransformAbsolute(InputInfoPtr pInfo, int v[MAX_VALUATORS])
+{
+EvdevPtr pEvdev = pInfo-private;
+float x, y, *t = pEvdev-transform;
+
+x = t[0]*v[0] + t[1]*v[1] + t[2];
Re: [PATCH 2/2] Support LED and tap-on-LED feature
At Wed, 21 Apr 2010 16:53:14 +1000,
Peter Hutterer wrote:
On Wed, Apr 21, 2010 at 08:26:41AM +0200, Takashi Iwai wrote:
At Wed, 21 Apr 2010 14:16:29 +1000,
Peter Hutterer wrote:
On Thu, Apr 15, 2010 at 06:12:11PM +0200, Takashi Iwai wrote:
This patch adds the control of the embedded LED on the top-left corner
of new Synaptics devices. For LED control, it requires the patch to
Linux synaptics input driver,
https://patchwork.kernel.org/patch/92434/
When evdev reports the presense of LED and LED_MUTE bits, the driver
assumes it supports the embeded LED control. This corresponds to the
touchpad-off state. When touchpad is disabled, LED is turned on.
For linking between the touchpad state and the LED state, a new callback
UpdateHardware is introduced. Only eventcomm.c supports this
(naturally).
A new feature for the LED-equipped device is that user can double-tap
on the LED to toggle the touchpad state on the fly. This is also linked
with the touchpad-off state.
There is a new parameter for controlling the LED double-tap behavior,
too.
It specifies the double-tap time. Passing zero disables the double-tap
feature.
Signed-off-by: Takashi Iwai ti...@suse.de
---
include/synaptics-properties.h |3 ++
man/synaptics.man | 13 +++
src/eventcomm.c| 39 +-
src/properties.c | 26 ++
src/synaptics.c| 73
++-
src/synapticsstr.h |6 +++
src/synproto.h |1 +
tools/synclient.c |1 +
8 files changed, 159 insertions(+), 3 deletions(-)
diff --git a/include/synaptics-properties.h
b/include/synaptics-properties.h
index c77afd3..56c3e1d 100644
--- a/include/synaptics-properties.h
+++ b/include/synaptics-properties.h
@@ -161,4 +161,7 @@
/* 8 bit (BOOL) */
#define SYNAPTICS_PROP_TOUCH_BUTTON_SENSE Synaptics Touch Button
Sense
+/* 32 bit */
+#define SYNAPTICS_PROP_LED_DOUBLE_TAP Synaptics LED Double Tap
+
#endif /* _SYNAPTICS_PROPERTIES_H_ */
diff --git a/man/synaptics.man b/man/synaptics.man
index f92ea0c..baa7e8f 100644
--- a/man/synaptics.man
+++ b/man/synaptics.man
@@ -530,6 +530,19 @@ clicking the button. On the other hand, this
reduces the available
space on the touchpad. The default is on.
Property: Synaptics Touch Button Sense
.
+.TP
+.BI Option \*qLEDDoubleTap\*q \*q integer \*q
+.
+The double-tap time for toggling the touchpad-control on the top-left
+corner LED.
+.
+Some devices have an LED on the top-left corner to indicate the
+touchpad state. User can double-tap on the LED to toggle the touchpad
+state. This option controls the double-tap time in milli-seconds.
+When it's zero, the LED-tapping feature is disabled. The default
+value is 400.
+Property: Synaptics LED Double Tap
a few questions to the principle:
- any specific reason this is a double-tap?
A single-tap is often too sensitive. User may toggle it mistakenly,
and wonders what happens suddenly. Also, Windows driver uses the
double-tap, too.
- the corner might be used for the same feature even if there is no LED
present, right?
The check is done only when has_led flag is set, so it's exclusive for
the device with LED control.
as I read this patch, the LED updating could be split out
from the actual gesture of disabling the touchpad by clicking into the
corner.
In general yes, the double-tapping to toggle touchpad is an individual
feature. But this becomes intuitive when coupled with LED, thus
rather somewhat special for LED-equipped device, I suppose.
gnome has a feature that allows you to disable the touchpad hitting Fn+F8,
and it then displays a popup box to notify the user of this. The same could
be done for this feature by monitoring the off property, even if there's no
LED present. So I think there are use-cases that warrant this feature
without the tight LED integration.
Right, but the positioned double-tapping (that is featured on Windows)
seems coupled with LED, I suppose.
@@ -496,6 +500,11 @@ SetProperty(DeviceIntPtr dev, Atom property,
XIPropertyValuePtr prop,
return BadValue;
para-touchpad_off = off;
+ if (!checkonly) {
+ if (priv-proto_ops priv-proto_ops-UpdateHardware)
+ priv-proto_ops-UpdateHardware(local);
+ }
+
indentation.
What is wrong here?
the property code is one of the few places where the indentation is
consistent (mainly because I wrote most of it :). Please use spaces here
instead of tabs, which generally is my preference
Re: [PATCH 1/2] Add Clickpad support
At Wed, 21 Apr 2010 16:31:29 +1000, Peter Hutterer wrote: On Wed, Apr 21, 2010 at 08:16:40AM +0200, Takashi Iwai wrote: At Wed, 21 Apr 2010 14:16:41 +1000, Peter Hutterer wrote: Thank you for the patch. Just as a heads-up, I'd like to wait with this until there's at least one kernel released with this support. My comments to the patch are inline. Thanks for review! On Thu, Apr 15, 2010 at 06:12:10PM +0200, Takashi Iwai wrote: This patch adds the support for Synaptics Clickpad devices. It requires the change in Linux kernel synaptics input driver, found in https://patchwork.kernel.org/patch/92435/ When the kernel driver sets only the left-button bit evbit, Clickpad mode is activated. In this mode, the bottom touch area is used as button emulations. Clicking at the bottom-left, bottom-center and bottom-right zone corresponds to a left, center and right click. There are two new parameters added. One is a parameter to specify the size of the bottom button-area, and another is to toggle the touch- sensing in the button area. Signed-off-by: Takashi Iwai ti...@suse.de --- include/synaptics-properties.h |6 man/synaptics.man | 21 +++ src/eventcomm.c| 15 ++ src/properties.c | 19 + src/synaptics.c| 56 src/synapticsstr.h |4 +++ tools/synclient.c |2 + 7 files changed, 123 insertions(+), 0 deletions(-) diff --git a/include/synaptics-properties.h b/include/synaptics-properties.h index cf330d8..c77afd3 100644 --- a/include/synaptics-properties.h +++ b/include/synaptics-properties.h @@ -155,4 +155,10 @@ /* 32 bit, 4 values, left, right, top, bottom */ #define SYNAPTICS_PROP_AREA Synaptics Area +/* 8 bit (0-100) */ +#define SYNAPTICS_PROP_TOUCH_BUTTON_AREA Synaptics Touch Button Area + +/* 8 bit (BOOL) */ +#define SYNAPTICS_PROP_TOUCH_BUTTON_SENSE Synaptics Touch Button Sense + #endif /* _SYNAPTICS_PROPERTIES_H_ */ diff --git a/man/synaptics.man b/man/synaptics.man index 59fbaac..f92ea0c 100644 --- a/man/synaptics.man +++ b/man/synaptics.man @@ -509,6 +509,27 @@ Ignore movements, scrolling and tapping which take place below this edge. The option is disabled by default and can be enabled by setting the AreaBottomEdge option to any integer value other than zero. Property: Synaptics Area . +.TP +.BI Option \*qTouchButtonArea\*q \*q integer \*q +The percentage of touch-button area of Clickpad device. +. +The synaptics driver supports ClickZone mode, which emulates the buttons +to click at the bottom of touchpad area. This option specifies how large +is the area to be used as the button area. Passing zero to this disables +the Clickpad behavior. The default value is 20. where does the 0-100 come from? what unit is this? it's answered in the header file but this should be in the man page as well. Right. also, I'd rather have this stored in device units and leave things like this is 20% of the area to higher-level tools. I don't mind in any way. Maybe this parameter is rarely changed. +Property: Synaptics Touch Button Area please make this so that the Synaptics Area is re-used instead of introducing another area property. This should remove the need for this property altogether and just need the enable/disable value in the other one. Do you mean to extend Synaptics Area property? I find it might be better to split from Synaptics Area, because this is pretty specific to Clickpad mode while Synaptics Area is a more generic one. But, mixing them up is technically no problem, of course. right now, the area detection code won't move the cursor if it's outside of the applicable area. this was essentially added for the touchpads the dell minis had. so as long as you're inside that area the pointer moves and outside nothing happens. What about scrolling? Is the vert/horiz scroll sensed outside the synaptics area? with this feature, you could modulate this option with a clickpad feature, i.e. if ClickPad is on, then the area outside is used as the button calculation area (or the bottom area anyway). I think this makes sense enough, given that the hardware is so different I don't think there's the need for having an inactive area and a separate clickpad area. or is this too confusing for the user - I'm not sure myself here. I'll check this possibility. how do you get the default of 20? isn't this something that should be auto-scaled from the device in case future devices have a different resolution? It looks same regardless of the
Re: Live builds
On Wed, Apr 21, 2010 at 02:28:30AM +0700, Mikhail Gusarov wrote: There's 'repo' by Android people. Seem to be quite independent from android and doing exactly this (cloning bunch of repos and keeping track of them). That's interesting. In particular that there is a global configuration, which could hold the $PREFIX, per-repo build flags, personal trees, etc. http://source.android.com/download/using-repo ___ 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 v3 2/4] xfree86/dri2: add AuthMagic hook for driver side support
From: Tiago Vignatti tiago.vigna...@nokia.com
With this new hook drmAuthMagic becomes useless and should be deprecated.
You might want to implement AuthMagic on driver side instead.
Attention: ABI being break.
Signed-off-by: Tiago Vignatti tiago.vigna...@nokia.com
Signed-off-by: Pauli Nieminen ext-pauli.niemi...@nokia.com
---
v3:
- Bump DRI2INFOREC_VERSION because xserver 1.8.0 was released with version 4.
- Prevent memory access error if ddx was compiled for older version.
hw/xfree86/dri2/dri2.c | 13 +++--
hw/xfree86/dri2/dri2.h |8 +++-
2 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
index 6c4dabc..8d2acf0 100644
--- a/hw/xfree86/dri2/dri2.c
+++ b/hw/xfree86/dri2/dri2.c
@@ -86,6 +86,7 @@ typedef struct _DRI2Screen {
DRI2ScheduleSwapProcPtr ScheduleSwap;
DRI2GetMSCProcPtr GetMSC;
DRI2ScheduleWaitMSCProcPtr ScheduleWaitMSC;
+DRI2AuthMagicProcPtrAuthMagic;
HandleExposuresProcPtr HandleExposures;
} DRI2ScreenRec;
@@ -794,8 +795,8 @@ DRI2Authenticate(ScreenPtr pScreen, drm_magic_t magic)
{
DRI2ScreenPtr ds = DRI2GetScreen(pScreen);
-if (ds == NULL || drmAuthMagic(ds-fd, magic))
- return FALSE;
+if (ds == NULL || (*ds-AuthMagic)(ds-fd, magic))
+return FALSE;
return TRUE;
}
@@ -842,6 +843,14 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
cur_minor = 1;
}
+if (info-version = 5) {
+ds-AuthMagic = info-AuthMagic;
+}
+
+if (!ds-AuthMagic)
+ds-AuthMagic = drmAuthMagic;
+
+
/* Initialize minor if needed and set to minimum provied by DDX */
if (!dri2_minor || dri2_minor cur_minor)
dri2_minor = cur_minor;
diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h
index ce8a5df..ea48ee9 100644
--- a/hw/xfree86/dri2/dri2.h
+++ b/hw/xfree86/dri2/dri2.h
@@ -66,6 +66,8 @@ typedef void (*DRI2CopyRegionProcPtr)(DrawablePtr
pDraw,
DRI2BufferPtr pSrcBuffer);
typedef void (*DRI2WaitProcPtr)(WindowPtr pWin,
unsigned int sequence);
+typedef int(*DRI2AuthMagicProcPtr)(int fd, uint32_t magic);
+
/**
* Schedule a buffer swap
*
@@ -155,7 +157,7 @@ typedef int (*DRI2ScheduleWaitMSCProcPtr)(ClientPtr
client,
/**
* Version of the DRI2InfoRec structure defined in this header
*/
-#define DRI2INFOREC_VERSION 4
+#define DRI2INFOREC_VERSION 5
typedef struct {
unsigned int version; /** Version of this struct */
@@ -179,6 +181,10 @@ typedef struct {
/* array of driver names, indexed by DRI2Driver* driver types */
/* a name of NULL means that driver is not supported */
const char * const *driverNames;
+
+/* added in version 5 */
+
+DRI2AuthMagicProcPtr AuthMagic;
} DRI2InfoRec, *DRI2InfoPtr;
extern _X_EXPORT int DRI2EventBase;
--
1.6.3.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 v3 4/4] xfree86/dri2: Use single error path in initialization
Signed-off-by: Pauli Nieminen ext-pauli.niemi...@nokia.com
---
hw/xfree86/dri2/dri2.c | 12
1 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
index e60b3a4..255c112 100644
--- a/hw/xfree86/dri2/dri2.c
+++ b/hw/xfree86/dri2/dri2.c
@@ -864,18 +864,14 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
/* Driver too old: use the old-style driverName field */
ds-numDrivers = 1;
ds-driverNames = xalloc(sizeof(*ds-driverNames));
- if (!ds-driverNames) {
- xfree(ds);
- return FALSE;
- }
+ if (!ds-driverNames)
+ goto err_out;
ds-driverNames[0] = info-driverName;
} else {
ds-numDrivers = info-numDrivers;
ds-driverNames = xalloc(info-numDrivers * sizeof(*ds-driverNames));
- if (!ds-driverNames) {
- xfree(ds);
- return FALSE;
- }
+ if (!ds-driverNames)
+ goto err_out;
memcpy(ds-driverNames, info-driverNames,
info-numDrivers * sizeof(*ds-driverNames));
}
--
1.6.3.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] Add Clickpad support (v2)
This patch adds the support for Synaptics Clickpad devices.
It requires the change in Linux kernel synaptics input driver, found in
https://patchwork.kernel.org/patch/92435/
The kernel patch is already included in linux-input GIT tree, which
will hit Linus tree sooner or later.
When the kernel driver sets only the left-button bit evbit, Clickpad
mode is activated. In this mode, the bottom touch area is used as
button emulations. Clicking at the bottom-left, bottom-center and
bottom-right zone corresponds to a left, center and right click.
Signed-off-by: Takashi Iwai ti...@suse.de
---
In this version, I dropped the parameters and simplified as much as
possible. I tried to remove the code to disable pointer moves, but
it results in the non-working dragging, as it seems relevant with
multi-tapping stuff. So it's still there.
src/eventcomm.c|6
src/synaptics.c| 72 +++-
src/synapticsstr.h |2 +
3 files changed, 79 insertions(+), 1 deletions(-)
diff --git a/src/eventcomm.c b/src/eventcomm.c
index d00d810..10d183d 100644
--- a/src/eventcomm.c
+++ b/src/eventcomm.c
@@ -252,6 +252,12 @@ event_query_axis_ranges(LocalDevicePtr local)
if ((priv-has_triple = (TEST_BIT(BTN_TOOL_TRIPLETAP, keybits) != 0)))
strcat(buf, triple);
xf86Msg(X_INFO, %s: buttons:%s\n, local-name, buf);
+
+ /* clickpad device reports only the single left button mask */
+ if (priv-has_left !priv-has_right !priv-has_middle) {
+ priv-is_clickpad = TRUE;
+ xf86Msg(X_INFO, %s: is Clickpad device\n, local-name);
+ }
}
}
diff --git a/src/synaptics.c b/src/synaptics.c
index 091dbe1..951b3fa 100644
--- a/src/synaptics.c
+++ b/src/synaptics.c
@@ -457,6 +457,18 @@ static void set_default_parameters(LocalDevicePtr local)
vertResolution = priv-resy;
}
+/* Clickpad mode -- bottom area is used as buttons */
+if (priv-is_clickpad) {
+ int button_bottom;
+ /* Clickpad devices usually the button area at the bottom, and
+* its size seems ca. 20% of the touchpad height no matter how
+* large the pad is.
+*/
+ button_bottom = priv-maxy - (abs(priv-maxy - priv-miny) * 20) / 100;
+ if (button_bottom b button_bottom = t)
+ b = button_bottom;
+}
+
/* set the parameters */
pars-left_edge = xf86SetIntOption(opts, LeftEdge, l);
pars-right_edge = xf86SetIntOption(opts, RightEdge, r);
@@ -2052,6 +2064,59 @@ HandleClickWithFingers(SynapticsParameters *para, struct
SynapticsHwState *hw)
}
}
+/* clickpad event handling */
+static void
+HandleClickpad(LocalDevicePtr local, struct SynapticsHwState *hw, edge_type
edge)
+{
+SynapticsPrivate *priv = (SynapticsPrivate *) (local-private);
+SynapticsParameters *para = priv-synpara;
+
+if (edge BOTTOM_EDGE) {
+ /* button area */
+ int width = priv-maxx - priv-minx;
+ int left_button_x, right_button_x;
+
+ /* left and right clickpad button ranges;
+* the gap between them is interpreted as a middle-button click
+*/
+ left_button_x = width * 2/ 5 + priv-minx;
+ right_button_x = width * 3 / 5 + priv-minx;
+
+ /* clickpad reports only one button, and we need
+* to fake left/right buttons depending on the touch position
+*/
+ if (hw-left) { /* clicked? */
+ hw-left = 0;
+ if (hw-x left_button_x)
+ hw-left = 1;
+ else if (hw-x right_button_x)
+ hw-right = 1;
+ else
+ hw-middle = 1;
+ }
+
+ /* Don't move pointer position in the button area during clicked,
+* except for horiz/vert scrolling is enabled.
+*
+* The synaptics driver tends to be pretty sensitive. This hack
+* is to avoid that the pointer moves slightly and misses the
+* poistion you aimed to click.
+*
+* Also, when the pointer movement is reported, the dragging
+* (with a sort of multi-touching) doesn't work well, too.
+*/
+ if (hw-left || !(para-scroll_edge_horiz ||
+ ((edge RIGHT_EDGE) para-scroll_edge_vert)))
+ hw-z = 0; /* don't move pointer */
+
+} else if (hw-left) {
+ /* dragging */
+ hw-left = priv-prev_hw.left;
+ hw-right = priv-prev_hw.right;
+ hw-middle = priv-prev_hw.middle;
+}
+priv-prev_hw = *hw;
+}
/*
* React on changes in the hardware state. This function is called every time
@@ -2102,6 +2167,12 @@ HandleState(LocalDevicePtr local, struct
SynapticsHwState *hw)
if (para-touchpad_off == 1)
return delay;
+edge = edge_detection(priv, hw-x, hw-y);
+
+/* Clickpad handling for button area */
+if (priv-is_clickpad)
+ HandleClickpad(local, hw, edge);
+
/* Treat the first two multi buttons as up/down for now. */
hw-up |=
Re: [PATCH v3 3/4] xfree86/dri2: wrap drm bits with macros and change drm_magic type
From: Pauli Nieminen ext-pauli.niemi...@nokia.com
Date: Wed, 21 Apr 2010 11:40:37 +0300
I'll reiterate an opinion stated here before. We should really try to
*reduce* the number of #ifdefs in the code base, not introduce new
ones.
diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
index 8d2acf0..e60b3a4 100644
--- a/hw/xfree86/dri2/dri2.c
+++ b/hw/xfree86/dri2/dri2.c
@@ -35,7 +35,9 @@
#endif
#include errno.h
+#ifdef WITH_LIBDRM
#include xf86drm.h
+#endif
#include xf86Module.h
#include scrnintstr.h
#include windowstr.h
@@ -791,7 +793,7 @@ DRI2Connect(ScreenPtr pScreen, unsigned int driverType,
int *fd,
}
Bool
-DRI2Authenticate(ScreenPtr pScreen, drm_magic_t magic)
+DRI2Authenticate(ScreenPtr pScreen, uint32_t magic)
{
DRI2ScreenPtr ds = DRI2GetScreen(pScreen);
@@ -848,8 +850,11 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
}
if (!ds-AuthMagic)
+#ifdef WITH_LIBDRM
ds-AuthMagic = drmAuthMagic;
-
+#else
+goto err_out;
+#endif
/* Initialize minor if needed and set to minimum provied by DDX */
if (!dri2_minor || dri2_minor cur_minor)
@@ -886,6 +891,12 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
}
return TRUE;
+
+err_out:
+xf86DrvMsg(pScreen-myNum, X_WARNING,
+[DRI2] Initialization failed for info version %d.\n,
info-version);
+xfree(ds);
+return FALSE;
}
void
diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h
index ea48ee9..0a14094 100644
--- a/hw/xfree86/dri2/dri2.h
+++ b/hw/xfree86/dri2/dri2.h
@@ -202,7 +202,7 @@ extern _X_EXPORT Bool DRI2Connect(ScreenPtr pScreen,
const char **driverName,
const char **deviceName);
-extern _X_EXPORT Bool DRI2Authenticate(ScreenPtr pScreen, drm_magic_t magic);
+extern _X_EXPORT Bool DRI2Authenticate(ScreenPtr pScreen, uint32_t magic);
extern _X_EXPORT int DRI2CreateDrawable(DrawablePtr pDraw);
diff --git a/hw/xfree86/dri2/dri2ext.c b/hw/xfree86/dri2/dri2ext.c
index 17df130..e8fd749 100644
--- a/hw/xfree86/dri2/dri2ext.c
+++ b/hw/xfree86/dri2/dri2ext.c
@@ -42,7 +42,9 @@
#include scrnintstr.h
#include pixmapstr.h
#include extnsionst.h
+#ifdef WITH_LIBDRM
#include xf86drm.h
+#endif
#include xfixes.h
#include dri2.h
#include protocol-versions.h
___
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: [PATCHv2] evdev: add 3x3 transformation matrix xinput property for multi-head handling
Hi all, For absolute input devices (E.G. touchscreens) in multi-head setups, we need a way to bind the device to an randr output. This adds the infrastructure to evdev to allow us to do so. I like the idea, but just if you haven't considered it: more or less the same thing could be done in dix (GetPointerEvents), where it would stand a remote chance to sync with randr, if desired. Also, in dix one would have more control when creating synthetic events, properly do Xi2 raw/screen coords etc. Not striking, but it may be better in the long run. Cheers, Simon Am 21.04.2010 08:47, schrieb Peter Korsgaard: The X server scales input coordinates to the dimensions of the shared (total) frame buffer, so to restrict motion to an output we need to scale/rotate/translate device coordinates to a subset of the frame buffer before passing them on to the X server. This is done here using a 3x3 transformation matrix, which is applied to the device coordinates using homogeneous coordinates, E.G.: [ c0 c1 c2 ] [ x ] [ c3 c4 c5 ] * [ y ] [ c6 c7 c8 ] * [ 1 ] Notice: As input devices have varying input ranges, the coordinates are first scaled to the [0..1] range for generality, and afterwards scaled back up. E.G. for a dual head setup (using same resolution) next to each other, you would want to scale the X coordinates of touchscreen connected to the both heads by 50%, and translate (offset) the coordinates of the rightmost head by 50%, or in matrix form: left: right: [ 0.5 0 0 ] [ 0.5 0 0.5 ] [ 0 1 0 ] [ 0 1 0 ] [ 0 0 1 ] [ 0 0 0 ] Which can be done using xinput: xinput set-prop left --type=float Evdev Coordinate Transformation Matrix \ 0.5 0 0 0 1 0 0 0 1 xinput set-prop right --type=float Evdev Coordinate Transformation Matrix \ 0.5 0 0.5 0 1 0 0 0 1 This is general enough to replace the various tweaks we have in evdev (InvertX/Y, Calibration, SwapAxes), but I have left them for now. Signed-off-by: Peter Korsgaard peter.korsga...@barco.com Acked-by: Oliver McFadden oliver.mcfad...@nokia.com --- Changes since V1: - Add missing xfree() noticed by Oliver include/evdev-properties.h |7 ++ man/evdev.man | 13 src/evdev.c| 139 +++- src/evdev.h|2 + 4 files changed, 160 insertions(+), 1 deletions(-) diff --git a/include/evdev-properties.h b/include/evdev-properties.h index 7df2876..7417a5b 100644 --- a/include/evdev-properties.h +++ b/include/evdev-properties.h @@ -66,4 +66,11 @@ /* BOOL */ #define EVDEV_PROP_SWAP_AXES Evdev Axes Swap +/* Coordinate transformation matrix */ +/* FLOAT, 9 values in row-major order, coordinates in 0..1 range: */ +/* [c0 c1 c2] [x] */ +/* [c5 c4 c5] * [y] */ +/* [c6 c7 c8] [1] */ +#define EVDEV_PROP_TRANSFORM Evdev Coordinate Transformation Matrix + #endif diff --git a/man/evdev.man b/man/evdev.man index 49ab12c..21d7aa4 100644 --- a/man/evdev.man +++ b/man/evdev.man @@ -161,6 +161,15 @@ originally reported by the kernel (e.g. touchscreens). The scaling to the custom coordinate system is done in-driver and the X server is unaware of the transformation. Property: Evdev Axis Calibration. .TP 7 +.BI Option \*qTransformationMatrix\*q \*q c0 c1 c2 c3 c4 c5 c6 c7 c8 \*q +Applies the 3x3 transformation matrix defined by c0..c8 in row-major +order to the device coordinates before passing them on to the X +server. The coefficients are floating point values, and the +coordinates are scaled to the 0..1 range before transformed. This +feature can be used to restrict absolute devices (e.g. touchscreens) +to specific outputs in multi-head setups. Property: Evdev Coordinate +Transformation Matrix. +.TP 7 .B Option \*qMode\*q \*qRelative\*q\fP|\fP\*qAbsolute\*q Sets the mode of the device if device has absolute axes. The default value for touchpads is relative, for other absolute. @@ -196,6 +205,10 @@ driver. 4 32-bit values, order min-x, max-x, min-y, max-y or 0 values to disable in-driver axis calibration. .TP 7 +.BI Evdev Coordinate Transformation Matrix +9 floating point values, defining the coordinate transformation matrix +in row-major order. +.TP 7 .BI Evdev Axis Inversion 2 boolean values (8 bit, 0 or 1), order X, Y. 1 inverts the axis. .TP 7 diff --git a/src/evdev.c b/src/evdev.c index 0678edf..31da4bf 100644 --- a/src/evdev.c +++ b/src/evdev.c @@ -32,6 +32,8 @@ #endif #include xorg-server.h +#include math.h + #include X11/keysym.h #include X11/extensions/XI.h @@ -119,6 +121,7 @@ static Atom prop_calibration = 0; static Atom prop_swap = 0; static Atom prop_axis_label = 0; static Atom prop_btn_label = 0; +static Atom prop_transform = 0; #endif /* All devices the evdev driver has allocated and knows about. @@ -347,6 +350,35 @@ EvdevQueueButtonClicks(InputInfoPtr pInfo, int button,
Re: Xv to pixmaps (was Xv Xdbe combination)
On 20-04-10 11:15, Michel Dänzer wrote: One unexpected thing I found was that the (x,y) of the drawable, when it was a pixmap, was not (0,0) but an apparently random number. This may point to a problem with what I have done, but it is worked around (see xf86xv.c:1819). Are you using XAA? Textured video to pixmaps can't work reliably with that. Use EXA (or UXA with newer versions of the intel driver). I have been using XAA, but it doesn't work when using EXA either One other potential problem I notice is that xf86XVDestroyPixmap() always removes all pixmap privates. In contrast to windows, pixmaps are reference counted; pScreen-DestroyPixmap decreases the reference count and only actually destroys the pixmap when it goes to 0. So xf86XVDestroyPixmap() should only remove the privates etc. when the reference count is 1, in which case the lower layers will decrease it to 0 and destroy the pixmap. Thanks. I didn't see that and have now fixed it. Also, the patch just removes the pDraw-type != DRAWABLE_WINDOW checks in a couple of places where the surrounding code still assumes that the drawable is a window. Maybe those cases aren't immediately relevant for you though. Yes, they are not being called at the moment. I would need to either reinsert them or implement them if we get to the point where a patch can be submitted. It might be possible to make the patch less invasive overall by changing xf86XVEnlistPortIn/RemovePortFromWindow() to xf86XVEnlistPortIn/RemovePortFromDrawable() which handle the drawable type internally. Good idea. I have a few questions: 1. Does REGION_INIT/REGION_UNINIT need to be called when using a pixmap? 2. Any reason why the drawable (x,y) is not (0,0) when it is a pixmap - should it be? The position seems to vary quite a bit, and the numbers seem quite large (2000). 3. Pixmap memory - is it card or system memory? Currently the pixmap is just black where I expect the image to have been drawn to the pixmap. I think this is related to the (x,y) of the drawable, but maybe others have other ideas. Thanks again. Matthew ___ 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: [PATCHv2] evdev: add 3x3 transformation matrix xinput property for multi-head handling
Simon == Simon Thum simon.t...@gmx.de writes: Simon Hi all, For absolute input devices (E.G. touchscreens) in multi-head setups, we need a way to bind the device to an randr output. This adds the infrastructure to evdev to allow us to do so. Simon I like the idea, but just if you haven't considered it: more or Simon less the same thing could be done in dix (GetPointerEvents), Simon where it would stand a remote chance to sync with randr, if Simon desired. Also, in dix one would have more control when creating Simon synthetic events, properly do Xi2 raw/screen coords etc. That was also my first idea, but I discussed it with Peter on IRC, and he preferred to keep it outside the server - to not hardcode any policy. With the current implementation we just have the most basic infrastructure in evdev, and the policy of how to handle touchscreens (and figuring which ones are connected to what randr outputs) is outside - I'm looking into adding support for it in gnome-display-properties now. I would furthermore like to support existing customers with touchscreens, and it is simpler to just get them to bump evdev than updating the server on whatever distribution they might be using. Thanks for your input. -- Bye, Peter Korsgaard ___ 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 util-macros] XORG_GIT_MODULE_VERSION: writes module version in xorg-git-version.h
On Tue, Apr 20, 2010 at 11:10 AM, Gaetan Nadon mems...@videotron.ca wrote: On Mon, 2010-04-19 at 21:34 -0700, Dan Nicholson wrote: On Mon, Apr 19, 2010 at 5:54 PM, Peter Hutterer peter.hutte...@who-t.net wrote: On Mon, Apr 19, 2010 at 11:00:08PM +0200, Julien Cristau wrote: On Mon, Apr 19, 2010 at 14:52:55 -0400, Gaetan Nadon wrote: Generates the git module version according to the git describe HEAD If the git module has pending changes, it appends -dirty to the version tag Signed-off-by: Gaetan Nadon mems...@videotron.ca --- xorg-macros.m4.in | 18 ++ 1 files changed, 18 insertions(+), 0 deletions(-) diff --git a/xorg-macros.m4.in b/xorg-macros.m4.in index 916b472..efb2e53 100644 --- a/xorg-macros.m4.in +++ b/xorg-macros.m4.in @@ -1009,3 +1009,21 @@ mv \$(top_srcdir)/.INSTALL.tmp \$(top_srcdir)/INSTALL) \ echo 'util-macros \pkgdatadir\ from xorg-macros.pc not found: installing possibly empty INSTALL.' 2) AC_SUBST([INSTALL_CMD]) ]) # XORG_INSTALL + +# XORG_GIT_MODULE_VERSION() +# - +# Minimum version: 1.8.0 +# +# Generates the git module version according to the git describe HEAD +# If the git module has pending changes, it appends -dirty to the version tag +# +AC_DEFUN([XORG_GIT_MODULE_VERSION], [ +GIT_MODULE_VERSION_CMD=VER=\`GIT_DIR=\$(top_srcdir)/.git git describe HEAD 2/dev/null\`; \ +DVER=\`GIT_DIR=\$(top_srcdir)/.git git diff-index HEAD 2/dev/null\`; \ +OUTSTR=\\#undef XORG_GIT_VERSION\ ; \ +OUTFILE=\xorg-git-version.h\; \ +test -n \\$\$VER\ OUTSTR=\\#define XORG_GIT_VERSION \$\$VER\ test -n \\$\$DVER\ OUTSTR=\\#define XORG_GIT_VERSION \$\$VER-dirty\; \ +test -e \\$\$OUTFILE\ || echo \\$\$OUTSTR\ \\$\$OUTFILE\; \ +CONTENT=\`cat \$\$OUTFILE\` test \\$\$CONTENT\ = \\$\$OUTSTR\ || echo \$\$OUTSTR \$\$OUTFILE; +AC_SUBST([GIT_MODULE_VERSION_CMD]) +]) # XORG_GIT_MODULE_VERSION my eyes! As I said in reply to the initial patch by Peter I'd like a way to disable this, because packages might be built from a git tree, which might be unrelated to the xorg one, or outside of any git tree. What's the output like if not building from git? Does 'make GIT_MODULE_VERSION_CMD=:' work to disable it (I guess not, because anything trying to include xorg-git-version.h will be unhappy)? it simply sets the #undef and nothing will be printed to the log file. Is there any specific argument against _running_ the macro as long if it doesn't add anything to the logfile? Gaetan: I'm not a big fan of the -dirty either but I guess if others want it we can leave it in. Two questions though: - If modules start using this macro, do they stillhave to add to DISTCLEANFILES and friends? I think it would be run all the time depending on .git/HEAD (or .git/`git symbolic-ref HEAD` like krh suggested), but it entirely depends what you do in the Makefile.am. - This macro only gets invoked on automake runs, right? So if I pull and just rebuild, would it update the git version? It depends how fancy you want to be with the Makefile.am. To me, the following is the right way to do it, but it requires touching more files. configure.ac: AM_CONDITIONAL([USING_GIT], [test -f $srcdir/.git/HEAD]) Makefile.am: if USING_GIT GIT_HEAD = $(top_srcdir)/.git/HEAD endif noinst_HEADERS = git-xorg-version.h git-xorg-version.h: $(GIT_HEAD) $(AM_V_GEN)$(GIT_MODULE_VERSION_CMD) $@ If you have a checkout, the header depends on .git/HEAD, so it will get rebuilt any time HEAD gets updated. Actually, that doesn't seem like it would work that well since HEAD seems to take the modification time of the ref it's following, so the header wouldn't get updated if you checked out an older branch. Probably best would be: noinst_HEADERS = git-xorg-version.h git-xorg-version.h: $(AM_V_GEN)$(GIT_MODULE_VERSION_CMD) $@ .PHONY: git-xorg-version.h Sounds the best to me. I would put the generated file in DISTCLEANFILES. It will cause less rebuilds than if it is in CLEANFILES, in the case the generated .h file is included by millions of other .h files and you just clean one subtree. Yeah, I think you'll need to, anyway, to pass distcheck. And then you'd just have to depend on GIT_MODULE_VERSION_CMD not updating the file unnecessarily to prevent spurious rebuilds. Gaeton's version seems to do that. It does. I posted the patch v2 yesterday. Is the usage of $@ is correct? VER=`GIT_DIR=$(top_srcdir)/.git git describe HEAD 2/dev/null`; DVER=`GIT_DIR=$(top_srcdir)/. git git diff-index HEAD 2/dev/null`; OUTSTR=\#undef XORG_GIT_VERSION ; OUTFILE=$(@); test -n $$VER OUTSTR=\#define XORG_GIT_VERSION $$VER; test -n $$DVER OUTSTR=$$OUTSTR-with-uncommitted-changes; test -e $$OUTFILE || echo $$OUTSTR $$OUTFILE; CONTENT=`cat $$OUTFILE` test $$CONTENT = $$OUTSTR || echo $$OUTSTR $$OUTFILE; I would personally replace all occurrences of $$OUTFILE with
Re: [PATCH v3 3/4] xfree86/dri2: wrap drm bits with macros and change drm_magic type
On Wed, Apr 21, 2010 at 03:00:52PM +0200, ext Mark Kettenis wrote: So instead you decide to increase the maintenance burden on everybody else by making the code harder to read and requiring them to remember to wrap new code that depends on libdrm in more #ifdefs. As I said, it's a trade-off. Customize an Xorg for a given system is only being discussed recently. I'd like to see an implementation very small and with few code, so I can maintain and enhance easily. Besides, Xorg is open-source and we should try discuss ways to embrace everyone's willings. Anyway, I'm pretty sure you understand my point of view. I'm totally opened to hear best approaches how to circumvent this. Tiago ___ 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 v3 3/4] xfree86/dri2: wrap drm bits with macros and change drm_magic type
On 21/04/10 15:32 +0200, Vignatti Tiago wrote: On Wed, Apr 21, 2010 at 03:00:52PM +0200, Mark Kettenis wrote: So instead you decide to increase the maintenance burden on everybody else by making the code harder to read and requiring them to remember to wrap new code that depends on libdrm in more #ifdefs. As I said, it's a trade-off. Customize an Xorg for a given system is only being discussed recently. I'd like to see an implementation very small and with few code, so I can maintain and enhance easily. Besides, Xorg is open-source and we should try discuss ways to embrace everyone's willings. Anyway, I'm pretty sure you understand my point of view. I'm totally opened to hear best approaches how to circumvent this. How about dropping libdrm dependency from dri2? There is nothing else except drmAuthMagic used from libdrm for dri2. Pauli ___ 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
[PULL] Death to MBE and Coverity findings
Hi Keith, You will see here three sets of work, in this order: - the death to Multibuffer extension - a tiny patch to remove unused macro in configure - several fix from Coverity static analysis tool Thanks, The following changes since commit b3ab978df861c08298f57529e3db980489055c35: Keith Packard (1): Merge remote branch 'whot/for-keith' are available in the git repository at: git://people.freedesktop.org/~vignatti/xserver.git for-keith Tiago Vignatti (12): Death to Multibuffer extension configure: remove unused builtin font macro from autoconf file exa: check for NULL pointer before dereferences it mi: check for NULL pointer before dereferences it in miPointerSetPosition xfree86: fix not reached code in parser Xi: fix not reached code in XSendExtensionEvent xfree86: check for NULL pointer before dereferences it in parser code Xi: check for NULL pointer before dereferences it in ListButtonInfo xkb: check for NULL pointer before dereferences it in XkbAddClientResource exa: don't need to check for NULL pointer if we already assumed it has a value xkb: check for NULL pointer before dereferences it in XkbWriteXKBSymbols xfree86: fix not reached code in tty code Xext/Makefile.am |9 - Xext/mbuf.c | 2014 -- Xext/mbufbf.c| 1007 --- Xext/mbufpx.c| 648 -- Xext/sync.c |2 +- Xext/xtest.c |3 +- Xi/sendexev.c|3 +- Xi/xiquerydevice.c |5 +- configure.ac |7 - exa/exa.c| 14 +- exa/exa_classic.c|3 +- exa/exa_driver.c |3 +- exa/exa_mixed.c |3 +- hw/xfree86/dixmods/extmod/modinit.c |9 - hw/xfree86/dixmods/extmod/modinit.h |5 - hw/xfree86/os-support/shared/posix_tty.c |5 - hw/xfree86/parser/read.c |4 +- hw/xfree86/parser/scan.c |7 +- include/dix-config.h.in |6 - include/globals.h|4 - mi/miinitext.c | 13 - mi/mipointer.c |6 +- os/utils.c |3 - xkb/xkbEvents.c |3 +- xkb/xkbout.c |9 +- 25 files changed, 39 insertions(+), 3756 deletions(-) delete mode 100644 Xext/mbuf.c delete mode 100644 Xext/mbufbf.c delete mode 100644 Xext/mbufpx.c ___ 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] mi: Add some sanity checking to miPaintWindow
Signed-off-by: Jeremy Huddleston jerem...@apple.com
---
mi/miexpose.c |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/mi/miexpose.c b/mi/miexpose.c
index 1c9c3a4..1a7cd0c 100644
--- a/mi/miexpose.c
+++ b/mi/miexpose.c
@@ -551,6 +551,9 @@ miPaintWindow(WindowPtr pWin, RegionPtr prgn, int what)
Bool solid = TRUE;
DrawablePtrdrawable = pWin-drawable;
+if(!drawable || drawable-type == UNDRAWABLE_WINDOW)
+ return;
+
#ifdef ROOTLESS
if(IsFramedWindow(pWin)) {
RootlessStartDrawing(pWin);
--
1.7.0.4
___
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] xfree86: fix not reached code in tty code
On Wed, Apr 21, 2010 at 12:19:07AM +0200, ext Peter Hutterer wrote: stack the reviewed patches into a for-keith branch, push them to your $HOME on people.freedesktop.org and then send Keith a pull request. what I usually do is 'git request-pull master git://people.freedesktop.org/~whot/xserver.git for-keith pull-req' and then 'mutt -H pull-req' note that this requires master to be something close-ish to upstream, if you've diverged in other ways just insert the last sha1 instead of master. in my case, master is usually origin/master from where I started development, so origin/master may have moved on since. given that you already have the patches in a tree, it's faster to do this than to wait for Keith to apply and push them, he has pull requests on precedence AFAICT. very useful information, Peter. Thanks. I was thinking maybe to drop this in our wiki. Do you think there's a better place than here: http://wiki.x.org/wiki/Development/Documentation/SubmittingPatches Cheers, Tiago ___ 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
jhbuild (was Live builds (was: Merged proto package))
On 20 April 2010 23:44, Peter Hutterer peter.hutte...@who-t.net wrote: On Tue, Apr 20, 2010 at 11:29:28PM +0100, David Gerard wrote: [ re: http://www.x.org/wiki/JhBuildInstructions ] Yes, if you look at the page you'll see I grovelled through setting it up from scratch on several distros! (I even tried on Solaris 10, but it was so much pain I just gave up ...) in that case - thanks for your efforts! seriously, I'd love it if everyone did that. we hear (or used to hear, anyway) a lot of complaints about complicated setup processes and lack of documentation but never had the followups on the wiki to fix this. so I quite appreciate it that you followed through and documented it for others. One thing it really shows is the pervasiveness of the assumption that the whole world is a Linux box. Of course, it isn't - so a helpful thing would be for those not using Linux to show how to build jhbuild for their OSes. Basically, I could only beat it into building on Ubuntu, Debian and Fedora ... couldn't manage it on FreeBSD, Solaris 10 or Cygwin. (And presumably a better jhbuild for Xorg will mean a better jhbuild for GNOME on those platforms.) - d. ___ 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: please test: Xlib/XCB fixes
Am 19.04.2010 20:26, schrieb Jamey Sharp: Yes, I used to know someone who used Promela/Spin, and that would be awesome. We actually have proofs or proof sketches for parts of XCB, so for example I am Confident that pure XCB applications will never fail to insert GetInputFocus requests when needed to maintain sequence number synchronization, and also that it will always do so at the last possible instant. (I can also prove by counterexample that Xlib doesn't have either of these properties :-/ which would require huge ABI changes to fix.) We haven't used model-checkers for that, just logic and Hoare triples or Z. Nice! Is that published/documented somewhere? -- Simon ___ 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 v3 3/4] xfree86/dri2: wrap drm bits with macros and change drm_magic type
On Wed, Apr 21, 2010 at 9:00 AM, Mark Kettenis mark.kette...@xs4all.nl wrote: Date: Wed, 21 Apr 2010 15:34:48 +0300 From: Tiago Vignatti tiago.vigna...@nokia.com On Wed, Apr 21, 2010 at 11:53:04AM +0200, ext Mark Kettenis wrote: I'll reiterate an opinion stated here before. We should really try to *reduce* the number of #ifdefs in the code base, not introduce new ones. In general you're right. We prefer clear code without these ugly #ifdefs all around. But this is not always the case and, I agree, a good justification is needed to introduce them. In our case, we don't want to employ libdrm in the device because it's simply not used. I don't care about the space that this library uses there, but I care about the maintenance that I'd have to do. And that counts a lot. So instead you decide to increase the maintenance burden on everybody else by making the code harder to read and requiring them to remember to wrap new code that depends on libdrm in more #ifdefs. Are you doing this for other people's benefit? I can only find one commit from you to the xserver (06c0372c3a1b45005) and it was more than two years ago. I didn't search very hard though. Matt ___ 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 v3 3/4] xfree86/dri2: wrap drm bits with macros and change drm_magic type
On Wed, Apr 21, 2010 at 4:40 AM, Pauli Nieminen ext-pauli.niemi...@nokia.com wrote: From: Tiago Vignatti tiago.vigna...@nokia.com Signed-off-by: Tiago Vignatti tiago.vigna...@nokia.com Signed-off-by: Pauli Nieminen ext-pauli.niemi...@nokia.com The whole series looks like it's got the _ext_-pauli... in the email address. Matt ___ 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 v3 3/4] xfree86/dri2: wrap drm bits with macros and change drm_magic type
Twas brillig at 11:40:37 21.04.2010 UTC+03 when ext-pauli.niemi...@nokia.com did gyre and gimble: PN diff --git a/hw/xfree86/dri2/dri2ext.c b/hw/xfree86/dri2/dri2ext.c PN index 17df130..e8fd749 100644 PN --- a/hw/xfree86/dri2/dri2ext.c PN +++ b/hw/xfree86/dri2/dri2ext.c PN @@ -42,7 +42,9 @@ PN #include scrnintstr.h PN #include pixmapstr.h PN #include extnsionst.h PN +#ifdef WITH_LIBDRM PN #include xf86drm.h PN +#endif PN #include xfixes.h PN #include dri2.h PN #include protocol-versions.h Actually I did not check it, but this hunk looks suspicious - why do you need to conditionally include header in .c when there are no #ifdefs below? -- http://fossarchy.blogspot.com/ pgpCqOKVtmFdF.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: please test: Xlib/XCB fixes
On Wed, Apr 21, 2010 at 10:11 AM, Simon Thum simon.t...@gmx.de wrote: Am 19.04.2010 20:26, schrieb Jamey Sharp: We actually have proofs or proof sketches for parts of XCB, so for example I am Confident that pure XCB applications will never fail to insert GetInputFocus requests when needed to maintain sequence number synchronization, and also that it will always do so at the last possible instant. (I can also prove by counterexample that Xlib doesn't have either of these properties :-/ which would require huge ABI changes to fix.) We haven't used model-checkers for that, just logic and Hoare triples or Z. Nice! Is that published/documented somewhere? This thread has reminded me that I never got around to writing up the proof, so I have a draft in-progress now. A different proof about XCB is published in X meets Z (http://xcb.freedesktop.org/Publications/), which I'd recommend to anyone unfamiliar with lightweight formal methods as a good introduction. Unfortunately XCB doesn't actually use quite the algorithm it describes, and last time I looked I had some concern about whether the proof was correct. :-/ But it does correctly explain one of the problems we encountered that was so hard that we wanted a proof about it. Jamey ___ 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 v3 3/4] xfree86/dri2: wrap drm bits with macros and change drm_magic type
Matt Turner wrote: On Wed, Apr 21, 2010 at 4:40 AM, Pauli Nieminen ext-pauli.niemi...@nokia.com wrote: From: Tiago Vignatti tiago.vigna...@nokia.com Signed-off-by: Tiago Vignatti tiago.vigna...@nokia.com Signed-off-by: Pauli Nieminen ext-pauli.niemi...@nokia.com The whole series looks like it's got the _ext_-pauli... in the email address. it's correct. This is the address of nokia's subcontractors (sort of discrimination, I agree!). Tiago ___ 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 util-macros] XORG_ENABLE_DEBUG: a configure option to enable code debugging
On Wed, 2010-04-21 at 11:22 +1000, Peter Hutterer wrote:
On Tue, Apr 20, 2010 at 08:32:46PM -0400, Gaetan Nadon wrote:
Provides a common configure option for all x.org modules.
Script friendly for turning on debug in several modules.
Integrates easily into existing modules debug code.
For both source code and makefiles
Configurable on/off default setting.
Signed-off-by: Gaetan Nadon mems...@videotron.ca
---
xorg-macros.m4.in | 32
1 files changed, 32 insertions(+), 0 deletions(-)
diff --git a/xorg-macros.m4.in b/xorg-macros.m4.in
index 66c8e98..112db89 100644
--- a/xorg-macros.m4.in
+++ b/xorg-macros.m4.in
@@ -1029,3 +1029,35 @@ test -e \\$\$OUTFILE\ || echo \\$\$OUTSTR\
\\$\$OUTFILE\; \
CONTENT=\`cat \$\$OUTFILE\` test \\$\$CONTENT\ = \\$\$OUTSTR\ ||
echo \$\$OUTSTR \$\$OUTFILE;
AC_SUBST([GIT_MODULE_VERSION_CMD])
]) # XORG_GIT_MODULE_VERSION
+
+# XORG_ENABLE_DEBUG (enable_debug=no)
+# ---
+# Minimum version: 1.8.0
+#
+# This macro defines a C preprocessor macro and a makefile conditional
+# to compile code for debugging purpose. (Not related to compiler options)
+#
+# Interface to module:
+# HAVE_DEBUG:used in makefiles to conditionally build targets
+#used in source code to conditionally debug code
+# --enable-debug: 'yes' user instructs the module to compile debug code
+# 'no' user instructs the module not to compile debug code
+# parm1: specify the default value, yes or no.
+#
+AC_DEFUN([XORG_ENABLE_DEBUG],[
+default=$1
+if test x$default = x ; then
+ default=no
+fi
+AC_ARG_ENABLE([debug],
+ AS_HELP_STRING([--enable-debug],
+ [Enable debugging code (default: no)]),
+ [enable_debug=$enableval], [enable_debug=$default])
+
+if test x$enable_debug = xyes; then
+AC_DEFINE([HAVE_DEBUG], [1], [Enable debugging code])
+fi
+AM_CONDITIONAL([HAVE_DEBUG], [test x$enable_debug = xyes])
+AC_MSG_CHECKING([whether to enable debug code])
+AC_MSG_RESULT([$enable_debug])
+]) # XORG_ENABLE_DEBUG
--
1.6.0.4
There are 4 modules with --enable-debug configure option but many more
modules (44) do
have something like #define DEBUG with a wide variety of names. One has to
dig in the code
to figure out how each module does it.
This macro will give the opportunity for modules to have a common configure
option where
debug can be turned on without having to read all the code and to change it.
It was tempting to use DEBUG as the #define name, but there were already
conflicts.
can you specify what those conflicts were?
Existing definitions in various drivers
#define DEBUG(str,param1) (void)screen
#define DEBUG(x)
#define DEBUG
#undef DEBUG
the use of ifdef DEBUG is quite common and having it turned on/off by a
configure option isn't really a conflict if the package didn't do it before.
No conflict from a configure.ac perspective.
I'd even argue that this macro should go into the XORG_DEFAULT_OPTIONS.
Sure.
I checked HAVE_DEBUG was not already used so it's easy to wrap whatever
#define is there
without changing much code.
Except for the 4 modules having a configure options, others edit the c
file to turn debug on.
For example, aiptek has:
#define DEBUG 1
#if DEBUG
# define DBG(lvl, f) {if ((lvl) = debug_level) f;}
#else
# define DBG(lvl, f)
#endif
which can be changed to:
#ifdef HAVE_DEBUG
#define DEBUG 1
#endif
#if DEBUG
# define DBG(lvl, f) {if ((lvl) = debug_level) f;}
#else
# define DBG(lvl, f)
#endif
which does not change the rest of the code and prevents conversion
errors.
Cheers,
Peter
signature.asc
Description: This is a digitally signed message part
___
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] Revert mi: don't thrash resources when displaying the software cursor across screens
Oh, so that was the reason behind the This has been free()d before comment
next to the FreePicture calls in the earlier version of that code. Sorry I
didn't track that down before reinstating them. Since FreeAllResources always
cleans up before CloseDownDevices, the attached patch should fix the invalid
reads and prevent any corruption.
Thanks,
- Pierre-Loup
On 04/20/2010 06:47 PM, Peter Hutterer wrote:
This commit leads to a segfault on the very first XTS test case.
Backtrace:
0: /opt/xorg/bin/Xorg (xorg_backtrace+0x3b) [0x80a33db]
1: /opt/xorg/bin/Xorg (0x8048000+0x62a75) [0x80aaa75]
2: (vdso) (__kernel_rt_sigreturn+0x0) [0x5d140c]
3: /lib/libc.so.6 (0x9bb000+0x73579) [0xa2e579]
4: /lib/libc.so.6 (realloc+0xe0) [0xa2e830]
5: /opt/xorg/bin/Xorg (Xrealloc+0x33) [0x80a3f33]
6: /opt/xorg/bin/Xorg (0x8048000+0x1ab79) [0x8062b79]
7: /opt/xorg/bin/Xorg (0x8048000+0x1ac4e) [0x8062c4e]
8: /opt/xorg/bin/Xorg (RegisterExtensionNames+0x2ce) [0x8062fbe]
9: /opt/xorg/bin/Xorg (AddExtension+0x19a) [0x807bd7a]
10: /opt/xorg//lib/xorg/modules/extensions/libextmod.so (0x728000+0x1169a)
[0x73969a]
11: /opt/xorg/bin/Xorg (InitExtensions+0x85) [0x80c0eb5]
12: /opt/xorg/bin/Xorg (0x8048000+0x1a51d) [0x806251d]
13: /lib/libc.so.6 (__libc_start_main+0xe6) [0x9d1bb6]
14: /opt/xorg/bin/Xorg (0x8048000+0x1a2a1) [0x80622a1]
Segmentation fault at address 0x10b2d5f8
valgrind output:
==5069== Invalid read of size 4
==5069==at 0x80F928D: FreePicture (picture.c:1531)
==5069==by 0x818DDEF: miDCDeviceCleanup (midispcur.c:867)
==5069==by 0x81B97F0: miSpriteDeviceCursorCleanup (misprite.c:968)
==5069==by 0x80995FA: miPointerDeviceCleanup (mipointer.c:292)
==5069==by 0x807973E: CloseDevice (devices.c:840)
==5069==by 0x80799B6: CloseDownDevices (devices.c:933)
==5069==by 0x8062705: main (main.c:309)
==5069== Address 0x4cce844 is 12 bytes inside a block of size 84 free'd
==5069==at 0x40057F6: free (vg_replace_malloc.c:325)
==5069==by 0x80A3DE0: Xfree (utils.c:1154)
==5069==by 0x80F9332: FreePicture (picture.c:1576)
==5069==by 0x80FBB4B: PictureDestroyWindow (picture.c:69)
==5069==by 0x810B1A3: damageDestroyWindow (damage.c:1840)
==5069==by 0x80864F1: FreeWindowResources (window.c:846)
==5069==by 0x8086812: DeleteWindow (window.c:925)
==5069==by 0x806B53E: FreeClientResources (resource.c:806)
==5069==by 0x806B60F: FreeAllResources (resource.c:823)
==5069==by 0x80626E4: main (main.c:299)
==5069==
==5069== Invalid write of size 4
==5069==at 0x80F9295: FreePicture (picture.c:1531)
==5069==by 0x818DDEF: miDCDeviceCleanup (midispcur.c:867)
==5069==by 0x81B97F0: miSpriteDeviceCursorCleanup (misprite.c:968)
==5069==by 0x80995FA: miPointerDeviceCleanup (mipointer.c:292)
==5069==by 0x807973E: CloseDevice (devices.c:840)
==5069==by 0x80799B6: CloseDownDevices (devices.c:933)
==5069==by 0x8062705: main (main.c:309)
==5069== Address 0x4cce844 is 12 bytes inside a block of size 84 free'd
==5069==at 0x40057F6: free (vg_replace_malloc.c:325)
==5069==by 0x80A3DE0: Xfree (utils.c:1154)
==5069==by 0x80F9332: FreePicture (picture.c:1576)
==5069==by 0x80FBB4B: PictureDestroyWindow (picture.c:69)
==5069==by 0x810B1A3: damageDestroyWindow (damage.c:1840)
==5069==by 0x80864F1: FreeWindowResources (window.c:846)
==5069==by 0x8086812: DeleteWindow (window.c:925)
==5069==by 0x806B53E: FreeClientResources (resource.c:806)
==5069==by 0x806B60F: FreeAllResources (resource.c:823)
==5069==by 0x80626E4: main (main.c:299)
XTS test case: Xproto pAllocColor
This reverts commit 00b8b7ad61b6f818271fb4d1e383113170309d72.
Signed-off-by: Peter Huttererpeter.hutte...@who-t.net
---
mi/midispcur.c | 270 +---
1 files changed, 159 insertions(+), 111 deletions(-)
diff --git a/mi/midispcur.c b/mi/midispcur.c
index 3a31b74..3fb7e02 100644
--- a/mi/midispcur.c
+++ b/mi/midispcur.c
@@ -59,9 +59,9 @@ static DevPrivateKey miDCScreenKey =miDCScreenKeyIndex;
static BoolmiDCCloseScreen(int index, ScreenPtr pScreen);
-/* per device per-screen private data */
-static int miDCSpriteKeyIndex[MAXSCREENS];
-static DevPrivateKey miDCSpriteKey = miDCSpriteKeyIndex;
+/* per device private data */
+static int miDCSpriteKeyIndex;
+static DevPrivateKey miDCSpriteKey =miDCSpriteKeyIndex;
typedef struct {
GCPtr pSourceGC, pMaskGC;
@@ -75,10 +75,10 @@ typedef struct {
#endif
} miDCBufferRec, *miDCBufferPtr;
-#define MIDCBUFFER(dev, screen) \
+#define MIDCBUFFER(dev) \
((DevHasCursor(dev)) ? \
- (miDCBufferPtr)dixLookupPrivate(dev-devPrivates, miDCSpriteKey +
(screen)-myNum) : \
- (miDCBufferPtr)dixLookupPrivate(dev-u.master-devPrivates, miDCSpriteKey +
(screen)-myNum))
+ (miDCBufferPtr)dixLookupPrivate(dev-devPrivates, miDCSpriteKey) : \
+ (miDCBufferPtr)dixLookupPrivate(dev-u.master-devPrivates, miDCSpriteKey))
/*
* The core pointer buffer will point to the
Re: [PATCH resent * 2] xfree86: fix not reached code in fi1236 driver from i2c
On Wed, 21 Apr 2010 18:28:13 +0300, Tiago Vignatti tiago.vigna...@nokia.com wrote: This issue was introduced in the first dump of the code in 2004. I haven't check what's the correct fix for it so I simply kept the behaviour of someone calling this and removed the unreachable code. Signed-off-by: Tiago Vignatti tiago.vigna...@nokia.com --- Keith, this patch is the last in my queue of Coverity findings. I sent twice and no one commented so far, so I'm expecting an answer from the RM now. Yeah, I looked at the code and your patch seems fine; this driver polls the hardware until the tuner locks or gives up, recording the result in last_afc_hint, so it seems correct to simply return the most recently received value. Reviewed-by: Keith Packard kei...@keithp.com -- keith.pack...@intel.com pgp8qA0TesKHm.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] dix: check for NULL pointer before dereferences it in DeviceEnterLeaveEvent
This patch also adds this change which isn't really described by the commit description: grab = mouse-deviceGrab.grab; but you forgot to change this: GrabPtr grab = mouse-deviceGrab.grab; to GrabPtr grab; On Apr 19, 2010, at 10:58, Tiago Vignatti wrote: mouse is already used before its checking should be performed. So check on the beginning instead. --- dix/events.c |6 +- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/dix/events.c b/dix/events.c index 6541652..8bec8af 100644 --- a/dix/events.c +++ b/dix/events.c @@ -4360,6 +4360,10 @@ DeviceEnterLeaveEvent( (mode == XINotifyPassiveUngrab type == XI_Enter)) return; +if (!mouse) +return; +grab = mouse-deviceGrab.grab; + btlen = (mouse-button) ? bits_to_bytes(mouse-button-numButtons) : 0; btlen = bytes_to_int32(btlen); len = sizeof(xXIEnterEvent) + btlen * 4; @@ -4378,7 +4382,7 @@ DeviceEnterLeaveEvent( event-root_x = FP1616(mouse-spriteInfo-sprite-hot.x, 0); event-root_y = FP1616(mouse-spriteInfo-sprite-hot.y, 0); -for (i = 0; mouse mouse-button i mouse-button-numButtons; i++) +for (i = 0; mouse-button i mouse-button-numButtons; i++) if (BitIsOn(mouse-button-down, i)) SetBit(event[1], i); -- 1.6.0.4 ___ 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] xkb: check for NULL pointer before dereferences it in XkbAddClientResource
Reviewed-by: Jeremy Huddleston jerem...@apple.com
On Apr 19, 2010, at 10:58, Tiago Vignatti wrote:
---
xkb/xkbEvents.c |3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/xkb/xkbEvents.c b/xkb/xkbEvents.c
index 33741e9..9755f98 100644
--- a/xkb/xkbEvents.c
+++ b/xkb/xkbEvents.c
@@ -1041,8 +1041,7 @@ XkbInterestPtr interest;
return ((interest-resource==id)?interest:NULL);
interest = interest-next;
}
-interest = xalloc(sizeof(XkbInterestRec));
-bzero(interest,sizeof(XkbInterestRec));
+interest = xcalloc(1, sizeof(XkbInterestRec));
if (interest) {
interest-dev = dev;
interest-client = client;
--
1.6.0.4
___
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] xkb: check for NULL pointer before dereferences it in XkbWriteXKBSymbols
Reviewed-by: Jeremy Huddleston jerem...@apple.com
On Apr 19, 2010, at 10:58, Tiago Vignatti wrote:
just moved the srv assignment to before it's being used.
---
xkb/xkbout.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/xkb/xkbout.c b/xkb/xkbout.c
index 68ede90..9daac9a 100644
--- a/xkb/xkbout.c
+++ b/xkb/xkbout.c
@@ -354,7 +354,6 @@ XkbServerMapPtr srv;
Bool showActions;
map= xkb-map;
-srv= xkb-server;
if ((!xkb)||(!map)||(!map-syms)||(!map-key_sym_map)) {
_XkbLibError(_XkbErrMissingSymbols,XkbWriteXKBSymbols,0);
return FALSE;
@@ -376,6 +375,7 @@ Bool showActions;
}
if (tmp0)
fprintf(file,\n);
+srv= xkb-server;
for (i=xkb-min_key_code;i=xkb-max_key_code;i++) {
Boolsimple;
if ((int)XkbKeyNumSyms(xkb,i)1)
--
1.6.0.4
___
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 util-macros] XORG_GIT_MODULE_VERSION: writes module version in xorg-git-version.h
On Wed, 2010-04-21 at 16:06 +1000, Peter Hutterer wrote: On Mon, Apr 19, 2010 at 10:01:06PM -0400, Gaetan Nadon wrote: On Tue, 2010-04-20 at 10:54 +1000, Peter Hutterer wrote: On Mon, Apr 19, 2010 at 11:00:08PM +0200, Julien Cristau wrote: On Mon, Apr 19, 2010 at 14:52:55 -0400, Gaetan Nadon wrote: Generates the git module version according to the git describe HEAD If the git module has pending changes, it appends -dirty to the version tag Signed-off-by: Gaetan Nadon mems...@videotron.ca --- xorg-macros.m4.in | 18 ++ 1 files changed, 18 insertions(+), 0 deletions(-) diff --git a/xorg-macros.m4.in b/xorg-macros.m4.in index 916b472..efb2e53 100644 --- a/xorg-macros.m4.in +++ b/xorg-macros.m4.in @@ -1009,3 +1009,21 @@ mv \$(top_srcdir)/.INSTALL.tmp \$(top_srcdir)/INSTALL) \ echo 'util-macros \pkgdatadir\ from xorg-macros.pc not found: installing possibly empty INSTALL.' 2) AC_SUBST([INSTALL_CMD]) ]) # XORG_INSTALL + +# XORG_GIT_MODULE_VERSION() +# - +# Minimum version: 1.8.0 +# +# Generates the git module version according to the git describe HEAD +# If the git module has pending changes, it appends -dirty to the version tag +# +AC_DEFUN([XORG_GIT_MODULE_VERSION], [ +GIT_MODULE_VERSION_CMD=VER=\`GIT_DIR=\$(top_srcdir)/.git git describe HEAD 2/dev/null\`; \ +DVER=\`GIT_DIR=\$(top_srcdir)/.git git diff-index HEAD 2/dev/null\`; \ +OUTSTR=\\#undef XORG_GIT_VERSION\ ; \ +OUTFILE=\xorg-git-version.h\; \ +test -n \\$\$VER\ OUTSTR=\\#define XORG_GIT_VERSION \$\$VER\ test -n \\$\$DVER\ OUTSTR=\\#define XORG_GIT_VERSION \$\$VER-dirty\; \ +test -e \\$\$OUTFILE\ || echo \\$\$OUTSTR\ \\$\$OUTFILE\; \ +CONTENT=\`cat \$\$OUTFILE\` test \\$\$CONTENT\ = \\$\$OUTSTR\ || echo \$\$OUTSTR \$\$OUTFILE; +AC_SUBST([GIT_MODULE_VERSION_CMD]) +]) # XORG_GIT_MODULE_VERSION my eyes! As I said in reply to the initial patch by Peter I'd like a way to disable this, because packages might be built from a git tree, which might be unrelated to the xorg one, or outside of any git tree. What's the output like if not building from git? Does 'make GIT_MODULE_VERSION_CMD=:' work to disable it (I guess not, because anything trying to include xorg-git-version.h will be unhappy)? it simply sets the #undef and nothing will be printed to the log file. Is there any specific argument against _running_ the macro as long if it doesn't add anything to the logfile? Gaetan: I'm not a big fan of the -dirty either but I guess if others want it we can leave it in. Two questions though: If you remotely help someone debugging a problem, you would like to know if tag/commit is really the code he is running or if there are local changes. With the commit number you can cross-reference the master repo and detect unpsuhed local commits. With the -dirty (now called -with-uncommitted-changes) you now the code is tainted with local changes you know nothing about. It's just additional info. I really hope that when I help someone debugging they'd tell me beforehand if they have local changes. in which case the dirty doesn't help much anyway because you still don't know what the changes do. but yeah, doesn't matter either way, I'm fine with it. - If modules start using this macro, do they stillhave to add to DISTCLEANFILES and friends? - This macro only gets invoked on automake runs, right? So if I pull and just rebuild, would it update the git version? All options are available in the Makefile.am regarding the invocation of the macro (or your original script). I have not paid attention to that, it looks you had it covered. The problem domain is similar to ChangeLog. You need a trigger when a new commit is available, but the best we could do is a dist hook. And you may not be running off git. What use case was this feature designed for? If someone follows git and files a bug (or sends the log) it'll be easy to see which commit they have locally. So questions like do you have commit 1234deadbeef in your local repo aren't needed, reducing the turnaround time. One suggested a dependency on a git file that changes with the HEAD commit, don't recall what the objections were. I'll think about it some more. Acked-by: Peter Hutterer peter.hutte...@who-t.net for version 2 but please make sure that someone else acks it too, automake stuff gives me the creeps. would also be good checking through Matthias' script from radeonhd for any obvious use-cases that this patch doesn't cover. It certainly seems more complex. Same purpose, more features and bullet proof. Has been in use 3 years. Dan later suggested to distribute a script rather
Re: [PATCH util-macros] XORG_GIT_MODULE_VERSION: writes module version in xorg-git-version.h
On Wed, 2010-04-21 at 06:03 -0700, Dan Nicholson wrote: On Tue, Apr 20, 2010 at 11:10 AM, Gaetan Nadon mems...@videotron.ca wrote: On Mon, 2010-04-19 at 21:34 -0700, Dan Nicholson wrote: On Mon, Apr 19, 2010 at 5:54 PM, Peter Hutterer peter.hutte...@who-t.net wrote: On Mon, Apr 19, 2010 at 11:00:08PM +0200, Julien Cristau wrote: On Mon, Apr 19, 2010 at 14:52:55 -0400, Gaetan Nadon wrote: Generates the git module version according to the git describe HEAD If the git module has pending changes, it appends -dirty to the version tag Signed-off-by: Gaetan Nadon mems...@videotron.ca --- xorg-macros.m4.in | 18 ++ 1 files changed, 18 insertions(+), 0 deletions(-) diff --git a/xorg-macros.m4.in b/xorg-macros.m4.in index 916b472..efb2e53 100644 --- a/xorg-macros.m4.in +++ b/xorg-macros.m4.in @@ -1009,3 +1009,21 @@ mv \$(top_srcdir)/.INSTALL.tmp \$(top_srcdir)/INSTALL) \ echo 'util-macros \pkgdatadir\ from xorg-macros.pc not found: installing possibly empty INSTALL.' 2) AC_SUBST([INSTALL_CMD]) ]) # XORG_INSTALL + +# XORG_GIT_MODULE_VERSION() +# - +# Minimum version: 1.8.0 +# +# Generates the git module version according to the git describe HEAD +# If the git module has pending changes, it appends -dirty to the version tag +# +AC_DEFUN([XORG_GIT_MODULE_VERSION], [ +GIT_MODULE_VERSION_CMD=VER=\`GIT_DIR=\$(top_srcdir)/.git git describe HEAD 2/dev/null\`; \ +DVER=\`GIT_DIR=\$(top_srcdir)/.git git diff-index HEAD 2/dev/null\`; \ +OUTSTR=\\#undef XORG_GIT_VERSION\ ; \ +OUTFILE=\xorg-git-version.h\; \ +test -n \\$\$VER\ OUTSTR=\\#define XORG_GIT_VERSION \$\$VER\ test -n \\$\$DVER\ OUTSTR=\\#define XORG_GIT_VERSION \$\$VER-dirty\; \ +test -e \\$\$OUTFILE\ || echo \\$\$OUTSTR\ \\$\$OUTFILE\; \ +CONTENT=\`cat \$\$OUTFILE\` test \\$\$CONTENT\ = \\$\$OUTSTR\ || echo \$\$OUTSTR \$\$OUTFILE; +AC_SUBST([GIT_MODULE_VERSION_CMD]) +]) # XORG_GIT_MODULE_VERSION my eyes! As I said in reply to the initial patch by Peter I'd like a way to disable this, because packages might be built from a git tree, which might be unrelated to the xorg one, or outside of any git tree. What's the output like if not building from git? Does 'make GIT_MODULE_VERSION_CMD=:' work to disable it (I guess not, because anything trying to include xorg-git-version.h will be unhappy)? it simply sets the #undef and nothing will be printed to the log file. Is there any specific argument against _running_ the macro as long if it doesn't add anything to the logfile? Gaetan: I'm not a big fan of the -dirty either but I guess if others want it we can leave it in. Two questions though: - If modules start using this macro, do they stillhave to add to DISTCLEANFILES and friends? I think it would be run all the time depending on .git/HEAD (or .git/`git symbolic-ref HEAD` like krh suggested), but it entirely depends what you do in the Makefile.am. - This macro only gets invoked on automake runs, right? So if I pull and just rebuild, would it update the git version? It depends how fancy you want to be with the Makefile.am. To me, the following is the right way to do it, but it requires touching more files. configure.ac: AM_CONDITIONAL([USING_GIT], [test -f $srcdir/.git/HEAD]) Makefile.am: if USING_GIT GIT_HEAD = $(top_srcdir)/.git/HEAD endif noinst_HEADERS = git-xorg-version.h git-xorg-version.h: $(GIT_HEAD) $(AM_V_GEN)$(GIT_MODULE_VERSION_CMD) $@ If you have a checkout, the header depends on .git/HEAD, so it will get rebuilt any time HEAD gets updated. Actually, that doesn't seem like it would work that well since HEAD seems to take the modification time of the ref it's following, so the header wouldn't get updated if you checked out an older branch. Probably best would be: noinst_HEADERS = git-xorg-version.h git-xorg-version.h: $(AM_V_GEN)$(GIT_MODULE_VERSION_CMD) $@ .PHONY: git-xorg-version.h Sounds the best to me. I would put the generated file in DISTCLEANFILES. It will cause less rebuilds than if it is in CLEANFILES, in the case the generated .h file is included by millions of other .h files and you just clean one subtree. Yeah, I think you'll need to, anyway, to pass distcheck. And then you'd just have to depend on GIT_MODULE_VERSION_CMD not updating the file unnecessarily to prevent spurious rebuilds. Gaeton's version seems to do that. It does. I posted the patch v2 yesterday. Is the usage of $@ is correct? VER=`GIT_DIR=$(top_srcdir)/.git git describe HEAD 2/dev/null`; DVER=`GIT_DIR=$(top_srcdir)/. git git diff-index HEAD 2/dev/null`; OUTSTR=\#undef XORG_GIT_VERSION ; OUTFILE=$(@); test -n $$VER OUTSTR=\#define XORG_GIT_VERSION $$VER; test -n $$DVER OUTSTR=$$OUTSTR-with-uncommitted-changes; test
Re: [PATCH util-macros] XORG_GIT_MODULE_VERSION: writes module version in xorg-git-version.h
On Wed, 2010-04-21 at 16:01 -0400, Gaetan Nadon wrote: On Wed, 2010-04-21 at 06:03 -0700, Dan Nicholson wrote: On Tue, Apr 20, 2010 at 11:10 AM, Gaetan Nadon mems...@videotron.ca wrote: On Mon, 2010-04-19 at 21:34 -0700, Dan Nicholson wrote: On Mon, Apr 19, 2010 at 5:54 PM, Peter Hutterer peter.hutte...@who-t.net wrote: On Mon, Apr 19, 2010 at 11:00:08PM +0200, Julien Cristau wrote: On Mon, Apr 19, 2010 at 14:52:55 -0400, Gaetan Nadon wrote: Generates the git module version according to the git describe HEAD If the git module has pending changes, it appends -dirty to the version tag Signed-off-by: Gaetan Nadon mems...@videotron.ca --- xorg-macros.m4.in | 18 ++ 1 files changed, 18 insertions(+), 0 deletions(-) diff --git a/xorg-macros.m4.in b/xorg-macros.m4.in index 916b472..efb2e53 100644 --- a/xorg-macros.m4.in +++ b/xorg-macros.m4.in @@ -1009,3 +1009,21 @@ mv \$(top_srcdir)/.INSTALL.tmp \$(top_srcdir)/INSTALL) \ echo 'util-macros \pkgdatadir\ from xorg-macros.pc not found: installing possibly empty INSTALL.' 2) AC_SUBST([INSTALL_CMD]) ]) # XORG_INSTALL + +# XORG_GIT_MODULE_VERSION() +# - +# Minimum version: 1.8.0 +# +# Generates the git module version according to the git describe HEAD +# If the git module has pending changes, it appends -dirty to the version tag +# +AC_DEFUN([XORG_GIT_MODULE_VERSION], [ +GIT_MODULE_VERSION_CMD=VER=\`GIT_DIR=\$(top_srcdir)/.git git describe HEAD 2/dev/null\`; \ +DVER=\`GIT_DIR=\$(top_srcdir)/.git git diff-index HEAD 2/dev/null\`; \ +OUTSTR=\\#undef XORG_GIT_VERSION\ ; \ +OUTFILE=\xorg-git-version.h\; \ +test -n \\$\$VER\ OUTSTR=\\#define XORG_GIT_VERSION \$\$VER\ test -n \\$\$DVER\ OUTSTR=\\#define XORG_GIT_VERSION \$\$VER-dirty\; \ +test -e \\$\$OUTFILE\ || echo \\$\$OUTSTR\ \\$\$OUTFILE\; \ +CONTENT=\`cat \$\$OUTFILE\` test \\$\$CONTENT\ = \\$\$OUTSTR\ || echo \$\$OUTSTR \$\$OUTFILE; +AC_SUBST([GIT_MODULE_VERSION_CMD]) +]) # XORG_GIT_MODULE_VERSION my eyes! As I said in reply to the initial patch by Peter I'd like a way to disable this, because packages might be built from a git tree, which might be unrelated to the xorg one, or outside of any git tree. What's the output like if not building from git? Does 'make GIT_MODULE_VERSION_CMD=:' work to disable it (I guess not, because anything trying to include xorg-git-version.h will be unhappy)? it simply sets the #undef and nothing will be printed to the log file. Is there any specific argument against _running_ the macro as long if it doesn't add anything to the logfile? Gaetan: I'm not a big fan of the -dirty either but I guess if others want it we can leave it in. Two questions though: - If modules start using this macro, do they stillhave to add to DISTCLEANFILES and friends? I think it would be run all the time depending on .git/HEAD (or .git/`git symbolic-ref HEAD` like krh suggested), but it entirely depends what you do in the Makefile.am. - This macro only gets invoked on automake runs, right? So if I pull and just rebuild, would it update the git version? It depends how fancy you want to be with the Makefile.am. To me, the following is the right way to do it, but it requires touching more files. configure.ac: AM_CONDITIONAL([USING_GIT], [test -f $srcdir/.git/HEAD]) Makefile.am: if USING_GIT GIT_HEAD = $(top_srcdir)/.git/HEAD endif noinst_HEADERS = git-xorg-version.h git-xorg-version.h: $(GIT_HEAD) $(AM_V_GEN)$(GIT_MODULE_VERSION_CMD) $@ If you have a checkout, the header depends on .git/HEAD, so it will get rebuilt any time HEAD gets updated. Actually, that doesn't seem like it would work that well since HEAD seems to take the modification time of the ref it's following, so the header wouldn't get updated if you checked out an older branch. Probably best would be: noinst_HEADERS = git-xorg-version.h git-xorg-version.h: $(AM_V_GEN)$(GIT_MODULE_VERSION_CMD) $@ .PHONY: git-xorg-version.h Sounds the best to me. I would put the generated file in DISTCLEANFILES. It will cause less rebuilds than if it is in CLEANFILES, in the case the generated .h file is included by millions of other .h files and you just clean one subtree. Yeah, I think you'll need to, anyway, to pass distcheck. And then you'd just have to depend on GIT_MODULE_VERSION_CMD not updating the file unnecessarily to prevent spurious rebuilds. Gaeton's version seems to do that. It does. I posted the patch v2 yesterday. Is the usage of $@ is correct? VER=`GIT_DIR=$(top_srcdir)/.git git describe HEAD 2/dev/null`; DVER=`GIT_DIR=$(top_srcdir)/. git git
[PATCH v3 util-macros] XORG_GIT_MODULE_VERSION: writes module version in xorg-git-version.h
Generates the git module version according to the git describe HEAD If not building under git, #undef XORG_GIT_VERSION is written Signed-off-by: Gaetan Nadon mems...@videotron.ca --- xorg-macros.m4.in | 16 1 files changed, 16 insertions(+), 0 deletions(-) diff --git a/xorg-macros.m4.in b/xorg-macros.m4.in index 916b472..c5f5a5b 100644 --- a/xorg-macros.m4.in +++ b/xorg-macros.m4.in @@ -1009,3 +1009,19 @@ mv \$(top_srcdir)/.INSTALL.tmp \$(top_srcdir)/INSTALL) \ echo 'util-macros \pkgdatadir\ from xorg-macros.pc not found: installing possibly empty INSTALL.' 2) AC_SUBST([INSTALL_CMD]) ]) # XORG_INSTALL + +# XORG_GIT_MODULE_VERSION() +# - +# Minimum version: 1.8.0 +# +# Generates the git module version according to the git describe HEAD +# If not building under git, #undef XORG_GIT_VERSION is written +# +AC_DEFUN([XORG_GIT_MODULE_VERSION], [ +GIT_MODULE_VERSION_CMD=VER=\`GIT_DIR=\$(top_srcdir)/.git git describe HEAD 2/dev/null\`; \ +OUTSTR=\\#undef XORG_GIT_VERSION\ ; \ +test -n \\$\$VER\ OUTSTR=\\#define XORG_GIT_VERSION \$\$VER\; \ +test -e \\$(@)\ || echo \\$\$OUTSTR\ \\$(@)\; \ +CONTENT=\`cat \$(@)\` test \\$\$CONTENT\ = \\$\$OUTSTR\ || echo \$\$OUTSTR \$(@); +AC_SUBST([GIT_MODULE_VERSION_CMD]) +]) # XORG_GIT_MODULE_VERSION -- 1.6.0.4 Third edition. Removed dirty repo marking replace OUTFILE with $@ ___ 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] xfree86: fix not reached code in tty code
On Wed, Apr 21, 2010 at 06:43:34PM +0300, Vignatti Tiago (Nokia-D/Helsinki) wrote: On Wed, Apr 21, 2010 at 12:19:07AM +0200, ext Peter Hutterer wrote: stack the reviewed patches into a for-keith branch, push them to your $HOME on people.freedesktop.org and then send Keith a pull request. what I usually do is 'git request-pull master git://people.freedesktop.org/~whot/xserver.git for-keith pull-req' and then 'mutt -H pull-req' note that this requires master to be something close-ish to upstream, if you've diverged in other ways just insert the last sha1 instead of master. in my case, master is usually origin/master from where I started development, so origin/master may have moved on since. given that you already have the patches in a tree, it's faster to do this than to wait for Keith to apply and push them, he has pull requests on precedence AFAICT. very useful information, Peter. Thanks. I was thinking maybe to drop this in our wiki. Do you think there's a better place than here: http://wiki.x.org/wiki/Development/Documentation/SubmittingPatches http://www.x.org/wiki/XServer describes the server process and the above is already on there. Though it would be good to have a link from the SubmittingPatches website, feel free to add it there in the appropriate place. (and add what is missing to the XServer page) 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] Revert mi: don't thrash resources when displaying the software cursor across screens
Since the revert is already pushed, here's a new version of the change as Peter
pushed it including the teardown crash fix.
Thanks,
- Pierre-Loup
On 04/20/2010 06:47 PM, Peter Hutterer wrote:
This commit leads to a segfault on the very first XTS test case.
Backtrace:
0: /opt/xorg/bin/Xorg (xorg_backtrace+0x3b) [0x80a33db]
1: /opt/xorg/bin/Xorg (0x8048000+0x62a75) [0x80aaa75]
2: (vdso) (__kernel_rt_sigreturn+0x0) [0x5d140c]
3: /lib/libc.so.6 (0x9bb000+0x73579) [0xa2e579]
4: /lib/libc.so.6 (realloc+0xe0) [0xa2e830]
5: /opt/xorg/bin/Xorg (Xrealloc+0x33) [0x80a3f33]
6: /opt/xorg/bin/Xorg (0x8048000+0x1ab79) [0x8062b79]
7: /opt/xorg/bin/Xorg (0x8048000+0x1ac4e) [0x8062c4e]
8: /opt/xorg/bin/Xorg (RegisterExtensionNames+0x2ce) [0x8062fbe]
9: /opt/xorg/bin/Xorg (AddExtension+0x19a) [0x807bd7a]
10: /opt/xorg//lib/xorg/modules/extensions/libextmod.so (0x728000+0x1169a)
[0x73969a]
11: /opt/xorg/bin/Xorg (InitExtensions+0x85) [0x80c0eb5]
12: /opt/xorg/bin/Xorg (0x8048000+0x1a51d) [0x806251d]
13: /lib/libc.so.6 (__libc_start_main+0xe6) [0x9d1bb6]
14: /opt/xorg/bin/Xorg (0x8048000+0x1a2a1) [0x80622a1]
Segmentation fault at address 0x10b2d5f8
valgrind output:
==5069== Invalid read of size 4
==5069==at 0x80F928D: FreePicture (picture.c:1531)
==5069==by 0x818DDEF: miDCDeviceCleanup (midispcur.c:867)
==5069==by 0x81B97F0: miSpriteDeviceCursorCleanup (misprite.c:968)
==5069==by 0x80995FA: miPointerDeviceCleanup (mipointer.c:292)
==5069==by 0x807973E: CloseDevice (devices.c:840)
==5069==by 0x80799B6: CloseDownDevices (devices.c:933)
==5069==by 0x8062705: main (main.c:309)
==5069== Address 0x4cce844 is 12 bytes inside a block of size 84 free'd
==5069==at 0x40057F6: free (vg_replace_malloc.c:325)
==5069==by 0x80A3DE0: Xfree (utils.c:1154)
==5069==by 0x80F9332: FreePicture (picture.c:1576)
==5069==by 0x80FBB4B: PictureDestroyWindow (picture.c:69)
==5069==by 0x810B1A3: damageDestroyWindow (damage.c:1840)
==5069==by 0x80864F1: FreeWindowResources (window.c:846)
==5069==by 0x8086812: DeleteWindow (window.c:925)
==5069==by 0x806B53E: FreeClientResources (resource.c:806)
==5069==by 0x806B60F: FreeAllResources (resource.c:823)
==5069==by 0x80626E4: main (main.c:299)
==5069==
==5069== Invalid write of size 4
==5069==at 0x80F9295: FreePicture (picture.c:1531)
==5069==by 0x818DDEF: miDCDeviceCleanup (midispcur.c:867)
==5069==by 0x81B97F0: miSpriteDeviceCursorCleanup (misprite.c:968)
==5069==by 0x80995FA: miPointerDeviceCleanup (mipointer.c:292)
==5069==by 0x807973E: CloseDevice (devices.c:840)
==5069==by 0x80799B6: CloseDownDevices (devices.c:933)
==5069==by 0x8062705: main (main.c:309)
==5069== Address 0x4cce844 is 12 bytes inside a block of size 84 free'd
==5069==at 0x40057F6: free (vg_replace_malloc.c:325)
==5069==by 0x80A3DE0: Xfree (utils.c:1154)
==5069==by 0x80F9332: FreePicture (picture.c:1576)
==5069==by 0x80FBB4B: PictureDestroyWindow (picture.c:69)
==5069==by 0x810B1A3: damageDestroyWindow (damage.c:1840)
==5069==by 0x80864F1: FreeWindowResources (window.c:846)
==5069==by 0x8086812: DeleteWindow (window.c:925)
==5069==by 0x806B53E: FreeClientResources (resource.c:806)
==5069==by 0x806B60F: FreeAllResources (resource.c:823)
==5069==by 0x80626E4: main (main.c:299)
XTS test case: Xproto pAllocColor
This reverts commit 00b8b7ad61b6f818271fb4d1e383113170309d72.
Signed-off-by: Peter Huttererpeter.hutte...@who-t.net
---
mi/midispcur.c | 270 +---
1 files changed, 159 insertions(+), 111 deletions(-)
diff --git a/mi/midispcur.c b/mi/midispcur.c
index 3a31b74..3fb7e02 100644
--- a/mi/midispcur.c
+++ b/mi/midispcur.c
@@ -59,9 +59,9 @@ static DevPrivateKey miDCScreenKey =miDCScreenKeyIndex;
static BoolmiDCCloseScreen(int index, ScreenPtr pScreen);
-/* per device per-screen private data */
-static int miDCSpriteKeyIndex[MAXSCREENS];
-static DevPrivateKey miDCSpriteKey = miDCSpriteKeyIndex;
+/* per device private data */
+static int miDCSpriteKeyIndex;
+static DevPrivateKey miDCSpriteKey =miDCSpriteKeyIndex;
typedef struct {
GCPtr pSourceGC, pMaskGC;
@@ -75,10 +75,10 @@ typedef struct {
#endif
} miDCBufferRec, *miDCBufferPtr;
-#define MIDCBUFFER(dev, screen) \
+#define MIDCBUFFER(dev) \
((DevHasCursor(dev)) ? \
- (miDCBufferPtr)dixLookupPrivate(dev-devPrivates, miDCSpriteKey +
(screen)-myNum) : \
- (miDCBufferPtr)dixLookupPrivate(dev-u.master-devPrivates, miDCSpriteKey +
(screen)-myNum))
+ (miDCBufferPtr)dixLookupPrivate(dev-devPrivates, miDCSpriteKey) : \
+ (miDCBufferPtr)dixLookupPrivate(dev-u.master-devPrivates, miDCSpriteKey))
/*
* The core pointer buffer will point to the index of the virtual core pointer
@@ -158,6 +158,10 @@ miDCInitialize (ScreenPtr pScreen, miPointerScreenFuncPtr
screenFuncs)
return TRUE;
}
+#define tossGC(gc) (gc ? FreeGC (gc, (GContext) 0) : 0)
+#define
[PATCH] xf86: Don't crash when switching modes through RandR without owning the VT.
While VT-switched, FB access is disabled and should remain so. Trying to switch
modes in that state would re-enable it, potentially causing crashes if trying
to access it before the driver has recovered from the mode switch.
Thanks,
- Pierre-Loup
From 4d526e470faeaa658cd2cb362d3132384baa478b Mon Sep 17 00:00:00 2001
From: Pierre-Loup A. Griffais pgriff...@nvidia.com
Date: Wed, 21 Apr 2010 17:59:42 -0700
Subject: [PATCH] xf86: Don't crash when switching modes through RandR without owning the VT.
While VT-switched, FB access is disabled and should remain so. Trying to switch
modes in that state would re-enable it, potentially causing crashes if trying
to access it before the driver has recovered from the mode switch.
Signed-off-by: Pierre-Loup A. Griffais pgriff...@nvidia.com
---
hw/xfree86/common/xf86RandR.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/xfree86/common/xf86RandR.c b/hw/xfree86/common/xf86RandR.c
index 02dcc34..d4beb2c 100644
--- a/hw/xfree86/common/xf86RandR.c
+++ b/hw/xfree86/common/xf86RandR.c
@@ -163,7 +163,7 @@ xf86RandRSetMode (ScreenPtr pScreen,
WindowPtr pRoot = WindowTable[pScreen-myNum];
Bool ret = TRUE;
-if (pRoot)
+if (pRoot scrp-vtSema)
(*scrp-EnableDisableFBAccess) (pScreen-myNum, FALSE);
if (useVirtual)
{
@@ -229,7 +229,7 @@ xf86RandRSetMode (ScreenPtr pScreen,
*/
xf86SetViewport (pScreen, pScreen-width, pScreen-height);
xf86SetViewport (pScreen, 0, 0);
-if (pRoot)
+if (pRoot scrp-vtSema)
(*scrp-EnableDisableFBAccess) (pScreen-myNum, TRUE);
return ret;
}
--
1.7.0.4
___
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] Revert mi: don't thrash resources when displaying the software cursor across screens
On Wed, 21 Apr 2010 16:51:17 -0700, Pierre-Loup A. Griffais pgriff...@nvidia.com wrote: Since the revert is already pushed, here's a new version of the change as Peter pushed it including the teardown crash fix. Is this fixing some known issue? Or is it just that it seems sub-optimal to change things around when drawing the sprite to a different screen? -- keith.pack...@intel.com pgptcIYId5yVI.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
[PATCH 1/2] Use screen privates for Xv offscreen images.
The DIX-layer Xv implementation provides a DDX-private hook for
per-screen data, which the xfree86 DDX was already using.
This extends that use to include offscreen surface records, replacing a
globally-allocated array that depended on MAXSCREENS.
Signed-off-by: Jamey Sharp ja...@minilop.net
---
hw/xfree86/common/xf86xv.c | 20 ++--
hw/xfree86/common/xf86xvpriv.h |2 ++
2 files changed, 8 insertions(+), 14 deletions(-)
diff --git a/hw/xfree86/common/xf86xv.c b/hw/xfree86/common/xf86xv.c
index bdcc4fc..8367f44 100644
--- a/hw/xfree86/common/xf86xv.c
+++ b/hw/xfree86/common/xf86xv.c
@@ -181,21 +181,15 @@ xf86XVListGenericAdaptors(
/ Offscreen surface stuff ***/
-typedef struct {
- XF86OffscreenImagePtr images;
- int num;
-} OffscreenImageRec;
-
-static OffscreenImageRec OffscreenImages[MAXSCREENS];
-
Bool
xf86XVRegisterOffscreenImages(
ScreenPtr pScreen,
XF86OffscreenImagePtr images,
int num
){
-OffscreenImages[pScreen-myNum].num = num;
-OffscreenImages[pScreen-myNum].images = images;
+XF86XVScreenPtr ScreenPriv = GET_XF86XV_SCREEN(pScreen);
+ScreenPriv-num = num;
+ScreenPriv-images = images;
return TRUE;
}
@@ -205,8 +199,9 @@ xf86XVQueryOffscreenImages(
ScreenPtr pScreen,
int *num
){
- *num = OffscreenImages[pScreen-myNum].num;
- return OffscreenImages[pScreen-myNum].images;
+ XF86XVScreenPtr ScreenPriv = GET_XF86XV_SCREEN(pScreen);
+ *num = ScreenPriv-num;
+ return ScreenPriv-images;
}
@@ -1177,9 +1172,6 @@ xf86XVCloseScreen(int i, ScreenPtr pScreen)
XvAdaptorPtr pa;
int c;
- /* Clear offscreen images */
- memset(OffscreenImages[pScreen-myNum], 0, sizeof(OffscreenImages[0]));
-
if(!ScreenPriv) return TRUE;
if(ScreenPriv-videoGC) {
diff --git a/hw/xfree86/common/xf86xvpriv.h b/hw/xfree86/common/xf86xvpriv.h
index 7623d29..7ac38a3 100644
--- a/hw/xfree86/common/xf86xvpriv.h
+++ b/hw/xfree86/common/xf86xvpriv.h
@@ -44,6 +44,8 @@ typedef struct {
Bool (*EnterVT)(int, int);
void (*LeaveVT)(int, int);
GCPtr videoGC;
+ XF86OffscreenImagePtrimages;
+ int num;
} XF86XVScreenRec, *XF86XVScreenPtr;
typedef struct {
--
1.7.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
[PATCH 2/2] Use screen privates for exclusive DGA clients.
Most DGA requests allow at most one client to be using DGA on each
screen. Instead of keeping track of the current client in a
MAXSCREEN-sized array, track it in a per-screen private.
Signed-off-by: Jamey Sharp ja...@minilop.net
---
hw/xfree86/dixmods/extmod/xf86dga2.c | 70 ++
1 files changed, 37 insertions(+), 33 deletions(-)
diff --git a/hw/xfree86/dixmods/extmod/xf86dga2.c
b/hw/xfree86/dixmods/extmod/xf86dga2.c
index 5367bcc..0385514 100644
--- a/hw/xfree86/dixmods/extmod/xf86dga2.c
+++ b/hw/xfree86/dixmods/extmod/xf86dga2.c
@@ -57,12 +57,12 @@ static void XDGAResetProc(ExtensionEntry *extEntry);
static void DGAClientStateChange (CallbackListPtr*, pointer, pointer);
-static ClientPtr DGAClients[MAXSCREENS];
-
unsigned char DGAReqCode = 0;
int DGAErrorBase;
int DGAEventBase;
+static int DGAScreenPrivateKeyIndex;
+static DevPrivateKey DGAScreenPrivateKey = DGAScreenPrivateKeyIndex;
static int DGAClientPrivateKeyIndex;
static DevPrivateKey DGAClientPrivateKey = DGAClientPrivateKeyIndex;
static int DGACallbackRefCount = 0;
@@ -73,6 +73,11 @@ typedef struct {
intminor;
} DGAPrivRec, *DGAPrivPtr;
+#define DGA_GETCLIENT(idx) ((ClientPtr) \
+dixLookupPrivate(screenInfo.screens[idx]-devPrivates,
DGAScreenPrivateKey))
+#define DGA_SETCLIENT(idx,p) \
+dixSetPrivate(screenInfo.screens[idx]-devPrivates, DGAScreenPrivateKey,
p)
+
#define DGA_GETPRIV(c) ((DGAPrivPtr) \
dixLookupPrivate((c)-devPrivates, DGAClientPrivateKey))
#define DGA_SETPRIV(c,p) \
@@ -93,9 +98,6 @@ XFree86DGAExtensionInit(INITARGS)
StandardMinorOpcode))) {
int i;
- for(i = 0; i MAXSCREENS; i++)
-DGAClients[i] = NULL;
-
DGAReqCode = (unsigned char)extEntry-base;
DGAErrorBase = extEntry-errorBase;
DGAEventBase = extEntry-eventBase;
@@ -282,7 +284,7 @@ DGAClientStateChange (
int i;
for(i = 0; i screenInfo.numScreens; i++) {
- if(DGAClients[i] == pci-client) {
+ if(DGA_GETCLIENT(i) == pci-client) {
client = pci-client;
break;
}
@@ -294,7 +296,7 @@ DGAClientStateChange (
XDGAModeRec mode;
PixmapPtr pPix;
- DGAClients[i] = NULL;
+ DGA_SETCLIENT(i, NULL);
DGASelectInput(i, NULL, 0);
DGASetMode(i, 0, mode, pPix);
@@ -311,10 +313,12 @@ ProcXDGASetMode(ClientPtr client)
XDGAModeRec mode;
xXDGAModeInfo info;
PixmapPtr pPix;
+ClientPtr owner;
int size;
if (stuff-screen screenInfo.numScreens)
return BadValue;
+owner = DGA_GETCLIENT(stuff-screen);
REQUEST_SIZE_MATCH(xXDGASetModeReq);
rep.type = X_Reply;
@@ -326,16 +330,15 @@ ProcXDGASetMode(ClientPtr client)
if (!DGAAvailable(stuff-screen))
return DGAErrorBase + XF86DGANoDirectVideoMode;
-if(DGAClients[stuff-screen]
- (DGAClients[stuff-screen] != client))
+if(owner owner != client)
return DGAErrorBase + XF86DGANoDirectVideoMode;
if(!stuff-mode) {
- if(DGAClients[stuff-screen]) {
+ if(owner) {
if(--DGACallbackRefCount == 0)
DeleteCallback(ClientStateCallback, DGAClientStateChange, NULL);
}
- DGAClients[stuff-screen] = NULL;
+ DGA_SETCLIENT(stuff-screen, NULL);
DGASelectInput(stuff-screen, NULL, 0);
DGASetMode(stuff-screen, 0, mode, pPix);
WriteToClient(client, sz_xXDGASetModeReply, (char*)rep);
@@ -345,12 +348,12 @@ ProcXDGASetMode(ClientPtr client)
if(Success != DGASetMode(stuff-screen, stuff-mode, mode, pPix))
return BadValue;
-if(!DGAClients[stuff-screen]) {
+if(!owner) {
if(DGACallbackRefCount++ == 0)
AddCallback (ClientStateCallback, DGAClientStateChange, NULL);
}
-DGAClients[stuff-screen] = client;
+DGA_SETCLIENT(stuff-screen, client);
if(pPix) {
if(AddResource(stuff-pid, RT_PIXMAP, (pointer)(pPix))) {
@@ -405,7 +408,7 @@ ProcXDGASetViewport(ClientPtr client)
if (stuff-screen screenInfo.numScreens)
return BadValue;
-if(DGAClients[stuff-screen] != client)
+if(DGA_GETCLIENT(stuff-screen) != client)
return DGAErrorBase + XF86DGADirectNotActivated;
REQUEST_SIZE_MATCH(xXDGASetViewportReq);
@@ -425,7 +428,7 @@ ProcXDGAInstallColormap(ClientPtr client)
if (stuff-screen screenInfo.numScreens)
return BadValue;
-if(DGAClients[stuff-screen] != client)
+if(DGA_GETCLIENT(stuff-screen) != client)
return DGAErrorBase + XF86DGADirectNotActivated;
REQUEST_SIZE_MATCH(xXDGAInstallColormapReq);
@@ -451,12 +454,12 @@ ProcXDGASelectInput(ClientPtr client)
if (stuff-screen screenInfo.numScreens)
return BadValue;
-if(DGAClients[stuff-screen] != client)
+if(DGA_GETCLIENT(stuff-screen) != client)
return DGAErrorBase + XF86DGADirectNotActivated;
REQUEST_SIZE_MATCH(xXDGASelectInputReq);
Re: [PATCH] Revert mi: don't thrash resources when displaying the software cursor across screens
On Wed, Apr 21, 2010 at 08:36:59PM -0700, Keith Packard wrote: On Wed, 21 Apr 2010 16:51:17 -0700, Pierre-Loup A. Griffais pgriff...@nvidia.com wrote: Since the revert is already pushed, here's a new version of the change as Peter pushed it including the teardown crash fix. Is this fixing some known issue? Or is it just that it seems sub-optimal to change things around when drawing the sprite to a different screen? Yes, it fixes a server crash when the cursor crosses screens. From the original thread [1]: On Mon, 2010-04-05 at 18:52 -0700, Pierre-Loup A. Griffais wrote: The DC code is broken for setups with several screens. Devs only have one pSave pixmap and there's no code to thrash them like p[Save|Restore]GC. That means if you have two X screens and force SW cursor on both, the server will end up passing a bunch of CopyAreas with mismatching screens to the driver, which can fail horribly if the driver does a good job abstracting screens away from each other. It's the CopyAreas with mismatching screens that are the problem. [1]: http://lists.x.org/archives/xorg-devel/2010-April/006874.html ___ 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 3/2] Track screens' installed colormaps as screen privates.
Several DDXes allow each screen to have at most one (or in some cases,
exactly one) installed colormap. These all use the same pattern: Declare
a global-lifetime array of MAXSCREENS ColormapPtrs, and index it by
screen number. This patch converts most of those to use screen privates
instead.
mi/micmap.c has the same pattern, except there the miInstalledMaps array
is used in the xfree86 DDX, and I guess it's part of the server's ABI.
So for now I left it alone.
Signed-off-by: Jamey Sharp ja...@minilop.net
---
fb/fbcmap.c | 16
hw/vfb/InitOutput.c | 23 +--
hw/xnest/Color.c| 22 +-
3 files changed, 30 insertions(+), 31 deletions(-)
diff --git a/fb/fbcmap.c b/fb/fbcmap.c
index 2ff3234..b775bc3 100644
--- a/fb/fbcmap.c
+++ b/fb/fbcmap.c
@@ -36,16 +36,18 @@
#error You should be compiling fbcmap_mi.c instead of fbcmap.c!
#endif
+static int cmapScrPrivateKeyIndex;
+static DevPrivateKey cmapScrPrivateKey = cmapScrPrivateKeyIndex;
-
-ColormapPtr FbInstalledMaps[MAXSCREENS];
+#define GetInstalledColormap(s) ((ColormapPtr)
dixLookupPrivate((s)-devPrivates, cmapScrPrivateKey))
+#define SetInstalledColormap(s,c) (dixSetPrivate((s)-devPrivates,
cmapScrPrivateKey, c))
int
fbListInstalledColormaps(ScreenPtr pScreen, Colormap *pmaps)
{
/* By the time we are processing requests, we can guarantee that there
* is always a colormap installed */
-*pmaps = FbInstalledMaps[pScreen-myNum]-mid;
+*pmaps = GetInstalledColormap(pScreen)-mid;
return (1);
}
@@ -53,8 +55,7 @@ fbListInstalledColormaps(ScreenPtr pScreen, Colormap *pmaps)
void
fbInstallColormap(ColormapPtr pmap)
{
-int index = pmap-pScreen-myNum;
-ColormapPtr oldpmap = FbInstalledMaps[index];
+ColormapPtr oldpmap = GetInstalledColormap(pmap-pScreen);
if(pmap != oldpmap)
{
@@ -63,7 +64,7 @@ fbInstallColormap(ColormapPtr pmap)
if(oldpmap != (ColormapPtr)None)
WalkTree(pmap-pScreen, TellLostMap, (char *)oldpmap-mid);
/* Install pmap */
- FbInstalledMaps[index] = pmap;
+ SetInstalledColormap(pmap-pScreen, pmap);
WalkTree(pmap-pScreen, TellGainedMap, (char *)pmap-mid);
}
}
@@ -71,8 +72,7 @@ fbInstallColormap(ColormapPtr pmap)
void
fbUninstallColormap(ColormapPtr pmap)
{
-int index = pmap-pScreen-myNum;
-ColormapPtr curpmap = FbInstalledMaps[index];
+ColormapPtr curpmap = GetInstalledColormap(pmap-pScreen);
if(pmap == curpmap)
{
diff --git a/hw/vfb/InitOutput.c b/hw/vfb/InitOutput.c
index e7dd1d9..bcf17ef 100644
--- a/hw/vfb/InitOutput.c
+++ b/hw/vfb/InitOutput.c
@@ -429,14 +429,18 @@ ddxProcessArgument(int argc, char *argv[], int i)
return 0;
}
-static ColormapPtr InstalledMaps[MAXSCREENS];
+static int cmapScrPrivateKeyIndex;
+static DevPrivateKey cmapScrPrivateKey = cmapScrPrivateKeyIndex;
+
+#define GetInstalledColormap(s) ((ColormapPtr)
dixLookupPrivate((s)-devPrivates, cmapScrPrivateKey))
+#define SetInstalledColormap(s,c) (dixSetPrivate((s)-devPrivates,
cmapScrPrivateKey, c))
static int
vfbListInstalledColormaps(ScreenPtr pScreen, Colormap *pmaps)
{
/* By the time we are processing requests, we can guarantee that there
* is always a colormap installed */
-*pmaps = InstalledMaps[pScreen-myNum]-mid;
+*pmaps = GetInstalledColormap(pScreen)-mid;
return (1);
}
@@ -444,8 +448,7 @@ vfbListInstalledColormaps(ScreenPtr pScreen, Colormap
*pmaps)
static void
vfbInstallColormap(ColormapPtr pmap)
{
-int index = pmap-pScreen-myNum;
-ColormapPtr oldpmap = InstalledMaps[index];
+ColormapPtr oldpmap = GetInstalledColormap(pmap-pScreen);
if (pmap != oldpmap)
{
@@ -461,7 +464,7 @@ vfbInstallColormap(ColormapPtr pmap)
if(oldpmap != (ColormapPtr)None)
WalkTree(pmap-pScreen, TellLostMap, (char *)oldpmap-mid);
/* Install pmap */
- InstalledMaps[index] = pmap;
+ SetInstalledColormap(pmap-pScreen, pmap);
WalkTree(pmap-pScreen, TellGainedMap, (char *)pmap-mid);
entries = pmap-pVisual-ColormapEntries;
@@ -502,7 +505,7 @@ vfbInstallColormap(ColormapPtr pmap)
static void
vfbUninstallColormap(ColormapPtr pmap)
{
-ColormapPtr curpmap = InstalledMaps[pmap-pScreen-myNum];
+ColormapPtr curpmap = GetInstalledColormap(pmap-pScreen);
if(pmap == curpmap)
{
@@ -523,7 +526,7 @@ vfbStoreColors(ColormapPtr pmap, int ndef, xColorItem
*pdefs)
XWDColor *pXWDCmap;
int i;
-if (pmap != InstalledMaps[pmap-pScreen-myNum])
+if (pmap != GetInstalledColormap(pmap-pScreen))
{
return;
}
@@ -832,10 +835,10 @@ vfbCloseScreen(int index, ScreenPtr pScreen)
/*
* XXX probably lots of stuff to clean. For now,
- * clear InstalledMaps[] so that server reset works correctly.
+ * clear installed colormaps so that server reset works correctly.
*/
-for (i = 0; i MAXSCREENS; i++)
-
