Re: intended direct touch behavior
On 10/09/2012 11:49 PM, Peter Hutterer wrote:
> On Tue, Oct 09, 2012 at 08:31:12AM -0700, Chase Douglas wrote:
>> I think you're hitting an issue that just never was resolved properly.
>> I think that, ideally, when you perform a direct touch interaction and
>> the cursor moves to that location, any future relative device motion
>> should be performed relative to the new location.
>
> correct, that's the expected behaviour (and the behaviour for non-touch
> devices). Fix would be in dix/getevents.c:updateSlaveDeviceCoords() and
> figuring out why master->last.valuators apparently have the wrong values.
Thanks, that's good to know. The issue is the storeLastValuators call
in GetTouchEvents. The code was adapted from fill_pointer_events, which
it looks like was written under the assumption that the device is not a
master device. Now GetTouchEvents does get called with dev being a
master device, which causes the problem. I don't know if this should
happen or not, but in any case, skipping the storeLastValuators call for
master devices fixes this issue (see attached patch).
The next issue I encountered is harder to debug: I somehow always end
up with a permanently active touch point that keeps getting replayed in
TouchEventHistoryReplay. I'll look into that when I get a chance.
Tom
diff --git a/dix/getevents.c b/dix/getevents.c
index 71d83c4..03efd43 100644
--- a/dix/getevents.c
+++ b/dix/getevents.c
@@ -2001,9 +2001,17 @@ GetTouchEvents(InternalEvent *events, DeviceIntPtr dev, uint32_t ddx_touchid,
clipValuators(dev, &mask);
-if (emulate_pointer)
+if (emulate_pointer && !IsMaster(dev))
storeLastValuators(dev, &mask, 0, 1, devx, devy);
+/* Update the MD's co-ordinates, which are always in desktop space. */
+if (emulate_pointer && !IsMaster(dev) && !IsFloating(dev)) {
+ DeviceIntPtr master = GetMaster(dev, MASTER_POINTER);
+
+ master->last.valuators[0] = screenx;
+ master->last.valuators[1] = screeny;
+}
+
event->root = scr->root->drawable.id;
event_set_root_coordinates(event, screenx, screeny);
___
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 xorg-gtest 8/8] xserver: use a default timout for 1s for Terminate() and Kill()
Terminate/Kill will only wait for the server to exit for a nonzero timeout,
requiring callers to always specify a timeout for it to be useful.
The use-case of sending a SIGTERM to the server but not waiting for
it to shut down is narrow, so require those to actually specify a zero
timeout instead of everyone else.
Signed-off-by: Peter Hutterer
---
include/xorg/gtest/xorg-gtest-xserver.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/xorg/gtest/xorg-gtest-xserver.h
b/include/xorg/gtest/xorg-gtest-xserver.h
index 90784e7..8721b94 100644
--- a/include/xorg/gtest/xorg-gtest-xserver.h
+++ b/include/xorg/gtest/xorg-gtest-xserver.h
@@ -80,7 +80,7 @@ class XServer : public xorg::testing::Process {
* @returns true if termination succeeded and, if a timout is given, the
* process shut down within that timeout. false otherwise.
*/
-virtual bool Terminate(unsigned int timeout = 0);
+virtual bool Terminate(unsigned int timeout = 1000);
/**
* Kills the server. With a vengeance.
@@ -92,7 +92,7 @@ class XServer : public xorg::testing::Process {
* @returns true if kill succeeded and, if a timout is given, the
* process shut down within that timeout. false otherwise.
*/
-virtual bool Kill(unsigned int timeout = 0);
+virtual bool Kill(unsigned int timeout = 1000);
/**
* Remove the log file used by this server. By default, this function
--
1.7.11.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
[PATCH xorg-gtest 7/8] test/process-test: prefix TESTCASE with newline in debugging output
Signed-off-by: Peter Hutterer
---
test/process-test.cpp | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/test/process-test.cpp b/test/process-test.cpp
index 935035f..392aff1 100644
--- a/test/process-test.cpp
+++ b/test/process-test.cpp
@@ -12,7 +12,7 @@ using namespace xorg::testing;
TEST(Process, StartWithNULLArg)
{
- SCOPED_TRACE("TESTCASE: invocation of 'ls' with no arguments");
+ SCOPED_TRACE("\nTESTCASE: invocation of 'ls' with no arguments");
Process p;
p.Start("ls", NULL);
ASSERT_GT(p.Pid(), 0);
@@ -20,7 +20,7 @@ TEST(Process, StartWithNULLArg)
TEST(Process, StartWithNULLTerminatedArg)
{
- SCOPED_TRACE("TESTCASE: invocation of 'ls' with NULL-terminated argument
list");
+ SCOPED_TRACE("\nTESTCASE: invocation of 'ls' with NULL-terminated argument
list");
Process p;
p.Start("ls", "-l", NULL);
@@ -29,7 +29,7 @@ TEST(Process, StartWithNULLTerminatedArg)
TEST(Process, ExitCodeSuccess)
{
- SCOPED_TRACE("TESTCASE: invocation of 'echo -n', check for success exit
status");
+ SCOPED_TRACE("\nTESTCASE: invocation of 'echo -n', check for success exit
status");
Process p;
ASSERT_EQ(p.GetState(), Process::NONE);
@@ -49,7 +49,7 @@ TEST(Process, ExitCodeSuccess)
TEST(Process, ExitCodeFailure)
{
- SCOPED_TRACE("TESTCASE: an invalid invocation of 'ls', check for error exit
status");
+ SCOPED_TRACE("\nTESTCASE: an invalid invocation of 'ls', check for error
exit status");
Process p;
ASSERT_EQ(p.GetState(), Process::NONE);
@@ -69,7 +69,7 @@ TEST(Process, ExitCodeFailure)
TEST(Process, ChildTearDown)
{
- SCOPED_TRACE("TESTCASE: ensure child process dies when parent does");
+ SCOPED_TRACE("\nTESTCASE: ensure child process dies when parent does");
int pipefd[2];
ASSERT_NE(pipe(pipefd), -1);
@@ -112,7 +112,7 @@ TEST(Process, ChildTearDown)
TEST(Process, TerminationFailure)
{
- SCOPED_TRACE("TESTCASE: if Process::Terminate() fails to terminate the \n"
+ SCOPED_TRACE("\nTESTCASE: if Process::Terminate() fails to terminate the \n"
"child process, kill must terminate it it instead");
sigset_t sig_mask;
@@ -137,7 +137,7 @@ TEST(Process, TerminationFailure)
TEST(Process, KillExitStatus)
{
- SCOPED_TRACE("TESTCASE: a child process killed must have a state of\n"
+ SCOPED_TRACE("\nTESTCASE: a child process killed must have a state of\n"
"FINISHED_FAILURE");
Process p;
p.Start(TEST_ROOT_DIR "process-test-helper", NULL);
@@ -149,7 +149,7 @@ TEST(Process, DoubleStart)
{
struct timespec sig_timeout = {0, 500L};
- SCOPED_TRACE("TESTCASE: starting a process after it has been started\n"
+ SCOPED_TRACE("\nTESTCASE: starting a process after it has been started\n"
"fails. Re-starting a process succeeds\n");
/* Process double-started must fail */
--
1.7.11.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
[PATCH xorg-gtest 6/8] test/process-test: add test for starting the same process object multiple times
Signed-off-by: Peter Hutterer
---
test/process-test.cpp | 86 +++
1 file changed, 86 insertions(+)
diff --git a/test/process-test.cpp b/test/process-test.cpp
index 0ed47aa..935035f 100644
--- a/test/process-test.cpp
+++ b/test/process-test.cpp
@@ -6,6 +6,8 @@
#include
#include
+#include
+
using namespace xorg::testing;
TEST(Process, StartWithNULLArg)
@@ -143,6 +145,90 @@ TEST(Process, KillExitStatus)
ASSERT_EQ(p.GetState(), Process::FINISHED_FAILURE);
}
+TEST(Process, DoubleStart)
+{
+ struct timespec sig_timeout = {0, 500L};
+
+ SCOPED_TRACE("TESTCASE: starting a process after it has been started\n"
+ "fails. Re-starting a process succeeds\n");
+
+ /* Process double-started must fail */
+ Process p;
+ p.Start("echo", "-n", NULL);
+ try {
+p.Start("echo", "-n", NULL);;
+FAIL() << "Expected exception";
+ } catch (std::runtime_error &e) {
+ }
+ p.Terminate(1000);
+ /* we sent it a SIGTERM, counts as failure */
+ ASSERT_EQ(p.GetState(), Process::FINISHED_FAILURE);
+
+ sigset_t sig_mask;
+ sigemptyset(&sig_mask);
+ sigaddset(&sig_mask, SIGCHLD);
+ sigaddset(&sig_mask, SIGUSR1);
+ sigprocmask(SIG_BLOCK, &sig_mask, 0);
+
+ /* restart job after a failed one, must succeed */
+ try {
+p.Start("echo", "-n", NULL);
+ } catch (std::runtime_error &e) {
+FAIL();
+ }
+
+ ASSERT_EQ(sigtimedwait(&sig_mask, NULL, &sig_timeout), SIGCHLD);
+ ASSERT_EQ(p.GetState(), Process::FINISHED_SUCCESS);
+
+ /* restart job after successful one, must succeed */
+ try {
+p.Start("echo", "-n", NULL);
+ } catch (std::runtime_error &e) {
+FAIL();
+ }
+ ASSERT_EQ(sigtimedwait(&sig_mask, NULL, &sig_timeout), SIGCHLD);
+ ASSERT_EQ(p.GetState(), Process::FINISHED_SUCCESS);
+
+ /* job that must be killed, followed by job */
+ sigemptyset(&sig_mask);
+ sigaddset(&sig_mask, SIGUSR1);
+ p.Start(TEST_ROOT_DIR "process-test-helper", NULL);
+ sigtimedwait(&sig_mask, NULL, &sig_timeout);
+ ASSERT_EQ(p.GetState(), Process::RUNNING);
+ p.Kill(100);
+ ASSERT_EQ(p.GetState(), Process::FINISHED_FAILURE);
+
+ /* restart job after successful one, must succeed */
+ try {
+p.Start("echo", "-n", NULL);
+ } catch (std::runtime_error &e) {
+FAIL();
+ }
+ sigemptyset(&sig_mask);
+ sigaddset(&sig_mask, SIGCHLD);
+ ASSERT_EQ(sigtimedwait(&sig_mask, NULL, &sig_timeout), SIGCHLD);
+ ASSERT_EQ(p.GetState(), Process::FINISHED_SUCCESS);
+
+ /* job that fails to terminate, starting another one must fail */
+ sigemptyset(&sig_mask);
+ sigaddset(&sig_mask, SIGUSR1);
+ p.Start(TEST_ROOT_DIR "process-test-helper", NULL);
+ sigtimedwait(&sig_mask, NULL, &sig_timeout);
+ ASSERT_EQ(p.GetState(), Process::RUNNING);
+ ASSERT_FALSE(p.Terminate(100));
+
+ try {
+p.Start("echo", "-n", NULL);
+FAIL() << "exception expected";
+ } catch (std::runtime_error &e) {
+ }
+
+ sigemptyset(&sig_mask);
+ sigaddset(&sig_mask, SIGCHLD);
+ sigaddset(&sig_mask, SIGUSR1);
+ sigprocmask(SIG_UNBLOCK, &sig_mask, 0);
+}
+
int main(int argc, char *argv[]) {
testing::InitGoogleTest(&argc, argv);
return RUN_ALL_TESTS();
--
1.7.11.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
[PATCH xorg-gtest 5/8] test: add test for termination exit status
Signed-off-by: Peter Hutterer
---
test/process-test.cpp | 10 ++
1 file changed, 10 insertions(+)
diff --git a/test/process-test.cpp b/test/process-test.cpp
index 964a97d..0ed47aa 100644
--- a/test/process-test.cpp
+++ b/test/process-test.cpp
@@ -133,6 +133,16 @@ TEST(Process, TerminationFailure)
ASSERT_TRUE(p.Kill(100));
}
+TEST(Process, KillExitStatus)
+{
+ SCOPED_TRACE("TESTCASE: a child process killed must have a state of\n"
+ "FINISHED_FAILURE");
+ Process p;
+ p.Start(TEST_ROOT_DIR "process-test-helper", NULL);
+ p.Kill(1000);
+ ASSERT_EQ(p.GetState(), Process::FINISHED_FAILURE);
+}
+
int main(int argc, char *argv[]) {
testing::InitGoogleTest(&argc, argv);
return RUN_ALL_TESTS();
--
1.7.11.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
[PATCH xorg-gtest 4/8] process: wait for SIGCHLD instead of busy looping
If for some reason we fail to handle the signals, usleep for the timeout
instead. This will slow down the tests, but still behave properly. And if a
test fails with the usleep, the SCOPED_TRACE will print out the warnings.
Signed-off-by: Peter Hutterer
---
src/process.cpp | 47 +++
1 file changed, 31 insertions(+), 16 deletions(-)
diff --git a/src/process.cpp b/src/process.cpp
index 0b0ddc3..7f7d330 100644
--- a/src/process.cpp
+++ b/src/process.cpp
@@ -127,25 +127,40 @@ void xorg::testing::Process::Start(const std::string&
program, ...) {
}
bool xorg::testing::Process::WaitForExit(unsigned int timeout) {
- for (unsigned int i = 0; i < timeout * 100; i++) {
-int status;
-int pid = waitpid(Pid(), &status, WNOHANG);
-
-if (pid == Pid()) {
- if (WIFEXITED(status)) {
-d_->state = WEXITSTATUS(status) ? FINISHED_FAILURE : FINISHED_SUCCESS;
-return true;
- } else if (WIFSIGNALED(status)) {
-d_->state = FINISHED_FAILURE;
-return true;
- }
-} else if (pid == -1)
- return errno == ECHILD;
+ sigset_t sig_mask, old_mask;
+ sigemptyset(&sig_mask);
+ sigaddset(&sig_mask, SIGCHLD);
+
+ if (sigprocmask(SIG_BLOCK, &sig_mask, &old_mask) == 0) {
+struct timespec sig_timeout = {timeout / 1000,
+ (timeout % 1000) * 100L};
+
+if (sigtimedwait(&sig_mask, NULL, &sig_timeout) != SIGCHLD && errno !=
EAGAIN) {
+ SCOPED_TRACE("INFO: Failure waiting for SIGCHLD: " +
+ std::string(strerror(errno)) + ". I slept instead.");
+ usleep(timeout * 1000);
+}
- usleep(10);
+if (!sigismember(&sig_mask, SIGCHLD)) {
+ if (sigprocmask(SIG_UNBLOCK, &sig_mask, NULL) == -1)
+SCOPED_TRACE("WARNING: Failed to unblock SIGCHLD. Tests may behave
funny.\n");
+}
+ } else { /* oops, can't wait for SIGCHLD, sleep instead */
+SCOPED_TRACE("INFO: Failed to set SIGCHLD mask, sleeping instead.\n");
+usleep(timeout * 1000);
}
- return false;
+ int status;
+ int pid = waitpid(Pid(), &status, WNOHANG);
+ if (pid == Pid()) {
+if (WIFEXITED(status)) {
+ d_->state = WEXITSTATUS(status) ? FINISHED_FAILURE : FINISHED_SUCCESS;
+} else if (WIFSIGNALED(status)) {
+ d_->state = FINISHED_FAILURE;
+}
+return true;
+ } else
+return (pid == -1 && errno == ECHILD);
}
bool xorg::testing::Process::KillSelf(int signal, unsigned int timeout) {
--
1.7.11.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
[PATCH xorg-gtest 3/8] process: on termination, check if the process exited and set the error code
This changes the meaning of Process::TERMINATED to "currently in termination
but we're not sure what happened to it"
Signed-off-by: Peter Hutterer
---
include/xorg/gtest/xorg-gtest-process.h | 5 -
src/process.cpp | 17 -
2 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/include/xorg/gtest/xorg-gtest-process.h
b/include/xorg/gtest/xorg-gtest-process.h
index f1fc0ec..8c581db 100644
--- a/include/xorg/gtest/xorg-gtest-process.h
+++ b/include/xorg/gtest/xorg-gtest-process.h
@@ -70,6 +70,8 @@ class Process {
* * A process in state ERROR or NONE will fail to Kill() or Terminate()
* * A process in state FINISHED_SUCCESS or FINISHED_FAILURE will always
* succeed to Kill() or Terminate()
+* * A process in state TERMINATED may change state to FINISHED_SUCCESS
+* or FINISHED_FAILURE when queried again.
*/
enum State {
ERROR, /**< An error has occured, state is now unknown */
@@ -77,7 +79,8 @@ class Process {
RUNNING, /**< The process has been started */
FINISHED_SUCCESS, /**< The process finished with an exit code of 0 */
FINISHED_FAILURE, /**< The process finished with a non-zero exit code */
- TERMINATED,/**< The process was successfully terminated by this
library */
+ TERMINATED,/**< The process was successfully terminated by this
+ library but it's state is currently unknown */
};
/**
diff --git a/src/process.cpp b/src/process.cpp
index a9c041e..0b0ddc3 100644
--- a/src/process.cpp
+++ b/src/process.cpp
@@ -131,9 +131,15 @@ bool xorg::testing::Process::WaitForExit(unsigned int
timeout) {
int status;
int pid = waitpid(Pid(), &status, WNOHANG);
-if (pid == Pid())
- return true;
-else if (pid == -1)
+if (pid == Pid()) {
+ if (WIFEXITED(status)) {
+d_->state = WEXITSTATUS(status) ? FINISHED_FAILURE : FINISHED_SUCCESS;
+return true;
+ } else if (WIFSIGNALED(status)) {
+d_->state = FINISHED_FAILURE;
+return true;
+ }
+} else if (pid == -1)
return errno == ECHILD;
usleep(10);
@@ -171,8 +177,9 @@ bool xorg::testing::Process::KillSelf(int signal, unsigned
int timeout) {
bool wait_success = true;
wait_success = WaitForExit(timeout);
- if (!wait_success)
-return false;
+ if (wait_success)
+d_->pid = -1;
+ return wait_success;
}
d_->pid = -1;
}
--
1.7.11.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
[PATCH xorg-gtest 2/8] test: add test-case for starting servers
With the SIGUSR1 patch, an XOpenDisplay() after XServer::Start() should
always succeed.
Signed-off-by: Peter Hutterer
---
test/xserver-test.cpp | 16
1 file changed, 16 insertions(+)
diff --git a/test/xserver-test.cpp b/test/xserver-test.cpp
index ba6c462..103657c 100644
--- a/test/xserver-test.cpp
+++ b/test/xserver-test.cpp
@@ -55,6 +55,22 @@ TEST(XServer, LogRemoval)
file.close();
}
+TEST(XServer, WaitForSIGUSR1)
+{
+ SCOPED_TRACE("TESTCASE: XOpenDisplay() following server.Start() must\n"
+ "succeed as we wait for the SIGUSR1 signal\n");
+ for (int i = 0; i < 20; i++) {
+XServer server;
+server.SetOption("-logfile", "/tmp/xorg-testing-xserver-sigusr1.log");
+server.SetOption("-noreset", "");
+server.Start();
+ASSERT_EQ(server.GetState(), Process::RUNNING);
+Display *dpy = XOpenDisplay(server.GetDisplayString().c_str());
+ASSERT_TRUE(dpy != NULL);
+XCloseDisplay(dpy);
+server.Terminate(500);
+ }
+}
int main(int argc, char *argv[]) {
testing::InitGoogleTest(&argc, argv);
--
1.7.11.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
[PATCH xorg-gtest 1/8] xserver: don't throw exceptions on failed startup
Startup failure can be a valid test-case, avoid throwing exceptions around.
Instead, update the process state on SIGCHLD, otherwise continue quietly
after the timeout.
A test that needs the server to be running, will figure out that it isn't
once XOpenDisplay() fails.
If the signal handling fails, still throw an exception, that's an actual
error case.
Signed-off-by: Peter Hutterer
---
src/xserver.cpp | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/src/xserver.cpp b/src/xserver.cpp
index 082818c..1ba4e08 100644
--- a/src/xserver.cpp
+++ b/src/xserver.cpp
@@ -308,6 +308,7 @@ void xorg::testing::XServer::Start(const std::string
&program) {
/* add SIGUSR1 to the signal mask */
sigemptyset(&sig_mask);
sigaddset(&sig_mask, SIGUSR1);
+ sigaddset(&sig_mask, SIGCHLD);
if (sigprocmask(SIG_BLOCK, &sig_mask, NULL)) {
err_msg.append("Failed to set signal mask: ");
err_msg.append(std::strerror(errno));
@@ -337,16 +338,19 @@ void xorg::testing::XServer::Start(const std::string
&program) {
raise(SIGSTOP);
/* wait for SIGUSR1 from XServer */
-if (SIGUSR1 != sigtimedwait(&sig_mask, NULL, &sig_timeout)) {
- if (errno == EAGAIN) {
-err_msg.append("XServer startup timed out: ");
- } else {
+int recv_sig = sigtimedwait(&sig_mask, NULL, &sig_timeout);
+if (recv_sig == SIGCHLD) {
+ GetState();
+} else if (recv_sig != SIGUSR1 && errno != EAGAIN) {
err_msg.append("Error while waiting for XServer startup: ");
- }
- err_msg.append(std::strerror(errno));
- throw std::runtime_error(err_msg);
+err_msg.append(std::strerror(errno));
+throw std::runtime_error(err_msg);
}
}
+
+ sigemptyset(&sig_mask);
+ sigaddset(&sig_mask, SIGCHLD);
+ sigprocmask(SIG_UNBLOCK, &sig_mask, NULL);
}
bool xorg::testing::XServer::Terminate(unsigned int timeout) {
--
1.7.11.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 xorg-gtest 1/4] process: use actual time for timeouts
On Tue, Oct 09, 2012 at 08:25:35AM -0700, Chase Douglas wrote:
> On Sun, Oct 7, 2012 at 6:55 PM, Peter Hutterer
> wrote:
> > loops and usleeps are nice and simple, but have a tendency to overrun the
> > actual timeouts given. Use the actual time instead.
> >
> > Signed-off-by: Peter Hutterer
> > ---
> > src/process.cpp | 19 +--
> > 1 file changed, 17 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/process.cpp b/src/process.cpp
> > index 4deea14..555e56b 100644
> > --- a/src/process.cpp
> > +++ b/src/process.cpp
> > @@ -30,6 +30,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> >
> > #include
> > @@ -127,16 +128,30 @@ void xorg::testing::Process::Start(const std::string&
> > program, ...) {
> > }
> >
> > bool xorg::testing::Process::WaitForExit(unsigned int timeout) {
> > - for (unsigned int i = 0; i < timeout * 100; i++) {
> > + struct timeval current;
> > + struct timeval endtime;
> > + struct timeval add;
> > +
> > + if (gettimeofday(¤t, NULL) != 0)
> > +throw std::runtime_error("Failed to get the time");
> > +
> > + add.tv_sec = timeout / 1000;
> > + add.tv_usec = (timeout % 1000) * 1000;
> > +
> > + timeradd(¤t, &add, &endtime);
> > +
> > + while (timercmp(¤t, &endtime, <)) {
> > int status;
> > int pid = waitpid(Pid(), &status, WNOHANG);
> >
> > +
> > if (pid == Pid())
> >return true;
> > else if (pid == -1)
> >return errno == ECHILD;
> >
> > - usleep(10);
>
> Don't we still want this sleep? Without it, we'll just be busy looping
> the entire time.
oops, right. better to wait for SIGCHILD here anyway, patch for that
forthcoming.
Cheers,
Peter
>
> > +if (gettimeofday(¤t, NULL) != 0)
> > + throw std::runtime_error("Failed to get the time");
> >}
> >
> >return false;
> > --
> > 1.7.11.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: [PULL] fix crash on shutdown
Peter Hutterer writes: > Denys Vlasenko (1): > os: fix typo in OsSigHandler() error message > > Peter Hutterer (1): > dix: fix crash on shutdown if a disabled device is still grabbed (XI1 > grab) Merged. 0a75bd6..4b7f003 master -> master -- keith.pack...@intel.com pgp7hUYMgLhlQ.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
[PULL] fix crash on shutdown
The following changes since commit 0a75bd640b3dc26b89d9e342999a7f4b7e98edbf: xfree86: add xf86UpdateDesktopDimensions() (2012-10-08 12:40:49 +1000) are available in the git repository at: git://people.freedesktop.org/~whot/xserver for-keith for you to fetch changes up to 4b7f00346daed20c96f3e8ea13ae411858a5424b: dix: fix crash on shutdown if a disabled device is still grabbed (XI1 grab) (2012-10-10 14:40:45 +1000) Denys Vlasenko (1): os: fix typo in OsSigHandler() error message Peter Hutterer (1): dix: fix crash on shutdown if a disabled device is still grabbed (XI1 grab) dix/events.c | 16 os/osinit.c | 2 +- 2 files changed, 13 insertions(+), 5 deletions(-) pgpfBfbV12xFb.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: fix crash on shutdown if a disabled device is still grabbed (XI1 grab)
Peter Hutterer writes: > A disabled device doesn't have a sprite (less so a sprite->win) and triggers > a NULL-pointer dereference on shutdown when all active grabs are released as > part of the cleanup. > > Fix this by checking for sprite being non-null and setting the focus window > to the NullWindow if it is. The rest of the patch just attempts to make > things more readable. Yeah, seems kinda like a kludge to me, but doesn't obviously change any relevant semantics. Reviewed-by: Keith Packard -- keith.pack...@intel.com pgpHqSm5mSAAf.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: intended direct touch behavior
On Tue, Oct 09, 2012 at 08:31:12AM -0700, Chase Douglas wrote: > On Mon, Oct 8, 2012 at 4:30 PM, Thomas Jaeger wrote: > > Hi, > > > > I'm trying to debug a few issues that I suspect are ultimately the > > result of wacom touch screens having a resolution different from the > > screen resolution [1], and I realized I don't even know what is supposed > > to happen when a relative device is used after a touch. The two > > possible behaviors are > > (1) jump back to the last (pre-touch) position of the master pointer > > (2) stay at the position of the last TouchEnd > > The server exhibits behaviors approximating both (1) and (2) depending > > on the circumstances: For example, on an empty session (with just the > > root window it will do (1) (but the cursor is shown at the point of the > > touch until the relative device is actually moved). In an xterm or xev > > window, it will do (2) (but jump to the device coordinates of the last > > touch, interpreted as screen coordinates, once the relative device is used). > > > > Which of the two behaviors is intended? I don't personally care either > > way, but it'd be nice to be consistent. > > I think you're hitting an issue that just never was resolved properly. > I think that, ideally, when you perform a direct touch interaction and > the cursor moves to that location, any future relative device motion > should be performed relative to the new location. correct, that's the expected behaviour (and the behaviour for non-touch devices). Fix would be in dix/getevents.c:updateSlaveDeviceCoords() and figuring out why master->last.valuators apparently have the wrong values. Cheers, Peter > What is currently happening is the relative device is remembering its > last location and performing future motion relative to its remembered > location. > > Basically, I think it would just take someone who is annoyed enough to > spend a half hour to fix it :). > > -- Chase > ___ > xorg-devel@lists.x.org: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: http://lists.x.org/mailman/listinfo/xorg-devel > ___ 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] xwininfo: report the Visual class of the selected Window
Looks like fallout from the XCB port? Anyhow, appreciated if someone
could push this on my behalf.
Thanks!
- Pierre-Loup
>From 12df33eb2424533fa0b9ae8b9ceecdd67fa84ffc Mon Sep 17 00:00:00 2001
From: "Pierre-Loup A. Griffais"
Date: Tue, 9 Oct 2012 20:33:10 -0700
Subject: [PATCH] xwininfo: report the Visual class of the selected Window
Not of the root window.
Signed-off-by: Pierre-Loup A. Griffais
---
xwininfo.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xwininfo.c b/xwininfo.c
index 70947ee..bb290b7 100644
--- a/xwininfo.c
+++ b/xwininfo.c
@@ -981,7 +981,7 @@ Display_Stats_Info (struct wininfo *w)
visual_iter = xcb_depth_visuals_iterator (depth_iter.data);
for (; visual_iter.rem; xcb_visualtype_next (&visual_iter)) {
- if (screen->root_visual == visual_iter.data->visual_id) {
+ if (win_attributes->visual == visual_iter.data->visual_id) {
visual_type = visual_iter.data;
break;
}
--
1.7.10.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
[PATCH] dix: fix crash on shutdown if a disabled device is still grabbed (XI1 grab)
A disabled device doesn't have a sprite (less so a sprite->win) and triggers
a NULL-pointer dereference on shutdown when all active grabs are released as
part of the cleanup.
Fix this by checking for sprite being non-null and setting the focus window
to the NullWindow if it is. The rest of the patch just attempts to make
things more readable.
Signed-off-by: Peter Hutterer
---
dix/events.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/dix/events.c b/dix/events.c
index c0e330b..ddb5b34 100644
--- a/dix/events.c
+++ b/dix/events.c
@@ -1593,13 +1593,10 @@ DeactivateKeyboardGrab(DeviceIntPtr keybd)
{
GrabPtr grab = keybd->deviceGrab.grab;
DeviceIntPtr dev;
-WindowPtr focusWin = keybd->focus ? keybd->focus->win
-: keybd->spriteInfo->sprite->win;
+WindowPtr focusWin;
Bool wasImplicit = (keybd->deviceGrab.fromPassiveGrab &&
keybd->deviceGrab.implicitGrab);
-if (focusWin == FollowKeyboardWin)
-focusWin = inputInfo.keyboard->focus->win;
if (keybd->valuator)
keybd->valuator->motionHintWindow = NullWindow;
keybd->deviceGrab.grab = NullGrab;
@@ -1610,6 +1607,17 @@ DeactivateKeyboardGrab(DeviceIntPtr keybd)
if (dev->deviceGrab.sync.other == grab)
dev->deviceGrab.sync.other = NullGrab;
}
+
+if (keybd->focus)
+focusWin = keybd->focus->win;
+else if (keybd->spriteInfo->sprite)
+focusWin = keybd->spriteInfo->sprite->win;
+else
+focusWin = NullWindow;
+
+if (focusWin == FollowKeyboardWin)
+focusWin = inputInfo.keyboard->focus->win;
+
DoFocusEvents(keybd, grab->window, focusWin, NotifyUngrab);
if (!wasImplicit && grab->grabtype == XI2)
--
1.7.11.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 xorg-gtest 0/2] Import googletest into xorg-gtest
On Tue, Oct 09, 2012 at 08:21:19AM -0700, Chase Douglas wrote: > On Wed, Oct 3, 2012 at 11:44 PM, Peter Hutterer > wrote: > > > > We need to build googletest and xorg-gtest from source for users of > > xorg-gtest because we cannot guarantee a stable ABI (see googletest > > documentation/C++ ODR for reasons). > > > > This means we need googletest source, xorg-gtest source and actual test > > sources around to compile. To further entertain developers, one shouldn't > > make install googletest (it will complain about it), one must provide an > > absolute path to googletest's sources for xorg-gtest, one must make install > > xorg-gtest, and then, in the client, provide the path to the googletest > > sources and set PKG_CONFIG_PATH etc. to the xorg-gtest installed bits. > > This is madness, or at least inconvenient, whichever comes first. > > > > Extra fun is to be had if one tries to build against a googletest that's in > > the package repo and may be out of date. > > > > These patches import googletest into xorg-gtest and thus allow xorg-gtest to > > be built with normal dependencies only, and likewise the clients to use > > xorg-gtest without having to worry about googletest. Clients must update to > > the new Makefile-xorg-gtest.am though to avoid build errors once this is > > merged. > > I'm ok with the change. It would be nice if there were a mechanism for > detecting when the gtest sources are out of date, but I don't know of > any prebuilt solution for that off-hand. We could have a script that > checks the latest version available online whenever make distcheck is > run. there's an RSS feed available for googletest releases. http://code.google.com/p/googletest/feeds not as good as an email, but good enough to see when a new version comes out. having said that, unless we find some limitations or issues with v1.6 there's probably not a lot of reason to immediately update to the latest and greatest. These tests don't really need to worry about security fixes, so it's really only the API that would be the limiting factor. > Reviewed-by: Chase Douglas thanks. 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: intended direct touch behavior
On Mon, Oct 8, 2012 at 4:30 PM, Thomas Jaeger wrote: > Hi, > > I'm trying to debug a few issues that I suspect are ultimately the > result of wacom touch screens having a resolution different from the > screen resolution [1], and I realized I don't even know what is supposed > to happen when a relative device is used after a touch. The two > possible behaviors are > (1) jump back to the last (pre-touch) position of the master pointer > (2) stay at the position of the last TouchEnd > The server exhibits behaviors approximating both (1) and (2) depending > on the circumstances: For example, on an empty session (with just the > root window it will do (1) (but the cursor is shown at the point of the > touch until the relative device is actually moved). In an xterm or xev > window, it will do (2) (but jump to the device coordinates of the last > touch, interpreted as screen coordinates, once the relative device is used). > > Which of the two behaviors is intended? I don't personally care either > way, but it'd be nice to be consistent. I think you're hitting an issue that just never was resolved properly. I think that, ideally, when you perform a direct touch interaction and the cursor moves to that location, any future relative device motion should be performed relative to the new location. What is currently happening is the relative device is remembering its last location and performing future motion relative to its remembered location. Basically, I think it would just take someone who is annoyed enough to spend a half hour to fix it :). -- Chase ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xorg-gtest 2/4] process: if termination fails, the state must not be TERMINATED
On Sun, Oct 7, 2012 at 7:46 PM, Peter Hutterer wrote:
> If a process is hung and doesn't respond to termination, a Kill() call must
> still try to actually kill the process. In the current code, unsuccessful
> termination would still set the state, preventing Kill() from actually
> working
>
> Signed-off-by: Peter Hutterer
> ---
> src/process.cpp | 11 +++
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/src/process.cpp b/src/process.cpp
> index 555e56b..abd107d 100644
> --- a/src/process.cpp
> +++ b/src/process.cpp
> @@ -158,8 +158,6 @@ bool xorg::testing::Process::WaitForExit(unsigned int
> timeout) {
> }
>
> bool xorg::testing::Process::KillSelf(int signal, unsigned int timeout) {
> - bool wait_success = true;
> -
>enum State state = GetState();
>switch (state) {
> case FINISHED_SUCCESS:
> @@ -184,12 +182,17 @@ bool xorg::testing::Process::KillSelf(int signal,
> unsigned int timeout) {
>d_->state = ERROR;
>return false;
> }
> -if (timeout > 0)
> +if (timeout > 0) {
> + bool wait_success = true;
> +
>wait_success = WaitForExit(timeout);
> + if (!wait_success)
> +return false;
> +}
> d_->pid = -1;
>}
>d_->state = TERMINATED;
> - return wait_success;
> + return true;
> }
>
> bool xorg::testing::Process::Terminate(unsigned int timeout) {
This fix looks good.
For this and the following two related patches:
Reviewed-by: Chase Douglas
___
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 xorg-gtest 1/4] process: use actual time for timeouts
On Sun, Oct 7, 2012 at 6:55 PM, Peter Hutterer wrote:
> loops and usleeps are nice and simple, but have a tendency to overrun the
> actual timeouts given. Use the actual time instead.
>
> Signed-off-by: Peter Hutterer
> ---
> src/process.cpp | 19 +--
> 1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/src/process.cpp b/src/process.cpp
> index 4deea14..555e56b 100644
> --- a/src/process.cpp
> +++ b/src/process.cpp
> @@ -30,6 +30,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> #include
> @@ -127,16 +128,30 @@ void xorg::testing::Process::Start(const std::string&
> program, ...) {
> }
>
> bool xorg::testing::Process::WaitForExit(unsigned int timeout) {
> - for (unsigned int i = 0; i < timeout * 100; i++) {
> + struct timeval current;
> + struct timeval endtime;
> + struct timeval add;
> +
> + if (gettimeofday(¤t, NULL) != 0)
> +throw std::runtime_error("Failed to get the time");
> +
> + add.tv_sec = timeout / 1000;
> + add.tv_usec = (timeout % 1000) * 1000;
> +
> + timeradd(¤t, &add, &endtime);
> +
> + while (timercmp(¤t, &endtime, <)) {
> int status;
> int pid = waitpid(Pid(), &status, WNOHANG);
>
> +
> if (pid == Pid())
>return true;
> else if (pid == -1)
>return errno == ECHILD;
>
> - usleep(10);
Don't we still want this sleep? Without it, we'll just be busy looping
the entire time.
> +if (gettimeofday(¤t, NULL) != 0)
> + throw std::runtime_error("Failed to get the time");
>}
>
>return false;
> --
> 1.7.11.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 xorg-gtest 0/2] Import googletest into xorg-gtest
On Wed, Oct 3, 2012 at 11:44 PM, Peter Hutterer wrote: > > We need to build googletest and xorg-gtest from source for users of > xorg-gtest because we cannot guarantee a stable ABI (see googletest > documentation/C++ ODR for reasons). > > This means we need googletest source, xorg-gtest source and actual test > sources around to compile. To further entertain developers, one shouldn't > make install googletest (it will complain about it), one must provide an > absolute path to googletest's sources for xorg-gtest, one must make install > xorg-gtest, and then, in the client, provide the path to the googletest > sources and set PKG_CONFIG_PATH etc. to the xorg-gtest installed bits. > This is madness, or at least inconvenient, whichever comes first. > > Extra fun is to be had if one tries to build against a googletest that's in > the package repo and may be out of date. > > These patches import googletest into xorg-gtest and thus allow xorg-gtest to > be built with normal dependencies only, and likewise the clients to use > xorg-gtest without having to worry about googletest. Clients must update to > the new Makefile-xorg-gtest.am though to avoid build errors once this is > merged. I'm ok with the change. It would be nice if there were a mechanism for detecting when the gtest sources are out of date, but I don't know of any prebuilt solution for that off-hand. We could have a script that checks the latest version available online whenever make distcheck is run. Anyway, Reviewed-by: Chase Douglas ___ 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
