[PATCH weston 1/5] tests: always build tests
check_PROGRAMS and friends are only built during make check. Which is a great way of introducing compiler errors in tests. Always build them, TESTS defines what's being run during make check. --- tests/Makefile.am | 12 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index 82bf630..398a275 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -29,18 +29,14 @@ clean-local: export abs_builddir noinst_LTLIBRARIES = \ - $(weston_test) + $(weston_test) \ + $(module_tests) noinst_PROGRAMS = \ $(setbacklight) \ - matrix-test - -check_LTLIBRARIES =\ - $(module_tests) - -check_PROGRAMS = \ $(shared_tests) \ - $(weston_tests) + $(weston_tests) \ + matrix-test AM_CFLAGS = $(GCC_CFLAGS) AM_CPPFLAGS = \ -- 1.8.3.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 2/5] tests: use variable for test name in weston-tests-env
Slightly more readable and makes it easier to switch to use $2 for something
in the future (if that's ever needed).
---
tests/weston-tests-env | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/tests/weston-tests-env b/tests/weston-tests-env
index 2e5fa95..b732250 100755
--- a/tests/weston-tests-env
+++ b/tests/weston-tests-env
@@ -1,5 +1,12 @@
#!/bin/bash
+TESTNAME=$1
+
+if test -z $TESTNAME; then
+ echo usage: $(basename $0) test name
+ exit 1;
+fi
+
WESTON=$abs_builddir/../src/weston
LOGDIR=$abs_builddir/logs
@@ -18,17 +25,17 @@ else
BACKEND=$abs_builddir/../src/.libs/wayland-backend.so
fi
-case $1 in
+case $TESTNAME in
*.la|*.so)
$WESTON --backend=$BACKEND \
- --socket=test-$(basename $1) \
- --modules=$abs_builddir/.libs/${1/.la/.so},xwayland.so \
+ --socket=test-$(basename $TESTNAME) \
+
--modules=$abs_builddir/.libs/${TESTNAME/.la/.so},xwayland.so \
--log=$SERVERLOG \
$OUTLOG
;;
*)
- WESTON_TEST_CLIENT_PATH=$abs_builddir/$1 $WESTON \
- --socket=test-$(basename $1) \
+ WESTON_TEST_CLIENT_PATH=$abs_builddir/$TESTNAME $WESTON \
+ --socket=test-$(basename $TESTNAME) \
--backend=$BACKEND \
--log=$SERVERLOG \
--modules=$abs_builddir/.libs/weston-test.so,xwayland.so \
--
1.8.3.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 3/5] tests: include config.h in weston-test-runner
--- tests/weston-test-runner.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/weston-test-runner.c b/tests/weston-test-runner.c index 27ea9e4..7cc1cbe 100644 --- a/tests/weston-test-runner.c +++ b/tests/weston-test-runner.c @@ -20,6 +20,7 @@ * OF THIS SOFTWARE. */ +#include config.h #include unistd.h #include stdio.h #include stdlib.h -- 1.8.3.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 4/5] tests: support -h/--help for the tests
Including listing the tests available in that binary
---
tests/weston-test-runner.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/tests/weston-test-runner.c b/tests/weston-test-runner.c
index 7cc1cbe..fefb93b 100644
--- a/tests/weston-test-runner.c
+++ b/tests/weston-test-runner.c
@@ -53,6 +53,16 @@ run_test(const struct weston_test *t)
exit(EXIT_SUCCESS);
}
+static void
+list_tests(void)
+{
+ const struct weston_test *t;
+
+ fprintf(stderr, Available test names:\n);
+ for (t = __start_test_section; t __stop_test_section; t++)
+ fprintf(stderr,%s\n, t-name);
+}
+
int main(int argc, char *argv[])
{
const struct weston_test *t;
@@ -61,6 +71,14 @@ int main(int argc, char *argv[])
siginfo_t info;
if (argc == 2) {
+ const char *testname = argv[1];
+ if (strcmp(testname, --help) == 0 ||
+ strcmp(testname, -h) == 0) {
+ fprintf(stderr, Usage: %s [test-name]\n,
program_invocation_short_name);
+ list_tests();
+ exit(EXIT_SUCCESS);
+ }
+
t = find_test(argv[1]);
if (t == NULL) {
fprintf(stderr, unknown test: \%s\\n, argv[1]);
--
1.8.3.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 5/5] tests: list available tests if an invalid test name is given
---
tests/weston-test-runner.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/weston-test-runner.c b/tests/weston-test-runner.c
index fefb93b..ed5baf0 100644
--- a/tests/weston-test-runner.c
+++ b/tests/weston-test-runner.c
@@ -82,6 +82,7 @@ int main(int argc, char *argv[])
t = find_test(argv[1]);
if (t == NULL) {
fprintf(stderr, unknown test: \%s\\n, argv[1]);
+ list_tests();
exit(EXIT_FAILURE);
}
--
1.8.3.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 1/5] tests: always build tests
On 11/09/2013 07:58, Peter Hutterer wrote: check_PROGRAMS and friends are only built during make check. Which is perfectly fine. Which is a great way of introducing compiler errors in tests. Agree, but we should fix the workflow, not some arbitrary “problem”. Always build them, TESTS defines what's being run during make check. That’s wrong. The check_* vars are meant this way to avoid forcing test-only dependencies if you disable tests and to allow one to test her code *before* updating the tests. Packagers tend to “fix” that the other way around (moving tests from noinst_ to check_) quite often… --- tests/Makefile.am | 12 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index 82bf630..398a275 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -29,18 +29,14 @@ clean-local: export abs_builddir noinst_LTLIBRARIES = \ - $(weston_test) + $(weston_test) \ + $(module_tests) noinst_PROGRAMS = \ $(setbacklight) \ - matrix-test - -check_LTLIBRARIES =\ - $(module_tests) - -check_PROGRAMS = \ $(shared_tests) \ - $(weston_tests) + $(weston_tests) \ + matrix-test AM_CFLAGS = $(GCC_CFLAGS) AM_CPPFLAGS = \ -- Quentin “Sardem FF7” Glidic ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 2/5] tests: use variable for test name in weston-tests-env
On 11/09/2013 07:58, Peter Hutterer wrote:
Slightly more readable and makes it easier to switch to use $2 for something
in the future (if that's ever needed).
I have a series to remove that script, making it more integrated with
autotools.
---
tests/weston-tests-env | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/tests/weston-tests-env b/tests/weston-tests-env
index 2e5fa95..b732250 100755
--- a/tests/weston-tests-env
+++ b/tests/weston-tests-env
@@ -1,5 +1,12 @@
#!/bin/bash
+TESTNAME=$1
+
+if test -z $TESTNAME; then
+ echo usage: $(basename $0) test name
+ exit 1;
+fi
+
WESTON=$abs_builddir/../src/weston
LOGDIR=$abs_builddir/logs
@@ -18,17 +25,17 @@ else
BACKEND=$abs_builddir/../src/.libs/wayland-backend.so
fi
-case $1 in
+case $TESTNAME in
*.la|*.so)
$WESTON --backend=$BACKEND \
- --socket=test-$(basename $1) \
- --modules=$abs_builddir/.libs/${1/.la/.so},xwayland.so \
+ --socket=test-$(basename $TESTNAME) \
+
--modules=$abs_builddir/.libs/${TESTNAME/.la/.so},xwayland.so \
--log=$SERVERLOG \
$OUTLOG
;;
*)
- WESTON_TEST_CLIENT_PATH=$abs_builddir/$1 $WESTON \
- --socket=test-$(basename $1) \
+ WESTON_TEST_CLIENT_PATH=$abs_builddir/$TESTNAME $WESTON \
+ --socket=test-$(basename $TESTNAME) \
--backend=$BACKEND \
--log=$SERVERLOG \
--modules=$abs_builddir/.libs/weston-test.so,xwayland.so \
--
Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 1/5] tests: always build tests
Quick thought: there's also an important psychological effect to building the tests on a standard make because it promotes them to the same importance as the rest of your code. They become less of an afterthought and it promotes greater care around how people design the tests (eg, making the tests clean, making sure they run quickly), as well as how the rest of the codebase interacts with the tests. We observed a similar effect at Canonical between the projects which had test building on by default as opposed to those that did not. It all depends on whether or not the tests are there as a basic safety line for managing releases or whether or not tests are used as a tool to iterate and improve quality. In the latter case, building them by default is a very sensible decision indeed. Regards, Sam. On Wed, Sep 11, 2013 at 4:53 PM, Peter Hutterer peter.hutte...@who-t.net wrote: On 11/09/13 17:32 , sardemff7+wayl...@sardemff7.net wrote: On 11/09/2013 07:58, Peter Hutterer wrote: check_PROGRAMS and friends are only built during make check. Which is perfectly fine. Which is a great way of introducing compiler errors in tests. Agree, but we should fix the workflow, not some arbitrary “problem”. from my experience, every project I've worked on that has a test suite needed this patch eventually, there's always a way to forget to run make check and suddenly you find out that it's been broken for months. (this is largely because test suites have a tendency to become outdated and useless, but...) Always build them, TESTS defines what's being run during make check. That’s wrong. how so? TESTS defines what's run during make check. check_* defines what's built, the two are related but not the same. The check_* vars are meant this way to avoid forcing test-only dependencies if you disable tests and to allow one to test her code *before* updating the tests. Packagers tend to “fix” that the other way around (moving tests from noinst_ to check_) quite often… I know the principle, I'm claiming that without this, tests will eventually break unless you find a way to force everyone to run make check. Which, coincidentally, wastes maintainer time too if you get a patch that's fine but breaks make check and you have to get through another revision. Cheers, Peter --- tests/Makefile.am | 12 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index 82bf630..398a275 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -29,18 +29,14 @@ clean-local: export abs_builddir noinst_LTLIBRARIES =\ -$(weston_test) +$(weston_test)\ +$(module_tests) noinst_PROGRAMS =\ $(setbacklight)\ -matrix-test - -check_LTLIBRARIES =\ -$(module_tests) - -check_PROGRAMS =\ $(shared_tests)\ -$(weston_tests) +$(weston_tests)\ +matrix-test AM_CFLAGS = $(GCC_CFLAGS) AM_CPPFLAGS =\ ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel -- Sam Spilsbury ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 1/5] tests: always build tests
On 11/09/2013 11:12, Sam Spilsbury wrote: Quick thought: there's also an important psychological effect to building the tests on a standard make because it promotes them to the same importance as the rest of your code. They become less of an afterthought and it promotes greater care around how people design the tests (eg, making the tests clean, making sure they run quickly), as well as how the rest of the codebase interacts with the tests. We observed a similar effect at Canonical between the projects which had test building on by default as opposed to those that did not. Then we should definitely fix users (developers) and their workflow, not some arbitrary “problem”, as I said already. It all depends on whether or not the tests are there as a basic safety line for managing releases or whether or not tests are used as a tool to iterate and improve quality. In the latter case, building them by default is a very sensible decision indeed. Not at all. They should be *run* by default in this case, not just be built. If their point is to check the code, they must do that, not just build against some headers. See the end of this email. On Wed, Sep 11, 2013 at 4:53 PM, Peter Hutterer wrote: from my experience, every project I've worked on that has a test suite needed this patch eventually, there's always a way to forget to run make check and suddenly you find out that it's been broken for months. (this is largely because test suites have a tendency to become outdated and useless, but...) Again, users need a “patch”. I’ll add a bit on that at the end of this email. how so? TESTS defines what's run during make check. check_* defines what's built, the two are related but not the same. Because building tests means you need their *dependencies*, which should definitely not be needed if you do not want to run them. I know the principle, I'm claiming that without this, tests will eventually break unless you find a way to force everyone to run make check. Which, coincidentally, wastes maintainer time too if you get a patch that's fine but breaks make check and you have to get through another revision. So, you have tests that build. Fine. Now what? If nobody run them, they are useless. The best way to force them is the vcs. Just add a git hook that runs them on commit or push, are you are sure you repo will never break. But you should teach people to run them, not force them to do so. A good example (for me) is Cairo. Their tests are noinst_* already (side note: we have to patch that in Exherbo to avoid circular dependencies on fresh installs). They are known to be broken for ages, but they build fine, sure! Having tests built by default is not a bad idea, it is a bad fix. The developer *will* think “Hey, tests build, everything’s ok!” while his commit just broke them. If you want to force tests build or run, you can tweak rules a bit: if RUN_TESTS_BY_DEFAULT ifneq ($(ALREADY_INSIDE),yes) all-local: $(MAKE) ALREADY_INSIDE=yes check endif else if BUILD_TESTS_BY_DEFAULT ifneq ($(ALREADY_INSIDE),yes) all-local: $(MAKE) TESTS= ALREADY_INSIDE=yes check endif endif endif It is both more explicit and saner, imo. As a last note: if tests do not introduce new dependencies *and* are quick to build, it is fine to build them by default from a packager’s point of view (but not with a check_ vs. noinst_ hack). But please, do not tell me “it will make people keep them up-to-date”. They should be *already*, since people *should* run (and thus build) them. Also, having them broken clearly states “we don’t care about them”, which is sane if upstream really do not care. -- Quentin “Sardem FF7” Glidic ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 1/5] tests: always build tests
On Wed, Sep 11, 2013 at 8:38 PM, sardemff7+wayl...@sardemff7.net wrote: On 11/09/2013 11:12, Sam Spilsbury wrote: Quick thought: there's also an important psychological effect to building the tests on a standard make because it promotes them to the same importance as the rest of your code. They become less of an afterthought and it promotes greater care around how people design the tests (eg, making the tests clean, making sure they run quickly), as well as how the rest of the codebase interacts with the tests. We observed a similar effect at Canonical between the projects which had test building on by default as opposed to those that did not. Then we should definitely fix users (developers) and their workflow, not some arbitrary “problem”, as I said already. I think we're all in agreement that having maintained tests is a good thing. Generally speaking I've found that most projects which use a test-driven workflow have them *built* by default and then have a separate rule (eg make check or make test) to run them. They also have a CI system which automatically runs the tests and fails patches which fail the tests. I don't know how well CI systems would work for a patches-on-the-mailinglist type workflow, but there are plenty of very well integrated systems for code hosts like launchpad and github. Building tests by default makes them more obvious, especially to newcomers. Running them by default would certainly make them even more obvious. But we don't run them by default because then we would have to accept the constant time process of running the tests every time make is invoked (which is about seven seconds in our case). There's also the problem with the weston tests not running headless (although they should). A run of the tests isn't mandatory every time the build completes - its up to the discretion of the developer as to when it would be appropriate to run them. Building them by default at least gives you quick feedback of when you stepped on a test. Building the tests by default is a structural change that promotes positive impacts on workflow and more thought about testing. It isn't an entire solution to the problem of developers not writing tests, but it certainly gets you part of the way there. At least with a testsuite which has no dependencies, there doesn't seem to be a very good reason not to at least build them by default. Best Regards, Sam. -- Sam Spilsbury ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH 0/4] Table driven tests for the vertex clipping code
This is the third series of patches for adding table driven tests for the vertex clipping code. From the last series I fixed some of the AM_LDFLAGS nightmare in tests/Makefile.am so that we don't have to build libshared_test into a static library and can instead use a libtool convenience library. Sam Spilsbury (4): Remove AM_LDFLAGS usage Add support for table-driven testing. Split vertex clipping code out into vertex-clipping.c in shared/ Added tests for the vertex clipping code. shared/Makefile.am | 4 +- shared/vertex-clipping.c | 317 + shared/vertex-clipping.h | 65 ++ src/gl-renderer.c | 292 + tests/Makefile.am | 40 -- tests/vertex-clip-test.c | 219 +++ tests/weston-test-runner.c | 123 +++--- tests/weston-test-runner.h | 52 +--- 8 files changed, 749 insertions(+), 363 deletions(-) create mode 100644 shared/vertex-clipping.c create mode 100644 shared/vertex-clipping.h create mode 100644 tests/vertex-clip-test.c -- 1.8.3.2 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH 1/4] Remove AM_LDFLAGS usage
We are not building everything here as a module, only the test modules. --- tests/Makefile.am | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index 82bf630..6234aa2 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -49,7 +49,7 @@ AM_CPPFLAGS = \ -I$(top_builddir)/src \ -DUNIT_TEST \ $(COMPOSITOR_CFLAGS) -AM_LDFLAGS = -module -avoid-version -rpath $(libdir) + config_parser_test_LDADD = \ ../shared/libshared.la \ @@ -58,11 +58,14 @@ config_parser_test_SOURCES =\ config-parser-test.c surface_global_test_la_SOURCES = surface-global-test.c +surface_global_test_la_LDFLAGS = -module -avoid-version -rpath $(libdir) surface_test_la_SOURCES = surface-test.c +surface_test_la_LDFLAGS = -module -avoid-version -rpath $(libdir) weston_test = weston-test.la weston_test_la_LIBADD = $(COMPOSITOR_LIBS) \ ../shared/libshared.la +weston_test_la_LDFLAGS = -module -avoid-version -rpath $(libdir) weston_test_la_CFLAGS = $(GCC_CFLAGS) $(COMPOSITOR_CFLAGS) weston_test_la_SOURCES = \ weston-test.c \ -- 1.8.3.2 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH 2/4] Add support for table-driven testing.
The new TEST_P macro takes a function name and a data argument to
point to an arbitrary array of known size of test data. This allows
multiple tests to be run with different datasets. The array is stored
as a void * but advanced by a known size on each iteration.
The data for each invocation of the test is provided as a data argument,
it is the responsibility of the test to cast it to something sensible.
Also fixed single-test running to only run the tests specified
---
tests/weston-test-runner.c | 123 -
tests/weston-test-runner.h | 52 +--
2 files changed, 113 insertions(+), 62 deletions(-)
diff --git a/tests/weston-test-runner.c b/tests/weston-test-runner.c
index 27ea9e4..646e1c3 100644
--- a/tests/weston-test-runner.c
+++ b/tests/weston-test-runner.c
@@ -46,18 +46,81 @@ find_test(const char *name)
}
static void
-run_test(const struct weston_test *t)
+run_test(const struct weston_test *t, void *data)
{
- t-run();
+ t-run(data);
exit(EXIT_SUCCESS);
}
+static int
+exec_and_report_test(const struct weston_test *t, void *test_data, int
iteration)
+{
+ int success = 0;
+ int hardfail = 0;
+ siginfo_t info;
+
+ pid_t pid = fork();
+ assert(pid = 0);
+
+ if (pid == 0)
+ run_test(t, test_data); /* never returns */
+
+ if (waitid(P_ALL, 0, info, WEXITED)) {
+ fprintf(stderr, waitid failed: %m\n);
+ abort();
+ }
+
+ if (test_data)
+ fprintf(stderr, test \%s/%i\:\t, t-name, iteration);
+ else
+ fprintf(stderr, test \%s\:\t, t-name);
+
+ switch (info.si_code) {
+ case CLD_EXITED:
+ fprintf(stderr, exit status %d, info.si_status);
+ if (info.si_status == EXIT_SUCCESS)
+ success = 1;
+ break;
+ case CLD_KILLED:
+ case CLD_DUMPED:
+ fprintf(stderr, signal %d, info.si_status);
+ if (info.si_status != SIGABRT)
+ hardfail = 1;
+ break;
+ }
+
+ if (t-must_fail)
+ success = !success;
+
+ if (success !hardfail) {
+ fprintf(stderr, , pass.\n);
+ return 1;
+ } else {
+ fprintf(stderr, , fail.\n);
+ return 0;
+ }
+}
+
+/* Returns number of tests and number of pass / fail in param args */
+static int
+iterate_test(const struct weston_test *t, int *passed)
+{
+ int i;
+ void *current_test_data = (void *) t-table_data;
+ for (i = 0; i t-n_elements; ++i, current_test_data +=
t-element_size)
+ {
+ if (exec_and_report_test(t, current_test_data, i))
+ ++(*passed);
+ }
+
+ return t-n_elements;
+}
+
int main(int argc, char *argv[])
{
const struct weston_test *t;
- pid_t pid;
- int total, pass;
- siginfo_t info;
+ int total = 0;
+ int pass = 0;
if (argc == 2) {
t = find_test(argv[1]);
@@ -66,51 +129,19 @@ int main(int argc, char *argv[])
exit(EXIT_FAILURE);
}
- run_test(t);
+ int number_passed_in_test = 0;
+ total += iterate_test(t, number_passed_in_test);
+ pass += number_passed_in_test;
}
-
- pass = 0;
- for (t = __start_test_section; t __stop_test_section; t++) {
- int success = 0;
- int hardfail = 0;
-
- pid = fork();
- assert(pid = 0);
-
- if (pid == 0)
- run_test(t); /* never returns */
-
- if (waitid(P_ALL, 0, info, WEXITED)) {
- fprintf(stderr, waitid failed: %m\n);
- abort();
+ else
+ {
+ for (t = __start_test_section; t __stop_test_section; t++) {
+ int number_passed_in_test = 0;
+ total += iterate_test(t, number_passed_in_test);
+ pass += number_passed_in_test;
}
-
- fprintf(stderr, test \%s\:\t, t-name);
- switch (info.si_code) {
- case CLD_EXITED:
- fprintf(stderr, exit status %d, info.si_status);
- if (info.si_status == EXIT_SUCCESS)
- success = 1;
- break;
- case CLD_KILLED:
- case CLD_DUMPED:
- fprintf(stderr, signal %d, info.si_status);
- if (info.si_status != SIGABRT)
- hardfail = 1;
- break;
- }
-
- if (t-must_fail)
- success = !success;
-
- if (success !hardfail) {
- pass++;
-
[PATCH 4/4] Added tests for the vertex clipping code.
This tests (via the table-driven testing method) that the correct number of vertices and also the correct vertices themselves are generated for an clip box and polygon of up to eight vertices. Also add a libshared_test.a so that we don't have to build weston-test-runner all the time --- tests/Makefile.am| 37 +--- tests/vertex-clip-test.c | 219 +++ 2 files changed, 244 insertions(+), 12 deletions(-) create mode 100644 tests/vertex-clip-test.c diff --git a/tests/Makefile.am b/tests/Makefile.am index 6234aa2..d3d5bf1 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1,7 +1,8 @@ TESTS = $(shared_tests) $(module_tests) $(weston_tests) shared_tests = \ - config-parser.test + config-parser.test \ + vertex-clip.test module_tests = \ surface-test.la \ @@ -50,19 +51,11 @@ AM_CPPFLAGS = \ -DUNIT_TEST \ $(COMPOSITOR_CFLAGS) - -config_parser_test_LDADD = \ - ../shared/libshared.la \ - $(COMPOSITOR_LIBS) -config_parser_test_SOURCES = \ - config-parser-test.c - surface_global_test_la_SOURCES = surface-global-test.c surface_global_test_la_LDFLAGS = -module -avoid-version -rpath $(libdir) surface_test_la_SOURCES = surface-test.c surface_test_la_LDFLAGS = -module -avoid-version -rpath $(libdir) -weston_test = weston-test.la weston_test_la_LIBADD = $(COMPOSITOR_LIBS) \ ../shared/libshared.la weston_test_la_LDFLAGS = -module -avoid-version -rpath $(libdir) @@ -75,17 +68,37 @@ weston_test_la_SOURCES =\ weston_test_runner_src = \ weston-test-runner.c\ weston-test-runner.h + +check_LTLIBRARIES = libshared_test.a + +libshared_test_a_SOURCES = \ + $(weston_test_runner_src) +libshared_test_a_CFLAGS = $(GCC_CFLAGS) $(COMPOSITOR_CFLAGS) + +config_parser_test_LDADD = \ + ../shared/libshared.la \ + libshared_test.a\ + $(COMPOSITOR_LIBS) +config_parser_test_SOURCES = \ + config-parser-test.c +vertex_clip_test_SOURCES = \ + vertex-clip-test.c +vertex_clip_test_LDADD = \ + ../shared/libshared.la \ + libshared_test.a\ + -lm -lrt + weston_test_client_src = \ weston-test-client-helper.c \ weston-test-client-helper.h \ wayland-test-protocol.c \ wayland-test-client-protocol.h \ subsurface-protocol.c \ - subsurface-client-protocol.h\ - $(weston_test_runner_src) + subsurface-client-protocol.h weston_test_client_libs = \ $(SIMPLE_CLIENT_LIBS) \ - ../shared/libshared.la + ../shared/libshared.la \ + libshared_test.a keyboard_weston_SOURCES = keyboard-test.c $(weston_test_client_src) keyboard_weston_LDADD = $(weston_test_client_libs) diff --git a/tests/vertex-clip-test.c b/tests/vertex-clip-test.c new file mode 100644 index 000..51188b2 --- /dev/null +++ b/tests/vertex-clip-test.c @@ -0,0 +1,219 @@ +/* + * Copyright © 2013 Sam Spilsbury smspil...@gmail.com + * + * Permission to use, copy, modify, distribute, and sell this software and + * its documentation for any purpose is hereby granted without fee, provided + * that the above copyright notice appear in all copies and that both that + * copyright notice and this permission notice appear in supporting + * documentation, and that the name of the copyright holders not be used in + * advertising or publicity pertaining to distribution of the software + * without specific, written prior permission. The copyright holders make + * no representations about the suitability of this software for any + * purpose. It is provided as is without express or implied warranty. + * + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS + * SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND + * FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY + * SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER + * RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF + * CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ +#include assert.h +#include stdio.h +#include stdlib.h +#include string.h +#include math.h + +#include weston-test-runner.h + +#include ../shared/vertex-clipping.h + +#define BOUNDING_BOX_TOP_Y 100.0f +#define BOUNDING_BOX_LEFT_X 50.0f +#define BOUNDING_BOX_RIGHT_X 100.0f +#define BOUNDING_BOX_BOTTOM_Y 50.0f + +#define INSIDE_X1 (BOUNDING_BOX_LEFT_X + 1.0f) +#define INSIDE_X2 (BOUNDING_BOX_RIGHT_X - 1.0f) +#define INSIDE_Y1 (BOUNDING_BOX_BOTTOM_Y + 1.0f) +#define INSIDE_Y2 (BOUNDING_BOX_TOP_Y - 1.0f) + +#define OUTSIDE_X1 (BOUNDING_BOX_LEFT_X - 1.0f)
[PATCH weston] xwm: place transient windows at the right position
---
src/xwayland/window-manager.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/xwayland/window-manager.c b/src/xwayland/window-manager.c
index a8b949b..4c7256e 100644
--- a/src/xwayland/window-manager.c
+++ b/src/xwayland/window-manager.c
@@ -967,7 +967,7 @@ weston_wm_handle_property_notify(struct weston_wm *wm,
xcb_generic_event_t *even
static void
weston_wm_window_create(struct weston_wm *wm,
- xcb_window_t id, int width, int height, int override)
+ xcb_window_t id, int width, int height, int x, int y,
int override)
{
struct weston_wm_window *window;
uint32_t values[1];
@@ -991,6 +991,8 @@ weston_wm_window_create(struct weston_wm *wm,
window-override_redirect = override;
window-width = width;
window-height = height;
+ window-x = x;
+ window-y = y;
geometry_reply = xcb_get_geometry_reply(wm-conn, geometry_cookie,
NULL);
/* technically we should use XRender and check the visual format's
@@ -1026,6 +1028,7 @@ weston_wm_handle_create_notify(struct weston_wm *wm,
xcb_generic_event_t *event)
weston_wm_window_create(wm, create_notify-window,
create_notify-width, create_notify-height,
+ create_notify-x, create_notify-y,
create_notify-override_redirect);
}
@@ -1062,6 +1065,7 @@ weston_wm_handle_reparent_notify(struct weston_wm *wm,
xcb_generic_event_t *even
if (reparent_notify-parent == wm-screen-root) {
weston_wm_window_create(wm, reparent_notify-window, 10, 10,
+ reparent_notify-x, reparent_notify-y,
reparent_notify-override_redirect);
} else if (!our_resource(wm, reparent_notify-parent)) {
window = hash_table_lookup(wm-window_hash,
@@ -1916,7 +1920,7 @@ xserver_map_shell_surface(struct weston_wm *wm,
WL_SHELL_SURFACE_FULLSCREEN_METHOD_DEFAULT,
0, NULL);
return;
- } else if (!window-override_redirect) {
+ } else if (!window-override_redirect !window-transient_for) {
shell_interface-set_toplevel(window-shsurf);
return;
} else {
--
1.8.4
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] xwm: set the shell_surface's title
add a new function pointer to the weston_shell_interface struct that
shells will set accordingly.
---
src/compositor.h | 2 ++
src/shell.c | 11 +--
src/xwayland/window-manager.c | 8
3 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/src/compositor.h b/src/compositor.h
index 891db0e..6c12757 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -107,6 +107,8 @@ struct weston_shell_interface {
int (*move)(struct shell_surface *shsurf, struct weston_seat *ws);
int (*resize)(struct shell_surface *shsurf,
struct weston_seat *ws, uint32_t edges);
+ void (*set_title)(struct shell_surface *shsurf,
+ const char *title);
};
diff --git a/src/shell.c b/src/shell.c
index dc15bfa..ea4315a 100644
--- a/src/shell.c
+++ b/src/shell.c
@@ -1595,13 +1595,19 @@ shell_surface_pong(struct wl_client *client, struct
wl_resource *resource,
}
static void
+set_title(struct shell_surface *shsurf, const char *title)
+{
+ free(shsurf-title);
+ shsurf-title = strdup(title);
+}
+
+static void
shell_surface_set_title(struct wl_client *client,
struct wl_resource *resource, const char *title)
{
struct shell_surface *shsurf = wl_resource_get_user_data(resource);
- free(shsurf-title);
- shsurf-title = strdup(title);
+ set_title(shsurf, title);
}
static void
@@ -4583,6 +4589,7 @@ module_init(struct weston_compositor *ec,
ec-shell_interface.set_xwayland = set_xwayland;
ec-shell_interface.move = surface_move;
ec-shell_interface.resize = surface_resize;
+ ec-shell_interface.set_title = set_title;
wl_list_init(shell-input_panel.surfaces);
diff --git a/src/xwayland/window-manager.c b/src/xwayland/window-manager.c
index 4c7256e..14bc0a3 100644
--- a/src/xwayland/window-manager.c
+++ b/src/xwayland/window-manager.c
@@ -337,6 +337,8 @@ static void
weston_wm_window_read_properties(struct weston_wm_window *window)
{
struct weston_wm *wm = window-wm;
+ struct weston_shell_interface *shell_interface =
+ wm-server-compositor-shell_interface;
#define F(field) offsetof(struct weston_wm_window, field)
const struct {
@@ -431,6 +433,9 @@ weston_wm_window_read_properties(struct weston_wm_window
*window)
}
free(reply);
}
+
+ if (window-shsurf window-name)
+ shell_interface-set_title(window-shsurf, window-name);
}
static void
@@ -1913,6 +1918,9 @@ xserver_map_shell_surface(struct weston_wm *wm,
window-surface,
shell_client);
+ if (window-name)
+ shell_interface-set_title(window-shsurf, window-name);
+
if (window-fullscreen) {
window-saved_width = window-width;
window-saved_height = window-height;
--
1.8.4
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v2] xwm: set the shell_surface's title
add a new function pointer to the weston_shell_interface struct that
shells will set accordingly.
---
src/compositor.h | 2 ++
src/shell.c | 11 +--
src/xwayland/window-manager.c | 11 +++
3 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/src/compositor.h b/src/compositor.h
index 3c1b643..ead0c91 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -107,6 +107,8 @@ struct weston_shell_interface {
int (*move)(struct shell_surface *shsurf, struct weston_seat *ws);
int (*resize)(struct shell_surface *shsurf,
struct weston_seat *ws, uint32_t edges);
+ void (*set_title)(struct shell_surface *shsurf,
+ const char *title);
};
diff --git a/src/shell.c b/src/shell.c
index dc15bfa..ea4315a 100644
--- a/src/shell.c
+++ b/src/shell.c
@@ -1595,13 +1595,19 @@ shell_surface_pong(struct wl_client *client, struct
wl_resource *resource,
}
static void
+set_title(struct shell_surface *shsurf, const char *title)
+{
+ free(shsurf-title);
+ shsurf-title = strdup(title);
+}
+
+static void
shell_surface_set_title(struct wl_client *client,
struct wl_resource *resource, const char *title)
{
struct shell_surface *shsurf = wl_resource_get_user_data(resource);
- free(shsurf-title);
- shsurf-title = strdup(title);
+ set_title(shsurf, title);
}
static void
@@ -4583,6 +4589,7 @@ module_init(struct weston_compositor *ec,
ec-shell_interface.set_xwayland = set_xwayland;
ec-shell_interface.move = surface_move;
ec-shell_interface.resize = surface_resize;
+ ec-shell_interface.set_title = set_title;
wl_list_init(shell-input_panel.surfaces);
diff --git a/src/xwayland/window-manager.c b/src/xwayland/window-manager.c
index f775734..b4f64d3 100644
--- a/src/xwayland/window-manager.c
+++ b/src/xwayland/window-manager.c
@@ -363,6 +363,8 @@ static void
weston_wm_window_read_properties(struct weston_wm_window *window)
{
struct weston_wm *wm = window-wm;
+ struct weston_shell_interface *shell_interface =
+ wm-server-compositor-shell_interface;
#define F(field) offsetof(struct weston_wm_window, field)
const struct {
@@ -468,6 +470,9 @@ weston_wm_window_read_properties(struct weston_wm_window
*window)
}
free(reply);
}
+
+ if (window-shsurf window-name)
+ shell_interface-set_title(window-shsurf, window-name);
}
static void
@@ -1875,6 +1880,9 @@ surface_destroy(struct wl_listener *listener, void *data)
wm_log(surface for xid %d destroyed\n, window-id);
+ /* This should have been freed by the shell.
+ Don't try to use it later. */
+ window-shsurf = NULL;
window-surface = NULL;
}
@@ -2029,6 +2037,9 @@ xserver_map_shell_surface(struct weston_wm *wm,
window-surface,
shell_client);
+ if (window-name)
+ shell_interface-set_title(window-shsurf, window-name);
+
if (window-fullscreen) {
window-saved_width = window-width;
window-saved_height = window-height;
--
1.8.4
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v2] xwm: place transient windows at the right position
---
src/xwayland/window-manager.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/xwayland/window-manager.c b/src/xwayland/window-manager.c
index bb766d3..f775734 100644
--- a/src/xwayland/window-manager.c
+++ b/src/xwayland/window-manager.c
@@ -1002,7 +1002,7 @@ weston_wm_handle_property_notify(struct weston_wm *wm,
xcb_generic_event_t *even
static void
weston_wm_window_create(struct weston_wm *wm,
- xcb_window_t id, int width, int height, int override)
+ xcb_window_t id, int width, int height, int x, int y,
int override)
{
struct weston_wm_window *window;
uint32_t values[1];
@@ -1026,6 +1026,8 @@ weston_wm_window_create(struct weston_wm *wm,
window-override_redirect = override;
window-width = width;
window-height = height;
+ window-x = x;
+ window-y = y;
geometry_reply = xcb_get_geometry_reply(wm-conn, geometry_cookie,
NULL);
/* technically we should use XRender and check the visual format's
@@ -1076,6 +1078,7 @@ weston_wm_handle_create_notify(struct weston_wm *wm,
xcb_generic_event_t *event)
weston_wm_window_create(wm, create_notify-window,
create_notify-width, create_notify-height,
+ create_notify-x, create_notify-y,
create_notify-override_redirect);
}
@@ -1112,6 +1115,7 @@ weston_wm_handle_reparent_notify(struct weston_wm *wm,
xcb_generic_event_t *even
if (reparent_notify-parent == wm-screen-root) {
weston_wm_window_create(wm, reparent_notify-window, 10, 10,
+ reparent_notify-x, reparent_notify-y,
reparent_notify-override_redirect);
} else if (!our_resource(wm, reparent_notify-parent)) {
window = hash_table_lookup(wm-window_hash,
@@ -2037,7 +2041,7 @@ xserver_map_shell_surface(struct weston_wm *wm,
shell_interface-set_fullscreen(window-shsurf,
WL_SHELL_SURFACE_FULLSCREEN_METHOD_DEFAULT,
0, output);
- } else if (!window-override_redirect) {
+ } else if (!window-override_redirect !window-transient_for) {
shell_interface-set_toplevel(window-shsurf);
return;
} else {
--
1.8.4
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] xwm: reset the surface and shsurf field on surface destruction
we may still get events from x, so by setting those fields to NULL we make sure we don't try to use destroyed objects. --- src/xwayland/window-manager.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/xwayland/window-manager.c b/src/xwayland/window-manager.c index e1f26f6..912553e 100644 --- a/src/xwayland/window-manager.c +++ b/src/xwayland/window-manager.c @@ -1878,6 +1878,9 @@ surface_destroy(struct wl_listener *listener, void *data) container_of(listener, struct weston_wm_window, surface_destroy_listener); + window-surface = NULL; + /* This should have been freed by the shell */ + window-shsurf = NULL; wm_log(surface for xid %d destroyed\n, window-id); window-surface = NULL; -- 1.8.4 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] xwm: reset the surface and shsurf field on surface destruction
Ignore this one, i made a bit of mess with rebase. Sorry for the noise 2013/9/11 Giulio Camuffo giuliocamu...@gmail.com we may still get events from x, so by setting those fields to NULL we make sure we don't try to use destroyed objects. --- src/xwayland/window-manager.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/xwayland/window-manager.c b/src/xwayland/window-manager.c index e1f26f6..912553e 100644 --- a/src/xwayland/window-manager.c +++ b/src/xwayland/window-manager.c @@ -1878,6 +1878,9 @@ surface_destroy(struct wl_listener *listener, void *data) container_of(listener, struct weston_wm_window, surface_destroy_listener); + window-surface = NULL; + /* This should have been freed by the shell */ + window-shsurf = NULL; wm_log(surface for xid %d destroyed\n, window-id); window-surface = NULL; -- 1.8.4 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] compositor: reset surface's resource field on resource destruction
with the surface ref-count feature a surface may live on after its
resource was destroyed. set it to NULL in that case, so that code
like find_resource_for_surface() in input.c will act accordingly.
---
src/compositor.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/compositor.c b/src/compositor.c
index 88df279..e4ce428 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -1070,6 +1070,9 @@ destroy_surface(struct wl_resource *resource)
{
struct weston_surface *surface = wl_resource_get_user_data(resource);
+ /* the surface may live so make sure nothing thinks
+ it still has a resource */
+ surface-resource = NULL;
weston_surface_destroy(surface);
}
--
1.8.4
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v2] Copying xkb_info when creating a seat causes problems
On Thu, Sep 05, 2013 at 01:31:40PM +, Andrew Wedgbury wrote:
Hi Kristian,
Here's a new patch for ref counting weston_xkb_info, as suggested.
So a seat created with a NULL keymap will now point to the global xkb_info.
Andrew, that looks great. It's a nice cleanup in itself and of course
fixes the premature munmap error. Thanks, applied.
Kristian
---
src/compositor.h |5 ++--
src/input.c| 77
src/text-backend.c |4 +--
3 files changed, 53 insertions(+), 33 deletions(-)
diff --git a/src/compositor.h b/src/compositor.h
index 6db3c61..3755650 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -398,6 +398,7 @@ struct weston_xkb_info {
int keymap_fd;
size_t keymap_size;
char *keymap_area;
+ int32_t ref_count;
xkb_mod_index_t shift_mod;
xkb_mod_index_t caps_mod;
xkb_mod_index_t ctrl_mod;
@@ -468,7 +469,7 @@ struct weston_seat {
void (*led_update)(struct weston_seat *ws, enum weston_led leds);
- struct weston_xkb_info xkb_info;
+ struct weston_xkb_info *xkb_info;
struct {
struct xkb_state *state;
enum weston_led leds;
@@ -588,7 +589,7 @@ struct weston_compositor {
struct xkb_rule_names xkb_names;
struct xkb_context *xkb_context;
- struct weston_xkb_info xkb_info;
+ struct weston_xkb_info *xkb_info;
/* Raw keyboard processing (no libxkbcommon initialization or handling)
*/
int use_xkbcommon;
diff --git a/src/input.c b/src/input.c
index aa40b4e..78b6ead 100644
--- a/src/input.c
+++ b/src/input.c
@@ -760,24 +760,24 @@ notify_modifiers(struct weston_seat *seat, uint32_t
serial)
/* And update the modifier_state for bindings. */
mods_lookup = mods_depressed | mods_latched;
seat-modifier_state = 0;
- if (mods_lookup (1 seat-xkb_info.ctrl_mod))
+ if (mods_lookup (1 seat-xkb_info-ctrl_mod))
seat-modifier_state |= MODIFIER_CTRL;
- if (mods_lookup (1 seat-xkb_info.alt_mod))
+ if (mods_lookup (1 seat-xkb_info-alt_mod))
seat-modifier_state |= MODIFIER_ALT;
- if (mods_lookup (1 seat-xkb_info.super_mod))
+ if (mods_lookup (1 seat-xkb_info-super_mod))
seat-modifier_state |= MODIFIER_SUPER;
- if (mods_lookup (1 seat-xkb_info.shift_mod))
+ if (mods_lookup (1 seat-xkb_info-shift_mod))
seat-modifier_state |= MODIFIER_SHIFT;
/* Finally, notify the compositor that LEDs have changed. */
if (xkb_state_led_index_is_active(seat-xkb_state.state,
- seat-xkb_info.num_led))
+ seat-xkb_info-num_led))
leds |= LED_NUM_LOCK;
if (xkb_state_led_index_is_active(seat-xkb_state.state,
- seat-xkb_info.caps_led))
+ seat-xkb_info-caps_led))
leds |= LED_CAPS_LOCK;
if (xkb_state_led_index_is_active(seat-xkb_state.state,
- seat-xkb_info.scroll_led))
+ seat-xkb_info-scroll_led))
leds |= LED_SCROLL_LOCK;
if (leds != seat-xkb_state.leds seat-led_update)
seat-led_update(seat, leds);
@@ -1243,8 +1243,8 @@ seat_get_keyboard(struct wl_client *client, struct
wl_resource *resource,
if (seat-compositor-use_xkbcommon) {
wl_keyboard_send_keymap(cr, WL_KEYBOARD_KEYMAP_FORMAT_XKB_V1,
- seat-xkb_info.keymap_fd,
- seat-xkb_info.keymap_size);
+ seat-xkb_info-keymap_fd,
+ seat-xkb_info-keymap_size);
} else {
int null_fd = open(/dev/null, O_RDONLY);
wl_keyboard_send_keymap(cr, WL_KEYBOARD_KEYMAP_FORMAT_NO_KEYMAP,
@@ -1351,8 +1351,12 @@ weston_compositor_xkb_init(struct weston_compositor
*ec,
return 0;
}
-static void xkb_info_destroy(struct weston_xkb_info *xkb_info)
+static void
+weston_xkb_info_destroy(struct weston_xkb_info *xkb_info)
{
+ if (--xkb_info-ref_count 0)
+ return;
+
if (xkb_info-keymap)
xkb_map_unref(xkb_info-keymap);
@@ -1360,6 +1364,7 @@ static void xkb_info_destroy(struct weston_xkb_info
*xkb_info)
munmap(xkb_info-keymap_area, xkb_info-keymap_size);
if (xkb_info-keymap_fd = 0)
close(xkb_info-keymap_fd);
+ free(xkb_info);
}
void
@@ -1377,14 +1382,22 @@ weston_compositor_xkb_destroy(struct
weston_compositor *ec)
free((char *) ec-xkb_names.layout);
free((char *) ec-xkb_names.variant);
free((char *) ec-xkb_names.options);
-
- xkb_info_destroy(ec-xkb_info);
+
+ if
Re: [PATCH xf86-video-intel] xwayland: shortcut Enter/ExitVT
On Wed, Aug 21, 2013 at 10:02:43AM +0200, Giovanni Campagna wrote:
Avoids a warning due to drmGetMaster and a crash with multimonitor,
caused by not having an intel_mode.
I finally applied this patch. The one thing I wasn't sure about is
the call to xf86SetDesiredModes() in I830EnterVT, but I don't think we
need it after all. We're still missing something though, the xrandr
output is missing a few bits:
Screen 0: minimum 320 x 200, current 1024 x 600, maximum 8192 x 8192
XWAYLAND-1 connected (normal left inverted right x axis y axis)
1024x600 59.7
which, compared to the native Xorg output:
Screen 0: minimum 320 x 200, current 4000 x 1600, maximum 8192 x 8192
eDP1 connected primary 1440x900+0+0 (normal left inverted right x axis y axis)
30mm x 179mm
1440x900 60.0*+
1024x768 60.0
800x60060.3 56.2
640x48059.9
is missing the 'primary' flags and the '1440x900+0+0' part. But at
least it comes up now.
thanks,
Kristian
---
src/intel_driver.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/intel_driver.c b/src/intel_driver.c
index d1da72d..b7702e6 100644
--- a/src/intel_driver.c
+++ b/src/intel_driver.c
@@ -1157,6 +1157,9 @@ static void I830LeaveVT(VT_FUNC_ARGS_DECL)
intel_screen_private *intel = intel_get_screen_private(scrn);
int ret;
+ if (xorgWayland)
+ return TRUE;
+
xf86RotateFreeShadow(scrn);
xf86_hide_cursors(scrn);
@@ -1176,6 +1179,9 @@ static Bool I830EnterVT(VT_FUNC_ARGS_DECL)
intel_screen_private *intel = intel_get_screen_private(scrn);
int ret;
+ if (xorgWayland)
+ return TRUE;
+
ret = drmSetMaster(intel-drmSubFD);
if (ret) {
xf86DrvMsg(scrn-scrnIndex, X_WARNING,
--
1.8.3.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH] client: Fix handling display-reader_count if poll fails
In wl_display_dispatch_queue, if poll fails then it would previously
return immediately and leak a reference in display-reader_count. Then
if the application ignores the error and tries to read again it will
block forever. This can happen for example if the poll fails with
EINTR which the application might consider to be a recoverable error.
This patch just makes it decrement the reader count when poll fails.
---
src/wayland-client.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/wayland-client.c b/src/wayland-client.c
index 48f06c7..576a773 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -1224,8 +1224,10 @@ wl_display_dispatch_queue(struct wl_display *display,
pfd[0].fd = display-fd;
pfd[0].events = POLLIN;
- if (poll(pfd, 1, -1) == -1)
+ if (poll(pfd, 1, -1) == -1) {
+ display-reader_count--;
return -1;
+ }
pthread_mutex_lock(display-mutex);
--
1.8.3.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] compositor: reset surface's resource field on resource destruction
Remember the problem we had a couple months ago with null surfaces being
passed to wl_pointer.leave if the leave was caused by the surface being
destroyed? If you set surface-resource = NULL before destroying it this
bug will come up again.
On Sep 11, 2013 11:28 AM, Giulio Camuffo giuliocamu...@gmail.com wrote:
with the surface ref-count feature a surface may live on after its
resource was destroyed. set it to NULL in that case, so that code
like find_resource_for_surface() in input.c will act accordingly.
---
src/compositor.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/compositor.c b/src/compositor.c
index 88df279..e4ce428 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -1070,6 +1070,9 @@ destroy_surface(struct wl_resource *resource)
{
struct weston_surface *surface =
wl_resource_get_user_data(resource);
+ /* the surface may live so make sure nothing thinks
+ it still has a resource */
+ surface-resource = NULL;
weston_surface_destroy(surface);
}
--
1.8.4
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 4/4] utils: tweak wl_list for better doxygen output
On Wed, Aug 28, 2013 at 06:02:02PM -0500, Aaron Faanes wrote:
---
src/wayland-util.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/wayland-util.h b/src/wayland-util.h
index 02d9458..fd735f7 100644
--- a/src/wayland-util.h
+++ b/src/wayland-util.h
@@ -61,8 +61,9 @@ struct wl_interface {
const struct wl_message *events;
};
-/**
- * wl_list - linked list
+/** \class wl_list
+ *
+ * \brief doubly-inked list
Thanks, committed with 'inked' fixed to 'linked' :-)
Kristian
*
* The list head is of struct wl_list type, and must be initialized
* using wl_list_init(). All entries in the list must be of the same
--
1.8.3.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v2] gl_renderer: Use EGL_WAYLAND_Y_INVERTED_WL to query wl_buffer's orientation
On Thu, Aug 29, 2013 at 11:36:44AM +0400, Stanislav Vorobiov wrote:
---
src/compositor.c |1 +
src/compositor.h |1 +
src/gl-renderer.c| 14 +-
src/weston-egl-ext.h |4
4 files changed, 19 insertions(+), 1 deletion(-)
Yeah, I think this looks good now. We need to document the new token
in the spec in mesa at least, in particular that if querying for
EGL_WAYLAND_Y_INVERTED_WL returns false, fall back to y-inverted.
thanks,
Kristian
diff --git a/src/compositor.c b/src/compositor.c
index 74f0aab..8c9e0fe 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -1088,6 +1088,7 @@ weston_buffer_from_resource(struct wl_resource
*resource)
buffer-resource = resource;
wl_signal_init(buffer-destroy_signal);
buffer-destroy_listener.notify = weston_buffer_destroy_handler;
+ buffer-y_inverted = 1;
wl_resource_add_destroy_listener(resource, buffer-destroy_listener);
return buffer;
diff --git a/src/compositor.h b/src/compositor.h
index 6db3c61..cb15d69 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -605,6 +605,7 @@ struct weston_buffer {
};
int32_t width, height;
uint32_t busy_count;
+ int y_inverted;
};
struct weston_buffer_reference {
diff --git a/src/gl-renderer.c b/src/gl-renderer.c
index e321211..0eee09e 100644
--- a/src/gl-renderer.c
+++ b/src/gl-renderer.c
@@ -77,6 +77,7 @@ struct gl_surface_state {
enum buffer_type buffer_type;
int pitch; /* in pixels */
int height; /* in pixels */
+ int y_inverted;
};
struct gl_renderer {
@@ -599,7 +600,11 @@ texture_region(struct weston_surface *es,
pixman_region32_t *region,
weston_surface_to_buffer_float(es, sx, sy,
bx, by);
*(v++) = bx * inv_width;
- *(v++) = by * inv_height;
+ if (gs-y_inverted) {
+ *(v++) = by * inv_height;
+ } else {
+ *(v++) = (gs-height - by) * inv_height;
+ }
}
vtxcnt[nvtx++] = n;
@@ -1260,6 +1265,7 @@ gl_renderer_attach_shm(struct weston_surface *es,
struct weston_buffer *buffer,
gs-target = GL_TEXTURE_2D;
gs-buffer_type = BUFFER_TYPE_SHM;
gs-needs_full_upload = 1;
+ gs-y_inverted = 1;
ensure_textures(gs, 1);
glBindTexture(GL_TEXTURE_2D, gs-textures[0]);
@@ -1284,6 +1290,8 @@ gl_renderer_attach_egl(struct weston_surface *es,
struct weston_buffer *buffer,
EGL_WIDTH, buffer-width);
gr-query_buffer(gr-egl_display, buffer-legacy_buffer,
EGL_HEIGHT, buffer-height);
+ gr-query_buffer(gr-egl_display, buffer-legacy_buffer,
+ EGL_WAYLAND_Y_INVERTED_WL, buffer-y_inverted);
for (i = 0; i gs-num_images; i++)
gr-destroy_image(gr-egl_display, gs-images[i]);
@@ -1340,6 +1348,7 @@ gl_renderer_attach_egl(struct weston_surface *es,
struct weston_buffer *buffer,
gs-pitch = buffer-width;
gs-height = buffer-height;
gs-buffer_type = BUFFER_TYPE_EGL;
+ gs-y_inverted = buffer-y_inverted;
}
static void
@@ -1363,6 +1372,7 @@ gl_renderer_attach(struct weston_surface *es, struct
weston_buffer *buffer)
glDeleteTextures(gs-num_textures, gs-textures);
gs-num_textures = 0;
gs-buffer_type = BUFFER_TYPE_NULL;
+ gs-y_inverted = 1;
return;
}
@@ -1377,6 +1387,7 @@ gl_renderer_attach(struct weston_surface *es, struct
weston_buffer *buffer)
weston_log(unhandled buffer type!\n);
weston_buffer_reference(gs-buffer_ref, NULL);
gs-buffer_type = BUFFER_TYPE_NULL;
+ gs-y_inverted = 1;
}
}
@@ -1409,6 +1420,7 @@ gl_renderer_create_surface(struct weston_surface
*surface)
* by zero there.
*/
gs-pitch = 1;
+ gs-y_inverted = 1;
pixman_region32_init(gs-texture_damage);
surface-renderer_state = gs;
diff --git a/src/weston-egl-ext.h b/src/weston-egl-ext.h
index 6aa49fd..bab87be 100644
--- a/src/weston-egl-ext.h
+++ b/src/weston-egl-ext.h
@@ -62,6 +62,10 @@ typedef EGLBoolean (EGLAPIENTRYP
PFNEGLQUERYWAYLANDBUFFERWL) (EGLDisplay dpy, st
#define EGL_BUFFER_AGE_EXT 0x313D
#endif
+#ifndef EGL_WAYLAND_Y_INVERTED_WL
+#define EGL_WAYLAND_Y_INVERTED_WL0x31DB /*
eglQueryWaylandBufferWL attribute */
+#endif
+
/* Mesas gl2ext.h and probably Khronos upstream defined
* GL_EXT_unpack_subimage with non _EXT suffixed GL_UNPACK_* tokens.
* In case we're using that mess, manually
Re: [PATCH weston v3] Copying xkb_info when creating a seat causes problems
On Fri, Sep 06, 2013 at 08:29:12AM +, Andrew Wedgbury wrote:
Sorry, I missed updating use of xkb_info in compositor-x11.c.
I've updated the patch.
Argh, and I ended up applying and pushing v2 before seeing this. Oh
well, I've applied the compositor-x11.c part of the patch and the x11
backend modifiers work again.
thanks,
Kristian
---
src/compositor-x11.c |2 +-
src/compositor.h |5 ++--
src/input.c | 77
+++---
src/text-backend.c |4 +--
4 files changed, 54 insertions(+), 34 deletions(-)
diff --git a/src/compositor-x11.c b/src/compositor-x11.c
index a896612..e04ea06 100644
--- a/src/compositor-x11.c
+++ b/src/compositor-x11.c
@@ -160,7 +160,7 @@ x11_compositor_get_keymap(struct x11_compositor *c)
static uint32_t
get_xkb_mod_mask(struct x11_compositor *c, uint32_t in)
{
- struct weston_xkb_info *info = c-core_seat.xkb_info;
+ struct weston_xkb_info *info = c-core_seat.xkb_info;
uint32_t ret = 0;
if ((in ShiftMask) info-shift_mod != XKB_MOD_INVALID)
diff --git a/src/compositor.h b/src/compositor.h
index 6db3c61..3755650 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -398,6 +398,7 @@ struct weston_xkb_info {
int keymap_fd;
size_t keymap_size;
char *keymap_area;
+ int32_t ref_count;
xkb_mod_index_t shift_mod;
xkb_mod_index_t caps_mod;
xkb_mod_index_t ctrl_mod;
@@ -468,7 +469,7 @@ struct weston_seat {
void (*led_update)(struct weston_seat *ws, enum weston_led leds);
- struct weston_xkb_info xkb_info;
+ struct weston_xkb_info *xkb_info;
struct {
struct xkb_state *state;
enum weston_led leds;
@@ -588,7 +589,7 @@ struct weston_compositor {
struct xkb_rule_names xkb_names;
struct xkb_context *xkb_context;
- struct weston_xkb_info xkb_info;
+ struct weston_xkb_info *xkb_info;
/* Raw keyboard processing (no libxkbcommon initialization or handling)
*/
int use_xkbcommon;
diff --git a/src/input.c b/src/input.c
index aa40b4e..78b6ead 100644
--- a/src/input.c
+++ b/src/input.c
@@ -760,24 +760,24 @@ notify_modifiers(struct weston_seat *seat, uint32_t
serial)
/* And update the modifier_state for bindings. */
mods_lookup = mods_depressed | mods_latched;
seat-modifier_state = 0;
- if (mods_lookup (1 seat-xkb_info.ctrl_mod))
+ if (mods_lookup (1 seat-xkb_info-ctrl_mod))
seat-modifier_state |= MODIFIER_CTRL;
- if (mods_lookup (1 seat-xkb_info.alt_mod))
+ if (mods_lookup (1 seat-xkb_info-alt_mod))
seat-modifier_state |= MODIFIER_ALT;
- if (mods_lookup (1 seat-xkb_info.super_mod))
+ if (mods_lookup (1 seat-xkb_info-super_mod))
seat-modifier_state |= MODIFIER_SUPER;
- if (mods_lookup (1 seat-xkb_info.shift_mod))
+ if (mods_lookup (1 seat-xkb_info-shift_mod))
seat-modifier_state |= MODIFIER_SHIFT;
/* Finally, notify the compositor that LEDs have changed. */
if (xkb_state_led_index_is_active(seat-xkb_state.state,
- seat-xkb_info.num_led))
+ seat-xkb_info-num_led))
leds |= LED_NUM_LOCK;
if (xkb_state_led_index_is_active(seat-xkb_state.state,
- seat-xkb_info.caps_led))
+ seat-xkb_info-caps_led))
leds |= LED_CAPS_LOCK;
if (xkb_state_led_index_is_active(seat-xkb_state.state,
- seat-xkb_info.scroll_led))
+ seat-xkb_info-scroll_led))
leds |= LED_SCROLL_LOCK;
if (leds != seat-xkb_state.leds seat-led_update)
seat-led_update(seat, leds);
@@ -1243,8 +1243,8 @@ seat_get_keyboard(struct wl_client *client, struct
wl_resource *resource,
if (seat-compositor-use_xkbcommon) {
wl_keyboard_send_keymap(cr, WL_KEYBOARD_KEYMAP_FORMAT_XKB_V1,
- seat-xkb_info.keymap_fd,
- seat-xkb_info.keymap_size);
+ seat-xkb_info-keymap_fd,
+ seat-xkb_info-keymap_size);
} else {
int null_fd = open(/dev/null, O_RDONLY);
wl_keyboard_send_keymap(cr, WL_KEYBOARD_KEYMAP_FORMAT_NO_KEYMAP,
@@ -1351,8 +1351,12 @@ weston_compositor_xkb_init(struct weston_compositor
*ec,
return 0;
}
-static void xkb_info_destroy(struct weston_xkb_info *xkb_info)
+static void
+weston_xkb_info_destroy(struct weston_xkb_info *xkb_info)
{
+ if (--xkb_info-ref_count 0)
+ return;
+
if (xkb_info-keymap)
xkb_map_unref(xkb_info-keymap);
@@ -1360,6 +1364,7 @@ static
Re: [PATCH] fbdev: Support of TrueColor and Directcolor of video hardware
On Tue, Sep 03, 2013 at 04:47:43PM +0200, mchalain [marc.chal...@gmail.com]
wrote:
From: mchalain marc.chal...@gmail.com
Thanks, committed.
Kristian
backend check the type of the framebuffer and accept DirectColor and TrueColor
I use a switch case to implement other cases in the future.
---
src/compositor-fbdev.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/src/compositor-fbdev.c b/src/compositor-fbdev.c
index 9c3d17e..dd3a71d 100644
--- a/src/compositor-fbdev.c
+++ b/src/compositor-fbdev.c
@@ -242,8 +242,15 @@ calculate_pixman_format(struct fb_var_screeninfo *vinfo,
return 0;
/* We only handle true-colour frame buffers at the moment. */
- if (finfo-visual != FB_VISUAL_TRUECOLOR || vinfo-grayscale != 0)
- return 0;
+ switch(finfo-visual) {
+ case FB_VISUAL_TRUECOLOR:
+ case FB_VISUAL_DIRECTCOLOR:
+ if (vinfo-grayscale != 0)
+ return 0;
+ break;
+ default:
+ return 0;
+ }
/* We only support formats with MSBs on the left. */
if (vinfo-red.msb_right != 0 || vinfo-green.msb_right != 0 ||
--
1.7.9.5
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH 0/4] Add support for eglSwapInterval
Here are some patches to add support for eglSwapInterval in Mesa's Wayland platform. The first two patches are for Mesa to actually implement it. The third patch is a minor tweak to Weston which is needed to get it to send the buffer release events at the correct time. The fourth patch is to mesa-demos to update eglut to work with the latest Wayland. The es2gears_wayland example can then be used to test the swap interval. I'll also attach a bodged version of the simple-egl example which modifies eglSwapInterval every 5 seconds and displays the frame rate. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH 1/2] wayland: Add support for eglSwapInterval
The Wayland EGL platform now respects the eglSwapInterval value. The value is
clamped to either 0 or 1 because it is difficult (and probably not useful) to
sync to more than 1 redraw.
The main change is that if the swap interval is 0 then it simply doesn't
install a frame callback so that the next time eglSwapBuffers is called it
won't delay.
The second change is that in get_back_bo instead of returning with an error if
all three buffers are locked it will now block in a dispatch loop so that it
can receive the buffer release events. The assumption is that the compositor
is unlikely to lock all three buffers so if we find that all the buffers are
locked then we are probably just rendering faster than we are processing the
release events. Therefore the release events should be available very early.
This also moves the vblank configuration defines from platform_x11.c to the
common egl_dri2.h header so they can be shared by both platforms.
---
src/egl/drivers/dri2/egl_dri2.h | 6 ++
src/egl/drivers/dri2/platform_wayland.c | 121 ++--
src/egl/drivers/dri2/platform_x11.c | 6 --
3 files changed, 107 insertions(+), 26 deletions(-)
diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
index fba5f81..849927b 100644
--- a/src/egl/drivers/dri2/egl_dri2.h
+++ b/src/egl/drivers/dri2/egl_dri2.h
@@ -221,6 +221,12 @@ struct dri2_egl_image
__DRIimage *dri_image;
};
+/* From xmlpool/options.h, user exposed so should be stable */
+#define DRI_CONF_VBLANK_NEVER 0
+#define DRI_CONF_VBLANK_DEF_INTERVAL_0 1
+#define DRI_CONF_VBLANK_DEF_INTERVAL_1 2
+#define DRI_CONF_VBLANK_ALWAYS_SYNC 3
+
/* standard typecasts */
_EGL_DRIVER_STANDARD_TYPECASTS(dri2_egl)
_EGL_DRIVER_TYPECAST(dri2_egl_image, _EGLImage, obj)
diff --git a/src/egl/drivers/dri2/platform_wayland.c
b/src/egl/drivers/dri2/platform_wayland.c
index ffc5959..83e7aab 100644
--- a/src/egl/drivers/dri2/platform_wayland.c
+++ b/src/egl/drivers/dri2/platform_wayland.c
@@ -180,8 +180,16 @@ dri2_create_window_surface(_EGLDriver *drv, _EGLDisplay
*disp,
_EGLConfig *conf, EGLNativeWindowType window,
const EGLint *attrib_list)
{
- return dri2_create_surface(drv, disp, EGL_WINDOW_BIT, conf,
+ struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
+ _EGLSurface *surf;
+
+ surf = dri2_create_surface(drv, disp, EGL_WINDOW_BIT, conf,
window, attrib_list);
+
+ if (surf != NULL)
+ drv-API.SwapInterval(drv, disp, surf, dri2_dpy-default_swap_interval);
+
+ return surf;
}
/**
@@ -261,24 +269,36 @@ get_back_bo(struct dri2_egl_surface *dri2_surf,
__DRIbuffer *buffer)
__DRIimage *image;
int i, name, pitch;
- /* There might be a buffer release already queued that wasn't processed */
- wl_display_dispatch_queue_pending(dri2_dpy-wl_dpy, dri2_dpy-wl_queue);
-
if (dri2_surf-back == NULL) {
- for (i = 0; i ARRAY_SIZE(dri2_surf-color_buffers); i++) {
- /* Get an unlocked buffer, preferrably one with a dri_buffer already
- * allocated. */
-if (dri2_surf-color_buffers[i].locked)
-continue;
- if (dri2_surf-back == NULL)
- dri2_surf-back = dri2_surf-color_buffers[i];
- else if (dri2_surf-back-dri_image == NULL)
- dri2_surf-back = dri2_surf-color_buffers[i];
+ /* There might be a buffer release already queued that wasn't processed
*/
+ wl_display_dispatch_queue_pending(dri2_dpy-wl_dpy, dri2_dpy-wl_queue);
+
+ while (1) {
+ for (i = 0; i ARRAY_SIZE(dri2_surf-color_buffers); i++) {
+/* Get an unlocked buffer, preferrably one with a dri_buffer
already
+ * allocated. */
+if (dri2_surf-color_buffers[i].locked)
+ continue;
+if (dri2_surf-back == NULL)
+ dri2_surf-back = dri2_surf-color_buffers[i];
+else if (dri2_surf-back-dri_image == NULL)
+ dri2_surf-back = dri2_surf-color_buffers[i];
+ }
+
+ if (dri2_surf-back)
+break;
+
+ /* If we make it here then here then all of the buffers are locked.
+ * It wouldn't make sense for the compositor to keep all three
+ * buffers so it must mean that we are rendering too fast without
+ * getting a chance to see the release events. Therefore we can just
+ * block for the release event here */
+ if (wl_display_dispatch_queue(dri2_dpy-wl_dpy,
+ dri2_dpy-wl_queue) 0)
+return -1;
}
}
- if (dri2_surf-back == NULL)
- return -1;
if (dri2_surf-back-dri_image == NULL) {
dri2_surf-back-dri_image =
dri2_dpy-image-createImage(dri2_dpy-dri_screen,
@@ -511,11 +531,13 @@ dri2_swap_buffers_with_damage(_EGLDriver *drv,
if (ret 0)
return EGL_FALSE;
- dri2_surf-frame_callback =
Re: [PATCH] wayland-server: Fix a uninitialized warning from clang
On Wed, Aug 28, 2013 at 05:43:34PM -0500, Aaron Faanes wrote:
This warning is unnecessary, since the pointer in question is only used
for pointer arithmetic, but setting it explicitly to NULL doesn't hurt.
This is more than just a warning, reading an uninitialized value is
undefined and not guaranteed to give the same result for multiple
reads (the wl_container_of macro reads it twice).
Applied, thanks.
Kristian
---
src/wayland-server.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/wayland-server.c b/src/wayland-server.c
index a1d69e5..d7c58b9 100644
--- a/src/wayland-server.c
+++ b/src/wayland-server.c
@@ -501,7 +501,7 @@ wl_resource_get_link(struct wl_resource *resource)
WL_EXPORT struct wl_resource *
wl_resource_from_link(struct wl_list *link)
{
- struct wl_resource *resource;
+ struct wl_resource *resource = NULL;
return wl_container_of(link, resource, link);
}
--
1.8.3.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 1/4] utils: Document wl_container_of
On Wed, Aug 28, 2013 at 06:01:59PM -0500, Aaron Faanes wrote:
The explanation added is admittedly verbose (and reads more like a
proof), but my hope is that the comment should demystify how
wl_container_of works for those not familiar with the technique used.
Understanding this function also helps understanding how wl_list works.
Hi Aaron,
I think it would be good to document wl_container_of, but I feel that
this is a little to verbose. Maybe we can just describe what it does
and not how it does it? Also, it's not specific to wl_list, it can be
used anywhere you have a pointer to a field in the struct and want to
work your way back to a pointer to the struct.
Kristian
---
src/wayland-util.h | 45 +
1 file changed, 45 insertions(+)
diff --git a/src/wayland-util.h b/src/wayland-util.h
index e5e4e25..02d9458 100644
--- a/src/wayland-util.h
+++ b/src/wayland-util.h
@@ -110,6 +110,51 @@ int wl_list_length(const struct wl_list *list);
int wl_list_empty(const struct wl_list *list);
void wl_list_insert_list(struct wl_list *list, struct wl_list *other);
+/**
+ * Retrieves a pointer to the content of a wl_list item.
+ *
+ * The ptr is a pointer to the wl_list item.
+ *
+ * The sample must be a pointer to the type of content that the list item
+ * stores, though it need not be a valid pointer; a null pointer will
+ * suffice.
+ *
+ * The member is the named location of ptr within the content.
+ *
+ * For instance, using the example above, assume the following list of
+ * integers:
+ *
+ * struct item_t {
+ * int foo;
+ * struct wl_list link;
+ * };
+ *
+ * Which lays out this content in memory like so:
+ *
+ * [content..][wl_list item][trailing content...]
+ *
+ * To retrieve a pointer to content given a pointer to its link, you would
+ * need to retrieve this size:
+ *
+ * [content..][wl_list item][trailing content...]
+ * ^-^
+ *
+ * Since we know the link is within item_t, we can retrieve a relative
+ * position of that link using subtraction:
+ *
+ * struct item_t *sample = NULL;
+ * offset = (sample-link) - sample;
+ *
+ * This offset will be valid even if sample is a null or invalid pointer
since
+ * we never dereference it; we only perform pointer arithmetic on it.
+ *
+ * Finally, with the offset and the given link, we can retrieve a pointer to
+ * content and complete the macro:
+ *
+ * content = link - offset;
+ * content = link - ((sample-link) - sample);
+ * content = ptr - ((sample-member) - sample);
+ */
#ifdef __GNUC__
#define wl_container_of(ptr, sample, member) \
(__typeof__(sample))((char *)(ptr) - \
--
1.8.3.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH] Revert compositor: Queue buffer.release instead of sending immediately
This reverts commit eccef6aadd142103ed151883e61c0e7a2fd98639.
Queuing the buffer release event instead of posting it immediately
causes problems if the client is not installing a frame callback and
instead is waiting for the buffer release events to throttle its
rendering. This will happen in Mesa when eglSwapInterval is set to
zero so that the client will try to render faster than the compositor
can display the buffers. The client will still want to throttle itself
to the buffer release events in that case so that it doesn't end up
allocating endless amounts of buffers while waiting for the release
events. Without this patch nothing will cause the connection to be
flushed so the client will just block forever.
Conflicts:
compositor/compositor.c
---
src/compositor.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/compositor.c b/src/compositor.c
index 74f0aab..3eb97e0 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -1113,8 +1113,8 @@ weston_buffer_reference(struct weston_buffer_reference
*ref,
ref-buffer-busy_count--;
if (ref-buffer-busy_count == 0) {
assert(wl_resource_get_client(ref-buffer-resource));
- wl_resource_queue_event(ref-buffer-resource,
- WL_BUFFER_RELEASE);
+ wl_resource_post_event(ref-buffer-resource,
+ WL_BUFFER_RELEASE);
}
wl_list_remove(ref-destroy_listener.link);
}
--
1.8.3.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 3/4] wayland-server: Document wl_signal
On Wed, Aug 28, 2013 at 06:02:01PM -0500, Aaron Faanes wrote:
This all sounds good, thanks.
Kristian
---
src/wayland-server.h | 37 +
1 file changed, 37 insertions(+)
diff --git a/src/wayland-server.h b/src/wayland-server.h
index 59fa43a..0e662de 100644
--- a/src/wayland-server.h
+++ b/src/wayland-server.h
@@ -143,22 +143,52 @@ struct wl_listener {
wl_notify_func_t notify;
};
+/** \class wl_signal
+ *
+ * \brief A source of a type of observable event
+ *
+ * Signals are recognized points where significant events can be observed.
+ * Compositors as well as the server can provide signals. Observers are
+ * added through \ref wl_signal_add.
+ */
struct wl_signal {
struct wl_list listener_list;
};
+/** Initialize a new \ref wl_signal for use.
+ *
+ * \param signal The signal that will be initialized
+ *
+ * \memberof wl_signal
+ */
static inline void
wl_signal_init(struct wl_signal *signal)
{
wl_list_init(signal-listener_list);
}
+/** Add the specified listener to this signal.
+ *
+ * \param signal The signal that will emit events to the listener
+ * \param listener The listener to add
+ *
+ * \memberof wl_signal
+ */
static inline void
wl_signal_add(struct wl_signal *signal, struct wl_listener *listener)
{
wl_list_insert(signal-listener_list.prev, listener-link);
}
+/** Gets the list item for the specified listener.
+ *
+ * \param signal The signal that contains the specified listener
+ * \param notify The listener that is the target of this search
+ * \return the list item that corresponds to the specified listener, or NULL
+ * if none was found
+ *
+ * \memberof wl_signal
+ */
static inline struct wl_listener *
wl_signal_get(struct wl_signal *signal, wl_notify_func_t notify)
{
@@ -171,6 +201,13 @@ wl_signal_get(struct wl_signal *signal, wl_notify_func_t
notify)
return NULL;
}
+/** Emits this signal, notifying all registered listeners.
+ *
+ * \param signal The signal object that will emit the signal
+ * \param data The data that will be emitted with the signal
+ *
+ * \memberof wl_signal
+ */
static inline void
wl_signal_emit(struct wl_signal *signal, void *data)
{
--
1.8.3.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 2/4] wayland-server: Document wl_listener
On Wed, Aug 28, 2013 at 06:02:00PM -0500, Aaron Faanes wrote:
---
src/wayland-server.h | 9 +
1 file changed, 9 insertions(+)
diff --git a/src/wayland-server.h b/src/wayland-server.h
index d77050d..59fa43a 100644
--- a/src/wayland-server.h
+++ b/src/wayland-server.h
@@ -129,6 +129,15 @@ wl_client_get_object(struct wl_client *client, uint32_t
id);
void
wl_client_post_no_memory(struct wl_client *client);
+/** \class wl_listener
+ *
+ * \brief A single listener for Wayland events and signals
+ *
+ * Many Wayland events use wl_listener for notification of significant
+ * events, like destruction. Clients should create these objects manually
+ * and register them as listeners using the registration methods (usually
+ * suffixed with _add and _notify) provided by other objects.
This sounds pretty good, though I'd explain that they're always used
with wl_signal and how to add a listener to a signal. Then explain
that sometimes the wl_signal is in an opaque struct and we provide
accessor functions to add the listener and give a couple of specific
examples.
Finally, maybe add a comment that it's common to embed a listener
struct in a bigger struct and then use wl_container_of to get back to
the bigger struct on the notify callback.
Kristian
+ */
struct wl_listener {
struct wl_list link;
wl_notify_func_t notify;
--
1.8.3.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH] eglut_wayland: Update to the Wayland 1.0 API
This fixes build errors with the eglut_wayland backend. In particular
it now uses the wl_registry and the new main loop mechanism from the
Wayland 1.0 API.
---
src/egl/eglut/eglut_wayland.c | 108 --
1 file changed, 94 insertions(+), 14 deletions(-)
diff --git a/src/egl/eglut/eglut_wayland.c b/src/egl/eglut/eglut_wayland.c
index 61207d2..25a51bc 100644
--- a/src/egl/eglut/eglut_wayland.c
+++ b/src/egl/eglut/eglut_wayland.c
@@ -1,6 +1,10 @@
#include wayland-client.h
#include wayland-egl.h
+#include poll.h
+#include errno.h
+#include string.h
+
#include eglutint.h
struct display {
@@ -20,42 +24,74 @@ static struct display display = {0, };
static struct window window = {0, };
static void
-display_handle_global(struct wl_display *display, uint32_t id,
- const char *interface, uint32_t version, void *data)
+registry_handle_global(void *data, struct wl_registry *registry, uint32_t id,
+ const char *interface, uint32_t version)
{
struct display *d = data;
if (strcmp(interface, wl_compositor) == 0) {
d-compositor =
- wl_display_bind(display, id, wl_compositor_interface);
+ wl_registry_bind(registry, id, wl_compositor_interface, 1);
} else if (strcmp(interface, wl_shell) == 0) {
- d-shell = wl_display_bind(display, id, wl_shell_interface);
+ d-shell = wl_registry_bind(registry, id, wl_shell_interface, 1);
}
}
+static void
+registry_handle_global_remove(void *data, struct wl_registry *registry,
+ uint32_t name)
+{
+}
+
+static const struct wl_registry_listener registry_listener = {
+ registry_handle_global,
+ registry_handle_global_remove
+};
+
+static void
+sync_callback(void *data, struct wl_callback *callback, uint32_t serial)
+{
+ int *done = data;
+
+ *done = 1;
+ wl_callback_destroy(callback);
+}
+
+static const struct wl_callback_listener sync_listener = {
+ sync_callback
+};
+
static int
-event_mask_update(uint32_t mask, void *data)
+wayland_roundtrip(struct wl_display *display)
{
- struct display *d = data;
+ struct wl_callback *callback;
+ int done = 0, ret = 0;
- d-mask = mask;
+ callback = wl_display_sync(display);
+ wl_callback_add_listener(callback, sync_listener, done);
+ while (ret != -1 !done)
+ ret = wl_display_dispatch(display);
- return 0;
+ if (!done)
+ wl_callback_destroy(callback);
+
+ return ret;
}
void
_eglutNativeInitDisplay(void)
{
+ struct wl_registry *registry;
+
_eglut-native_dpy = display.display = wl_display_connect(NULL);
if (!_eglut-native_dpy)
_eglutFatal(failed to initialize native display);
- wl_display_add_global_listener(_eglut-native_dpy,
- display_handle_global, display);
-
- wl_display_get_fd(_eglut-native_dpy, event_mask_update, display);
- wl_display_iterate(_eglut-native_dpy, WL_DISPLAY_READABLE);
+ registry = wl_display_get_registry(_eglut-native_dpy);
+ wl_registry_add_listener(registry, registry_listener, display);
+ wayland_roundtrip(_eglut-native_dpy);
+ wl_registry_destroy(registry);
_eglut-surface_type = EGL_WINDOW_BIT;
}
@@ -124,12 +160,56 @@ draw(void *data, struct wl_callback *callback, uint32_t
time)
void
_eglutNativeEventLoop(void)
{
+ struct pollfd pollfd;
+ int ret;
+
draw(window, NULL, 0);
+ pollfd.fd = wl_display_get_fd(display.display);
+ pollfd.events = POLLIN;
+ pollfd.revents = 0;
+
while (1) {
- wl_display_iterate(display.display, display.mask);
+ wl_display_dispatch_pending(display.display);
if (_eglut-idle_cb)
_eglut-idle_cb();
+
+ ret = wl_display_flush(display.display);
+ if (ret 0 errno == EAGAIN)
+ pollfd.events |= POLLOUT;
+ else if (ret 0)
+ break;
+
+ if (poll(pollfd, 1, _eglut-redisplay ? 0 : -1) == -1)
+ break;
+
+ if (pollfd.revents (POLLERR | POLLHUP))
+ break;
+
+ if (pollfd.revents POLLIN) {
+ ret = wl_display_dispatch(display.display);
+ if (ret == -1)
+break;
+ }
+
+ if (pollfd.revents POLLOUT) {
+ ret = wl_display_flush(display.display);
+ if (ret == 0)
+pollfd.events = ~POLLOUT;
+ else if (ret == -1 errno != EAGAIN)
+break;
+ }
+
+ if (_eglut-redisplay) {
+ struct eglut_window *win = _eglut-current;
+
+ _eglut-redisplay = 0;
+
+ if (win-display_cb)
+win-display_cb();
+
+ eglSwapBuffers(_eglut-dpy, win-surface);
+ }
}
}
--
1.8.3.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 0/4] Add support for eglSwapInterval
Here's a hacked version of simple-egl which modifies eglSwapInterval
every 5 seconds and shows the frame rate to verify that it works.
- Neil
-
Intel Corporation (UK) Limited
Registered No. 1134945 (England)
Registered Office: Pipers Way, Swindon SN3 1RJ
VAT No: 860 2173 47
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
/*
* Copyright © 2011 Benjamin Franzke
* Copyright © 2013 Intel Corporation
*
* Permission to use, copy, modify, distribute, and sell this software and its
* documentation for any purpose is hereby granted without fee, provided that
* the above copyright notice appear in all copies and that both that copyright
* notice and this permission notice appear in supporting documentation, and
* that the name of the copyright holders not be used in advertising or
* publicity pertaining to distribution of the software without specific,
* written prior permission. The copyright holders make no representations
* about the suitability of this software for any purpose. It is provided as
* is without express or implied warranty.
*
* THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
* INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
* EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
* CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
* DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
* TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
* OF THIS SOFTWARE.
*/
#include stdio.h
#include stdlib.h
#include string.h
#include stdbool.h
#include math.h
#include assert.h
#include signal.h
#include poll.h
#include errno.h
#include sys/time.h
#include wayland-client.h
#include wayland-egl.h
#include GLES2/gl2.h
#include EGL/egl.h
#include EGL/eglext.h
struct window;
struct seat;
struct display {
struct wl_display *display;
struct wl_registry *registry;
struct wl_compositor *compositor;
struct wl_shell *shell;
struct {
EGLDisplay dpy;
EGLContext ctx;
EGLConfig conf;
} egl;
struct window *window;
};
struct geometry {
int width, height;
};
struct window {
struct display *display;
struct geometry geometry;
struct wl_egl_window *native;
struct wl_surface *surface;
struct wl_shell_surface *shell_surface;
EGLSurface egl_surface;
int frame_count;
int last_fps_time;
int interval_seconds;
int swap_interval;
};
static int running = 1;
#define MIN(a, b) ((a) (b) ? (a) : (b))
static void
init_egl(struct display *display)
{
static const EGLint context_attribs[] = {
EGL_CONTEXT_CLIENT_VERSION, 2,
EGL_NONE
};
EGLint config_attribs[] = {
EGL_SURFACE_TYPE, EGL_WINDOW_BIT,
EGL_RED_SIZE, 1,
EGL_GREEN_SIZE, 1,
EGL_BLUE_SIZE, 1,
EGL_ALPHA_SIZE, 0,
EGL_RENDERABLE_TYPE, EGL_OPENGL_ES2_BIT,
EGL_NONE
};
EGLint major, minor, n;
EGLBoolean ret;
display-egl.dpy = eglGetDisplay(display-display);
assert(display-egl.dpy);
ret = eglInitialize(display-egl.dpy, major, minor);
assert(ret == EGL_TRUE);
ret = eglBindAPI(EGL_OPENGL_ES_API);
assert(ret == EGL_TRUE);
ret = eglChooseConfig(display-egl.dpy, config_attribs,
display-egl.conf, 1, n);
assert(ret n == 1);
display-egl.ctx = eglCreateContext(display-egl.dpy,
display-egl.conf,
EGL_NO_CONTEXT, context_attribs);
assert(display-egl.ctx);
}
static void
fini_egl(struct display *display)
{
eglTerminate(display-egl.dpy);
eglReleaseThread();
}
static void
handle_ping(void *data, struct wl_shell_surface *shell_surface,
uint32_t serial)
{
wl_shell_surface_pong(shell_surface, serial);
}
static void
handle_configure(void *data, struct wl_shell_surface *shell_surface,
uint32_t edges, int32_t width, int32_t height)
{
struct window *window = data;
if (window-native)
wl_egl_window_resize(window-native, width, height, 0, 0);
window-geometry.width = width;
window-geometry.height = height;
}
static void
handle_popup_done(void *data, struct wl_shell_surface *shell_surface)
{
}
static const struct wl_shell_surface_listener shell_surface_listener = {
handle_ping,
handle_configure,
handle_popup_done
};
static void
create_surface(struct window *window)
{
struct display *display = window-display;
EGLBoolean ret;
window-surface = wl_compositor_create_surface(display-compositor);
window-shell_surface = wl_shell_get_shell_surface(display-shell,
window-surface);
wl_shell_surface_add_listener(window-shell_surface,
shell_surface_listener, window);
window-native =
wl_egl_window_create(window-surface,
window-geometry.width,
window-geometry.height);
window-egl_surface =
Re: [PATCH] gitignore: add ./compile
On Sun, Sep 08, 2013 at 11:12:33AM +, Chang Liu wrote: ./compile is a GNU autotools helper script and should be ignored by git Thanks, added. Kristian --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 99b7089..f243100 100644 --- a/.gitignore +++ b/.gitignore @@ -15,6 +15,7 @@ ctags /aclocal.m4 /wayland-scanner.m4 /autom4te.cache +/compile /config.guess /config.h /config.h.in -- 1.8.3.4 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland] wayland-server: Add a wl_resource_for_each_safe macro
On Fri, Sep 06, 2013 at 05:56:27PM +0100, Rob Bradford wrote: From: Rob Bradford r...@linux.intel.com A version of wl_resource_for_each that is safe for iteration when items in the list are removed. Looks good, committed. Kristian --- src/wayland-server.h | 8 1 file changed, 8 insertions(+) diff --git a/src/wayland-server.h b/src/wayland-server.h index d77050d..c0365c0 100644 --- a/src/wayland-server.h +++ b/src/wayland-server.h @@ -313,6 +313,14 @@ wl_resource_get_destroy_listener(struct wl_resource *resource, wl_resource_get_link(resource) != (list); \ resource = wl_resource_from_link(wl_resource_get_link(resource)-next)) +#define wl_resource_for_each_safe(resource, tmp, list) \ + for (resource = 0, tmp = 0, \ + resource = wl_resource_from_link((list)-next),\ + tmp = wl_resource_from_link((list)-next-next); \ + wl_resource_get_link(resource) != (list); \ + resource = tmp, \ + tmp = wl_resource_from_link(wl_resource_get_link(resource)-next)) + struct wl_shm_buffer; struct wl_shm_buffer * -- 1.8.3.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] Restore alpha after fade or zoom effect.
On Sat, Sep 07, 2013 at 05:10:25PM +0200, Axel Davy wrote:
After these effects, alpha could not have been 1.0, preventing
not redrawing behind opaque windows.
Nice catch, we don't wan't to end up with slightly transparent
windows. I think we can just set surface-alpha to animation-stop in
weston_surface_animation_destroy() though. We already reset the
transformation there, so resetting alpha doesn't seem like a big problem.
Kristian
Signed-off-by: Axel Davy axel.d...@ens.fr
---
src/shell.c | 15 +--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/src/shell.c b/src/shell.c
index cd94aa5..d7e2d1e 100644
--- a/src/shell.c
+++ b/src/shell.c
@@ -3485,6 +3485,15 @@ weston_surface_set_initial_position (struct
weston_surface *surface,
}
static void
+surface_restore_alpha(struct weston_surface_animation *animation, void *data)
+{
+ struct weston_surface *surface = data;
+
+ surface-alpha = 1.0;
+ weston_surface_geometry_dirty(surface);
+}
+
+static void
map(struct desktop_shell *shell, struct weston_surface *surface,
int32_t width, int32_t height, int32_t sx, int32_t sy)
{
@@ -3576,10 +3585,12 @@ map(struct desktop_shell *shell, struct
weston_surface *surface,
{
switch (shell-win_animation_type) {
case ANIMATION_FADE:
- weston_fade_run(surface, 0.0, 1.0, 300.0, NULL, NULL);
+ weston_fade_run(surface, 0.0, 1.0, 300.0,
+ surface_restore_alpha, surface);
break;
case ANIMATION_ZOOM:
- weston_zoom_run(surface, 0.5, 1.0, NULL, NULL);
+ weston_zoom_run(surface, 0.5, 1.0,
+ surface_restore_alpha, surface);
break;
default:
break;
--
1.8.1.2
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] Revert compositor: Queue buffer.release instead of sending immediately
Hi,
On 11 September 2013 14:31, Neil Roberts n...@linux.intel.com wrote:
This reverts commit eccef6aadd142103ed151883e61c0e7a2fd98639.
Queuing the buffer release event instead of posting it immediately
causes problems if the client is not installing a frame callback and
instead is waiting for the buffer release events to throttle its
rendering. This will happen in Mesa when eglSwapInterval is set to
zero so that the client will try to render faster than the compositor
can display the buffers. The client will still want to throttle itself
to the buffer release events in that case so that it doesn't end up
allocating endless amounts of buffers while waiting for the release
events. Without this patch nothing will cause the connection to be
flushed so the client will just block forever.
Yeah, this is good to me. Media pipelines also really want release
events ASAP, so they can reuse hardware-decoder buffers.
Cheers,
Daniel
---
src/compositor.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/compositor.c b/src/compositor.c
index 74f0aab..3eb97e0 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -1113,8 +1113,8 @@ weston_buffer_reference(struct weston_buffer_reference
*ref,
ref-buffer-busy_count--;
if (ref-buffer-busy_count == 0) {
assert(wl_resource_get_client(ref-buffer-resource));
- wl_resource_queue_event(ref-buffer-resource,
- WL_BUFFER_RELEASE);
+ wl_resource_post_event(ref-buffer-resource,
+ WL_BUFFER_RELEASE);
}
wl_list_remove(ref-destroy_listener.link);
}
--
1.8.3.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] add libhybris support
On Fri, Sep 06, 2013 at 03:16:09PM +0300, Adrian Negreanu wrote:
it uses the Android fbdev HAL[1] (through libhybris[2])
and the libhybris implementation of wayland-egl.
Configure flags:
cairo:
--enable-glesv2=yes --enable-egl=yes
weston:
--with-cairo-glesv2 --enable-fbdev-compositor
hybris:
--enable-wayland --enable-arch=x86
--with-android-headers=android-headers --enable-alinker=jb
The android headers are extracted from an AOSP tree,
using hybris/utils/extract-headers.sh
[1]:
https://github.com/android/platform_hardware_libhardware/blob/master/include/hardware/fb.h
[2]: https://github.com/libhybris/libhybris
Signed-off-by: Adrian Negreanu adrian.m.negre...@intel.com
Thanks Adrian, that's very cool. Patch committed.
Kristian
---
src/compositor-fbdev.c | 98
+---
1 file changed, 76 insertions(+), 22 deletions(-)
diff --git a/src/compositor-fbdev.c b/src/compositor-fbdev.c
index 7a6f255..6308707 100644
--- a/src/compositor-fbdev.c
+++ b/src/compositor-fbdev.c
@@ -43,6 +43,7 @@
#include launcher-util.h
#include pixman-renderer.h
#include udev-seat.h
+#include gl-renderer.h
struct fbdev_compositor {
struct weston_compositor base;
@@ -51,6 +52,7 @@ struct fbdev_compositor {
struct udev *udev;
struct tty *tty;
struct udev_input input;
+ int use_pixman;
};
struct fbdev_screeninfo {
@@ -90,6 +92,7 @@ struct fbdev_output {
struct fbdev_parameters {
int tty;
char *device;
+ int use_gl;
};
static const char default_seat[] = seat0;
@@ -118,7 +121,7 @@ fbdev_output_start_repaint_loop(struct weston_output
*output)
}
static void
-fbdev_output_repaint(struct weston_output *base, pixman_region32_t *damage)
+fbdev_output_repaint_pixman(struct weston_output *base, pixman_region32_t
*damage)
{
struct fbdev_output *output = to_fbdev_output(base);
struct weston_compositor *ec = output-base.compositor;
@@ -191,6 +194,26 @@ fbdev_output_repaint(struct weston_output *base,
pixman_region32_t *damage)
100 / output-mode.refresh);
}
+static void
+fbdev_output_repaint(struct weston_output *base, pixman_region32_t *damage)
+{
+ struct fbdev_output *output = to_fbdev_output(base);
+ struct fbdev_compositor *fbc = output-compositor;
+ struct weston_compositor *ec = fbc-base;
+
+ if (fbc-use_pixman) {
+ fbdev_output_repaint_pixman(base,damage);
+ } else {
+ ec-renderer-repaint_output(base, damage);
+ /* Update the damage region. */
+ pixman_region32_subtract(ec-primary_plane.damage,
+ ec-primary_plane.damage, damage);
+
+ wl_event_source_timer_update(output-finish_frame_timer,
+ 100 / output-mode.refresh);
+ }
+}
+
static int
finish_frame_handler(void *data)
{
@@ -489,10 +512,11 @@ fbdev_output_create(struct fbdev_compositor *compositor,
weston_log(Creating frame buffer failed.\n);
goto out_free;
}
-
- if (fbdev_frame_buffer_map(output, fb_fd) 0) {
- weston_log(Mapping frame buffer failed.\n);
- goto out_free;
+ if (compositor-use_pixman) {
+ if (fbdev_frame_buffer_map(output, fb_fd) 0) {
+ weston_log(Mapping frame buffer failed.\n);
+ goto out_free;
+ }
}
output-base.start_repaint_loop = fbdev_output_start_repaint_loop;
@@ -586,8 +610,18 @@ fbdev_output_create(struct fbdev_compositor *compositor,
if (output-base.transform != WL_OUTPUT_TRANSFORM_NORMAL)
pixman_image_set_transform(output-shadow_surface, transform);
- if (pixman_renderer_output_create(output-base) 0)
- goto out_shadow_surface;
+ if (compositor-use_pixman) {
+ if (pixman_renderer_output_create(output-base) 0)
+ goto out_shadow_surface;
+ } else {
+ setenv(HYBRIS_EGLPLATFORM, wayland, 1);
+ if (gl_renderer_output_create(output-base,
+ (EGLNativeWindowType)NULL) 0) {
+ weston_log(gl_renderer_output_create failed.\n);
+ goto out_shadow_surface;
+ }
+ }
+
loop = wl_display_get_event_loop(compositor-base.wl_display);
output-finish_frame_timer =
@@ -621,23 +655,28 @@ static void
fbdev_output_destroy(struct weston_output *base)
{
struct fbdev_output *output = to_fbdev_output(base);
+ struct fbdev_compositor *compositor = output-compositor;
weston_log(Destroying fbdev output.\n);
/* Close the frame buffer. */
fbdev_output_disable(base);
- if (base-renderer_state != NULL)
-
Re: [PATCH weston 1/2] vaapi-recorder: Encode frames in a separate thread
On Fri, Sep 06, 2013 at 05:49:37PM +0300, Ander Conselvan de Oliveira wrote:
Previously, vaapi_recorder_frame() would wait until the encoded
contents for a frame is written to the output file descriptor. This
delayed the repainting of the next frame, and affected frame rate
when capturing with high resolutions. Instead, wait only if there is
and attempted to encode two frames at the same time.
Increases framerate from 30 to 60 fps when capturing at 1920x1200 on
my SandryBridge system, although there are periodic slowdowns due to
disk writes.
Thanks Ander, both committed.
Kristian
---
src/compositor-drm.c | 2 -
src/vaapi-recorder.c | 101
---
2 files changed, 96 insertions(+), 7 deletions(-)
diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index eb3ec61..7f6ffbc 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -2471,8 +2471,6 @@ recorder_frame_notify(struct wl_listener *listener,
void *data)
}
vaapi_recorder_frame(output-recorder, fd, output-current-stride / 4);
-
- close(fd);
}
static void *
diff --git a/src/vaapi-recorder.c b/src/vaapi-recorder.c
index c0210f0..e9127da 100644
--- a/src/vaapi-recorder.c
+++ b/src/vaapi-recorder.c
@@ -47,11 +47,13 @@
#include stdint.h
#include string.h
#include unistd.h
+#include assert.h
#include sys/types.h
#include sys/stat.h
#include fcntl.h
+#include pthread.h
#include va/va.h
#include va/va_drm.h
@@ -89,6 +91,16 @@ struct vaapi_recorder {
int width, height;
int frame_count;
+ int destroying;
+ pthread_t worker_thread;
+ pthread_mutex_t mutex;
+ pthread_cond_t input_cond;
+
+ struct {
+ int valid;
+ int prime_fd, stride;
+ } input;
+
VADisplay va_dpy;
/* video post processing is used for colorspace conversion */
@@ -116,6 +128,9 @@ struct vaapi_recorder {
} encoder;
};
+static void *
+worker_thread_function(void *);
+
/* bistream code used for writing the packed headers */
#define BITSTREAM_ALLOCATE_STEPPING 4096
@@ -886,6 +901,33 @@ vpp_destroy(struct vaapi_recorder *r)
vaDestroyConfig(r-va_dpy, r-vpp.cfg);
}
+static int
+setup_worker_thread(struct vaapi_recorder *r)
+{
+ pthread_mutex_init(r-mutex, NULL);
+ pthread_cond_init(r-input_cond, NULL);
+ pthread_create(r-worker_thread, NULL, worker_thread_function, r);
+
+ return 1;
+}
+
+static void
+destroy_worker_thread(struct vaapi_recorder *r)
+{
+ pthread_mutex_lock(r-mutex);
+
+ /* Make sure the worker thread finishes */
+ r-destroying = 1;
+ pthread_cond_signal(r-input_cond);
+
+ pthread_mutex_unlock(r-mutex);
+
+ pthread_join(r-worker_thread, NULL);
+
+ pthread_mutex_destroy(r-mutex);
+ pthread_cond_destroy(r-input_cond);
+}
+
struct vaapi_recorder *
vaapi_recorder_create(int drm_fd, int width, int height, const char
*filename)
{
@@ -904,9 +946,12 @@ vaapi_recorder_create(int drm_fd, int width, int height,
const char *filename)
flags = O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC;
r-output_fd = open(filename, flags, 0644);
- if (r-output_fd 0)
+ if (setup_worker_thread(r) 0)
goto err_free;
+ if (r-output_fd 0)
+ goto err_thread;
+
r-va_dpy = vaGetDisplayDRM(drm_fd);
if (!r-va_dpy) {
weston_log(failed to create VA display\n);
@@ -936,6 +981,8 @@ err_va_dpy:
vaTerminate(r-va_dpy);
err_fd:
close(r-output_fd);
+err_thread:
+ destroy_worker_thread(r);
err_free:
free(r);
@@ -945,6 +992,8 @@ err_free:
void
vaapi_recorder_destroy(struct vaapi_recorder *r)
{
+ destroy_worker_thread(r);
+
encoder_destroy(r);
vpp_destroy(r);
@@ -1033,20 +1082,22 @@ convert_rgb_to_yuv(struct vaapi_recorder *r,
VASurfaceID rgb_surface)
return status;
}
-void
-vaapi_recorder_frame(struct vaapi_recorder *r, int prime_fd,
- int stride)
+static void
+recorder_frame(struct vaapi_recorder *r)
{
VASurfaceID rgb_surface;
VAStatus status;
- status = create_surface_from_fd(r, prime_fd, stride, rgb_surface);
+ status = create_surface_from_fd(r, r-input.prime_fd,
+ r-input.stride, rgb_surface);
if (status != VA_STATUS_SUCCESS) {
weston_log([libva recorder]
failed to create surface from bo\n);
return;
}
+ close(r-input.prime_fd);
+
status = convert_rgb_to_yuv(r, rgb_surface);
if (status != VA_STATUS_SUCCESS) {
weston_log([libva recorder]
@@ -1059,4 +1110,44 @@ vaapi_recorder_frame(struct vaapi_recorder *r, int
prime_fd,
vaDestroySurfaces(r-va_dpy, rgb_surface, 1);
}
+static void *
Re: [PATCH weston 1/4] input: Use new wl_resource_for_each for sending updated seat caps
On Fri, Sep 06, 2013 at 05:48:19PM +0100, Rob Bradford wrote:
From: Rob Bradford r...@linux.intel.com
Thanks, applied, much nicer with the macro.
Kristian
---
src/input.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/input.c b/src/input.c
index aa40b4e..e6d074e 100644
--- a/src/input.c
+++ b/src/input.c
@@ -398,8 +398,8 @@ weston_touch_destroy(struct weston_touch *touch)
static void
seat_send_updated_caps(struct weston_seat *seat)
{
- struct wl_list *link;
enum wl_seat_capability caps = 0;
+ struct wl_resource *resource;
if (seat-pointer)
caps |= WL_SEAT_CAPABILITY_POINTER;
@@ -408,9 +408,8 @@ seat_send_updated_caps(struct weston_seat *seat)
if (seat-touch)
caps |= WL_SEAT_CAPABILITY_TOUCH;
- for (link = seat-base_resource_list.next;
- link != seat-base_resource_list; link = link-next) {
- wl_seat_send_capabilities(wl_resource_from_link(link), caps);
+ wl_resource_for_each(resource, seat-base_resource_list) {
+ wl_seat_send_capabilities(resource, caps);
}
}
--
1.8.3.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v2] compositor: reset surface's resource field on resource destruction
with the surface ref-count feature a surface may live on after its resource was destroyed. set it to NULL in that case, so that code like find_resource_for_surface() in input.c will act accordingly. --- src/compositor.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/compositor.c b/src/compositor.c index 88df279..a79f911 100644 --- a/src/compositor.c +++ b/src/compositor.c @@ -1071,6 +1071,9 @@ destroy_surface(struct wl_resource *resource) struct weston_surface *surface = wl_resource_get_user_data(resource); weston_surface_destroy(surface); + /* the surface may live so make sure nothing thinks +it still has a resourc*e */ + surface-resource = NULL; } static void -- 1.8.4 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] eglut_wayland: Update to the Wayland 1.0 API
On 09/11/2013 08:33 PM, Neil Roberts wrote:
This fixes build errors with the eglut_wayland backend. In particular
it now uses the wl_registry and the new main loop mechanism from the
Wayland 1.0 API.
---
FYI, I've posted a patch to mesa-dev few weeks ago too, but yours seems
better, since mine didn't include polling stuff, but es2gears did
actually work.
http://lists.freedesktop.org/archives/mesa-dev/2013-August/043858.html
src/egl/eglut/eglut_wayland.c | 108
--
1 file changed, 94 insertions(+), 14 deletions(-)
diff --git a/src/egl/eglut/eglut_wayland.c b/src/egl/eglut/eglut_wayland.c
index 61207d2..25a51bc 100644
--- a/src/egl/eglut/eglut_wayland.c
+++ b/src/egl/eglut/eglut_wayland.c
@@ -1,6 +1,10 @@
#include wayland-client.h
#include wayland-egl.h
+#include poll.h
+#include errno.h
+#include string.h
+
#include eglutint.h
struct display {
@@ -20,42 +24,74 @@ static struct display display = {0, };
static struct window window = {0, };
static void
-display_handle_global(struct wl_display *display, uint32_t id,
- const char *interface, uint32_t version, void *data)
+registry_handle_global(void *data, struct wl_registry *registry, uint32_t id,
+ const char *interface, uint32_t version)
{
struct display *d = data;
if (strcmp(interface, wl_compositor) == 0) {
d-compositor =
- wl_display_bind(display, id, wl_compositor_interface);
+ wl_registry_bind(registry, id, wl_compositor_interface, 1);
} else if (strcmp(interface, wl_shell) == 0) {
- d-shell = wl_display_bind(display, id, wl_shell_interface);
+ d-shell = wl_registry_bind(registry, id, wl_shell_interface, 1);
}
}
+static void
+registry_handle_global_remove(void *data, struct wl_registry *registry,
+ uint32_t name)
+{
+}
+
+static const struct wl_registry_listener registry_listener = {
+ registry_handle_global,
+ registry_handle_global_remove
+};
+
+static void
+sync_callback(void *data, struct wl_callback *callback, uint32_t serial)
+{
+ int *done = data;
+
+ *done = 1;
+ wl_callback_destroy(callback);
+}
+
+static const struct wl_callback_listener sync_listener = {
+ sync_callback
+};
+
static int
-event_mask_update(uint32_t mask, void *data)
+wayland_roundtrip(struct wl_display *display)
{
- struct display *d = data;
+ struct wl_callback *callback;
+ int done = 0, ret = 0;
- d-mask = mask;
+ callback = wl_display_sync(display);
+ wl_callback_add_listener(callback, sync_listener, done);
+ while (ret != -1 !done)
+ ret = wl_display_dispatch(display);
- return 0;
+ if (!done)
+ wl_callback_destroy(callback);
+
+ return ret;
}
void
_eglutNativeInitDisplay(void)
{
+ struct wl_registry *registry;
+
_eglut-native_dpy = display.display = wl_display_connect(NULL);
if (!_eglut-native_dpy)
_eglutFatal(failed to initialize native display);
- wl_display_add_global_listener(_eglut-native_dpy,
- display_handle_global, display);
-
- wl_display_get_fd(_eglut-native_dpy, event_mask_update, display);
- wl_display_iterate(_eglut-native_dpy, WL_DISPLAY_READABLE);
+ registry = wl_display_get_registry(_eglut-native_dpy);
+ wl_registry_add_listener(registry, registry_listener, display);
+ wayland_roundtrip(_eglut-native_dpy);
+ wl_registry_destroy(registry);
_eglut-surface_type = EGL_WINDOW_BIT;
}
@@ -124,12 +160,56 @@ draw(void *data, struct wl_callback *callback, uint32_t
time)
void
_eglutNativeEventLoop(void)
{
+ struct pollfd pollfd;
+ int ret;
+
draw(window, NULL, 0);
+ pollfd.fd = wl_display_get_fd(display.display);
+ pollfd.events = POLLIN;
+ pollfd.revents = 0;
+
while (1) {
- wl_display_iterate(display.display, display.mask);
+ wl_display_dispatch_pending(display.display);
if (_eglut-idle_cb)
_eglut-idle_cb();
+
+ ret = wl_display_flush(display.display);
+ if (ret 0 errno == EAGAIN)
+ pollfd.events |= POLLOUT;
+ else if (ret 0)
+ break;
+
+ if (poll(pollfd, 1, _eglut-redisplay ? 0 : -1) == -1)
+ break;
+
+ if (pollfd.revents (POLLERR | POLLHUP))
+ break;
+
+ if (pollfd.revents POLLIN) {
+ ret = wl_display_dispatch(display.display);
+ if (ret == -1)
+break;
+ }
+
+ if (pollfd.revents POLLOUT) {
+ ret = wl_display_flush(display.display);
+ if (ret == 0)
+pollfd.events = ~POLLOUT;
+ else if (ret == -1 errno != EAGAIN)
+break;
+ }
+
+ if (_eglut-redisplay) {
+ struct eglut_window *win = _eglut-current;
+
+ _eglut-redisplay = 0;
+
+ if (win-display_cb)
+win-display_cb();
+
+
Re: [PATCH weston 1/5] tests: always build tests
On Wed, Sep 11, 2013 at 03:58:07PM +1000, Peter Hutterer wrote: check_PROGRAMS and friends are only built during make check. Which is a great way of introducing compiler errors in tests. Always build them, TESTS defines what's being run during make check. Yay, I like that. I thought you had to use check_* with for make check, but of course it's TESTS that matter. thanks, Kristian --- tests/Makefile.am | 12 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index 82bf630..398a275 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -29,18 +29,14 @@ clean-local: export abs_builddir noinst_LTLIBRARIES = \ - $(weston_test) + $(weston_test) \ + $(module_tests) noinst_PROGRAMS =\ $(setbacklight) \ - matrix-test - -check_LTLIBRARIES = \ - $(module_tests) - -check_PROGRAMS = \ $(shared_tests) \ - $(weston_tests) + $(weston_tests) \ + matrix-test AM_CFLAGS = $(GCC_CFLAGS) AM_CPPFLAGS =\ -- 1.8.3.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 2/5] tests: use variable for test name in weston-tests-env
On Wed, Sep 11, 2013 at 03:58:08PM +1000, Peter Hutterer wrote:
Slightly more readable and makes it easier to switch to use $2 for something
in the future (if that's ever needed).
Yup, that looks better.
Kristian
---
tests/weston-tests-env | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/tests/weston-tests-env b/tests/weston-tests-env
index 2e5fa95..b732250 100755
--- a/tests/weston-tests-env
+++ b/tests/weston-tests-env
@@ -1,5 +1,12 @@
#!/bin/bash
+TESTNAME=$1
+
+if test -z $TESTNAME; then
+ echo usage: $(basename $0) test name
+ exit 1;
+fi
+
WESTON=$abs_builddir/../src/weston
LOGDIR=$abs_builddir/logs
@@ -18,17 +25,17 @@ else
BACKEND=$abs_builddir/../src/.libs/wayland-backend.so
fi
-case $1 in
+case $TESTNAME in
*.la|*.so)
$WESTON --backend=$BACKEND \
- --socket=test-$(basename $1) \
- --modules=$abs_builddir/.libs/${1/.la/.so},xwayland.so \
+ --socket=test-$(basename $TESTNAME) \
+
--modules=$abs_builddir/.libs/${TESTNAME/.la/.so},xwayland.so \
--log=$SERVERLOG \
$OUTLOG
;;
*)
- WESTON_TEST_CLIENT_PATH=$abs_builddir/$1 $WESTON \
- --socket=test-$(basename $1) \
+ WESTON_TEST_CLIENT_PATH=$abs_builddir/$TESTNAME $WESTON \
+ --socket=test-$(basename $TESTNAME) \
--backend=$BACKEND \
--log=$SERVERLOG \
--modules=$abs_builddir/.libs/weston-test.so,xwayland.so \
--
1.8.3.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 5/5] tests: list available tests if an invalid test name is given
On Wed, Sep 11, 2013 at 04:08:47PM +1000, Peter Hutterer wrote:
---
tests/weston-test-runner.c | 1 +
1 file changed, 1 insertion(+)
All patches committed, thanks.
Kristian
diff --git a/tests/weston-test-runner.c b/tests/weston-test-runner.c
index fefb93b..ed5baf0 100644
--- a/tests/weston-test-runner.c
+++ b/tests/weston-test-runner.c
@@ -82,6 +82,7 @@ int main(int argc, char *argv[])
t = find_test(argv[1]);
if (t == NULL) {
fprintf(stderr, unknown test: \%s\\n, argv[1]);
+ list_tests();
exit(EXIT_FAILURE);
}
--
1.8.3.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 1/5] tests: always build tests
On Wed, Sep 11, 2013 at 02:38:55PM +0200, sardemff7+wayl...@sardemff7.net wrote: On 11/09/2013 11:12, Sam Spilsbury wrote: Quick thought: there's also an important psychological effect to building the tests on a standard make because it promotes them to the same importance as the rest of your code. They become less of an afterthought and it promotes greater care around how people design the tests (eg, making the tests clean, making sure they run quickly), as well as how the rest of the codebase interacts with the tests. We observed a similar effect at Canonical between the projects which had test building on by default as opposed to those that did not. Then we should definitely fix users (developers) and their workflow, not some arbitrary “problem”, as I said already. It all depends on whether or not the tests are there as a basic safety line for managing releases or whether or not tests are used as a tool to iterate and improve quality. In the latter case, building them by default is a very sensible decision indeed. Not at all. They should be *run* by default in this case, not just be built. If their point is to check the code, they must do that, not just build against some headers. See the end of this email. I agree with the sentiment here, but I think Peters patch is a pragmatic step towards that goal. Kristian ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] xwm: place transient windows at the right position
On Wed, Sep 11, 2013 at 03:41:58PM +0200, Giulio Camuffo wrote:
---
src/xwayland/window-manager.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
That makes sense, thanks.
Kristian
diff --git a/src/xwayland/window-manager.c b/src/xwayland/window-manager.c
index a8b949b..4c7256e 100644
--- a/src/xwayland/window-manager.c
+++ b/src/xwayland/window-manager.c
@@ -967,7 +967,7 @@ weston_wm_handle_property_notify(struct weston_wm *wm,
xcb_generic_event_t *even
static void
weston_wm_window_create(struct weston_wm *wm,
- xcb_window_t id, int width, int height, int override)
+ xcb_window_t id, int width, int height, int x, int y,
int override)
{
struct weston_wm_window *window;
uint32_t values[1];
@@ -991,6 +991,8 @@ weston_wm_window_create(struct weston_wm *wm,
window-override_redirect = override;
window-width = width;
window-height = height;
+ window-x = x;
+ window-y = y;
geometry_reply = xcb_get_geometry_reply(wm-conn, geometry_cookie,
NULL);
/* technically we should use XRender and check the visual format's
@@ -1026,6 +1028,7 @@ weston_wm_handle_create_notify(struct weston_wm *wm,
xcb_generic_event_t *event)
weston_wm_window_create(wm, create_notify-window,
create_notify-width, create_notify-height,
+ create_notify-x, create_notify-y,
create_notify-override_redirect);
}
@@ -1062,6 +1065,7 @@ weston_wm_handle_reparent_notify(struct weston_wm *wm,
xcb_generic_event_t *even
if (reparent_notify-parent == wm-screen-root) {
weston_wm_window_create(wm, reparent_notify-window, 10, 10,
+ reparent_notify-x, reparent_notify-y,
reparent_notify-override_redirect);
} else if (!our_resource(wm, reparent_notify-parent)) {
window = hash_table_lookup(wm-window_hash,
@@ -1916,7 +1920,7 @@ xserver_map_shell_surface(struct weston_wm *wm,
WL_SHELL_SURFACE_FULLSCREEN_METHOD_DEFAULT,
0, NULL);
return;
- } else if (!window-override_redirect) {
+ } else if (!window-override_redirect !window-transient_for) {
shell_interface-set_toplevel(window-shsurf);
return;
} else {
--
1.8.4
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v2] compositor: reset surface's resource field on resource destruction
Sure, thanks. 2013/9/11 Kristian Høgsberg hoegsb...@gmail.com On Wed, Sep 11, 2013 at 08:54:13PM +0200, Giulio Camuffo wrote: with the surface ref-count feature a surface may live on after its resource was destroyed. set it to NULL in that case, so that code like find_resource_for_surface() in input.c will act accordingly. That makes sense, applied. When you send out a v2 patch can you add a brief comment about what changed below the ---? --- (that is, here) Kristian src/compositor.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/compositor.c b/src/compositor.c index 88df279..a79f911 100644 --- a/src/compositor.c +++ b/src/compositor.c @@ -1071,6 +1071,9 @@ destroy_surface(struct wl_resource *resource) struct weston_surface *surface = wl_resource_get_user_data(resource); weston_surface_destroy(surface); + /* the surface may live so make sure nothing thinks + it still has a resourc*e */ + surface-resource = NULL; } static void -- 1.8.4 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v2] compositor: reset surface's resource field on resource destruction
On Wed, Sep 11, 2013 at 08:54:13PM +0200, Giulio Camuffo wrote: with the surface ref-count feature a surface may live on after its resource was destroyed. set it to NULL in that case, so that code like find_resource_for_surface() in input.c will act accordingly. That makes sense, applied. When you send out a v2 patch can you add a brief comment about what changed below the ---? --- (that is, here) Kristian src/compositor.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/compositor.c b/src/compositor.c index 88df279..a79f911 100644 --- a/src/compositor.c +++ b/src/compositor.c @@ -1071,6 +1071,9 @@ destroy_surface(struct wl_resource *resource) struct weston_surface *surface = wl_resource_get_user_data(resource); weston_surface_destroy(surface); + /* the surface may live so make sure nothing thinks + it still has a resourc*e */ + surface-resource = NULL; } static void -- 1.8.4 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 0/4] Add support for eglSwapInterval
On Wed, Sep 11, 2013 at 07:28:30PM +0100, Neil Roberts wrote: Here are some patches to add support for eglSwapInterval in Mesa's Wayland platform. The first two patches are for Mesa to actually implement it. The third patch is a minor tweak to Weston which is needed to get it to send the buffer release events at the correct time. The fourth patch is to mesa-demos to update eglut to work with the latest Wayland. The es2gears_wayland example can then be used to test the swap interval. I'll also attach a bodged version of the simple-egl example which modifies eglSwapInterval every 5 seconds and displays the frame rate. This looks good, I've pushed patches 1, 2 and 4, as for 3 see my reply to you and Daniel. Kristian ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] client: Fix handling display-reader_count if poll fails
On Wed, Sep 11, 2013 at 07:21:17PM +0100, Neil Roberts wrote:
In wl_display_dispatch_queue, if poll fails then it would previously
return immediately and leak a reference in display-reader_count. Then
if the application ignores the error and tries to read again it will
block forever. This can happen for example if the poll fails with
EINTR which the application might consider to be a recoverable error.
This patch just makes it decrement the reader count when poll fails.
Oh wow, tricky. Yes, that's the right idea, but we need to call
wl_display_cancel_read() instead of just decrementing to make sure we
grab the lock when we decrement and wake up sleeping readers in case
we were the last reader.
Kristian
---
src/wayland-client.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/wayland-client.c b/src/wayland-client.c
index 48f06c7..576a773 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -1224,8 +1224,10 @@ wl_display_dispatch_queue(struct wl_display *display,
pfd[0].fd = display-fd;
pfd[0].events = POLLIN;
- if (poll(pfd, 1, -1) == -1)
+ if (poll(pfd, 1, -1) == -1) {
+ display-reader_count--;
return -1;
+ }
pthread_mutex_lock(display-mutex);
--
1.8.3.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] wayland: Add an extension to create wl_buffers from EGLImages
I want to add the use case that I describe is not with subsurfaces. What I imagine is having a System compositor on card A (integrated), and a Session compositor on card A (integrated card/energy saving) or card B (dedicated card/max performance). The Session compositor wouldn't use subsurfaces, and would always give a full buffer to the System compositor (as weston running under weston). The Session compositor window is fullscreen and the System compositor optimizes performance by using the Session compositor buffer as a scanout buffer. If a fullscreen application runs inside the Session compositor, with your extension, the Session compositor would not need to copy the content of the window application while on card A/energy saving mode. Thanks to the extension, the fullscreen application will be used as a scanout buffer directly by the System compositor. But if we are on the dedicated card (max performance mode), we need a copy because the clients use tiling: your extension should fail (or do a copy). Axel Davy System compositor: set up wl_drm to import buffers on card A Session compositor: Detects it is on card B and not the card of wl_drm. Creates a buffer without tiling and use wl_drm to share it with the System compositor. The session compositor creates a new wl_drm for its clients. The clients: see they are on card B that is the card of the wl_drm they see. They create a buffer with tiling since it is better performance wise, and share it with the Session compositor. Without your extension, the session compositor has to copy the contents of the clients (with card B) in its own buffer (readable by card A and B) and commit it to the System compositor. Card A only sees a buffer without tiling, even if the clients used tiling. Your extension avoid the copy, and the tiling mode is not changed (it would need a copy). Since A doesn't understand the tiling mode of B, the buffers will not be displayed correctly. Prime support isn't implemented in Mesa yet (It'll use render-nodes). I suggest you design your extension so it can fail if the texture cannot be used on the card of the System compositor. When Prime support would be ready, it will be needed to check if the card described by wl_drm of the System compositor and the Session compositor is the same than the one used by the session compositor. I assume a variable would be added to the dri2_dpy structure to tell if it can use tiling or not when creating buffers, and you would just need to check it. Axel Davy Le 09/09/2013 20:39, Neil Roberts a écrit : Is this problem specific to the extension or is it a general problem? Would there not be the same issue if the session compositor wasn't using the extension but was creating textures from the client surfaces instead? Presumably if cards A and B don't share a tiling mode then it won't be possible for card B to make a texture out of buffers created on card A either. My assumption was that if you have successfully created an EGLImage out of the client's surface then it is a valid buffer for that EGL context. Perhaps this situation should be detected in eglCreateImage rather than in eglCreateWaylandBufferFromImage. I apologise in advance if I've misunderstood the problem. Regards, - Neil Axel Davy axel.d...@ens.fr writes: I think there is a problem with tiling handling in your patch. Thinks of a session compositor running on an other graphic card for example. Main compositor: card A session compositor: card B. buffer is shared between A and B and when creating it, tiling was disabled (because A and B share no tiling mode) clients in the session compositor: card B. buffer is created on B, when creating it, tiling was enabled. In this configuration, your function should fail or do a copy (because A won't recognize the tiling mode of B) Axel Davy - Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] wayland: Add an extension to create wl_buffers from EGLImages
System compositor: set up wl_drm to import buffers on card A Session compositor: Detects it is on card B and not the card of wl_drm. Creates a buffer without tiling and use wl_drm to share it with the System compositor. The session compositor creates a new wl_drm for its clients. The clients: see they are on card B that is the card of the wl_drm they see. They create a buffer with tiling since it is better performance wise, and share it with the Session compositor. Without your extension, the session compositor has to copy the contents of the clients (with card B) in its own buffer (readable by card A and B) and commit it to the System compositor. Card A only sees a buffer without tiling, even if the clients used tiling. Your extension avoid the copy, and the tiling mode is not changed (it would need a copy). Since A doesn't understand the tiling mode of B, the buffers will not be displayed correctly. Prime support isn't implemented in Mesa yet (It'll use render-nodes). I suggest you design your extension so it can fail if the texture cannot be used on the card of the System compositor. When Prime support would be ready, it will be needed to check if the card described by wl_drm of the System compositor and the Session compositor is the same than the one used by the session compositor. I assume a variable would be added to the dri2_dpy structure to tell if it can use tiling or not when creating buffers, and you would just need to check it. Axel Davy Le 09/09/2013 20:39, Neil Roberts a écrit : Is this problem specific to the extension or is it a general problem? Would there not be the same issue if the session compositor wasn't using the extension but was creating textures from the client surfaces instead? Presumably if cards A and B don't share a tiling mode then it won't be possible for card B to make a texture out of buffers created on card A either. My assumption was that if you have successfully created an EGLImage out of the client's surface then it is a valid buffer for that EGL context. Perhaps this situation should be detected in eglCreateImage rather than in eglCreateWaylandBufferFromImage. I apologise in advance if I've misunderstood the problem. Regards, - Neil Axel Davy axel.d...@ens.fr writes: I think there is a problem with tiling handling in your patch. Thinks of a session compositor running on an other graphic card for example. Main compositor: card A session compositor: card B. buffer is shared between A and B and when creating it, tiling was disabled (because A and B share no tiling mode) clients in the session compositor: card B. buffer is created on B, when creating it, tiling was enabled. In this configuration, your function should fail or do a copy (because A won't recognize the tiling mode of B) Axel Davy - Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 1/2] wayland: Add support for eglSwapInterval
I think you should too set the number of back buffers to 4 instead of 3.
It looks like if the compositor wants to use the buffers as framebuffers
and do a pageflip,
it uses 2 buffers at a time (one used for the frame displayed, and one
used for the pending pageFlip request). The third buffer sent is not
released and waits for the next frame. If there is only 3 buffers, the
client will be without free buffers and blocked in eglSwapBuffers.
I may be wrong, but I think the number of back buffers should be set to 4.
Axel Davy
Le 11/09/2013 20:28, Neil Roberts a écrit :
The Wayland EGL platform now respects the eglSwapInterval value. The value is
clamped to either 0 or 1 because it is difficult (and probably not useful) to
sync to more than 1 redraw.
The main change is that if the swap interval is 0 then it simply doesn't
install a frame callback so that the next time eglSwapBuffers is called it
won't delay.
The second change is that in get_back_bo instead of returning with an error if
all three buffers are locked it will now block in a dispatch loop so that it
can receive the buffer release events. The assumption is that the compositor
is unlikely to lock all three buffers so if we find that all the buffers are
locked then we are probably just rendering faster than we are processing the
release events. Therefore the release events should be available very early.
This also moves the vblank configuration defines from platform_x11.c to the
common egl_dri2.h header so they can be shared by both platforms.
---
src/egl/drivers/dri2/egl_dri2.h | 6 ++
src/egl/drivers/dri2/platform_wayland.c | 121 ++--
src/egl/drivers/dri2/platform_x11.c | 6 --
3 files changed, 107 insertions(+), 26 deletions(-)
diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
index fba5f81..849927b 100644
--- a/src/egl/drivers/dri2/egl_dri2.h
+++ b/src/egl/drivers/dri2/egl_dri2.h
@@ -221,6 +221,12 @@ struct dri2_egl_image
__DRIimage *dri_image;
};
+/* From xmlpool/options.h, user exposed so should be stable */
+#define DRI_CONF_VBLANK_NEVER 0
+#define DRI_CONF_VBLANK_DEF_INTERVAL_0 1
+#define DRI_CONF_VBLANK_DEF_INTERVAL_1 2
+#define DRI_CONF_VBLANK_ALWAYS_SYNC 3
+
/* standard typecasts */
_EGL_DRIVER_STANDARD_TYPECASTS(dri2_egl)
_EGL_DRIVER_TYPECAST(dri2_egl_image, _EGLImage, obj)
diff --git a/src/egl/drivers/dri2/platform_wayland.c
b/src/egl/drivers/dri2/platform_wayland.c
index ffc5959..83e7aab 100644
--- a/src/egl/drivers/dri2/platform_wayland.c
+++ b/src/egl/drivers/dri2/platform_wayland.c
@@ -180,8 +180,16 @@ dri2_create_window_surface(_EGLDriver *drv, _EGLDisplay
*disp,
_EGLConfig *conf, EGLNativeWindowType window,
const EGLint *attrib_list)
{
- return dri2_create_surface(drv, disp, EGL_WINDOW_BIT, conf,
+ struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
+ _EGLSurface *surf;
+
+ surf = dri2_create_surface(drv, disp, EGL_WINDOW_BIT, conf,
window, attrib_list);
+
+ if (surf != NULL)
+ drv-API.SwapInterval(drv, disp, surf, dri2_dpy-default_swap_interval);
+
+ return surf;
}
/**
@@ -261,24 +269,36 @@ get_back_bo(struct dri2_egl_surface *dri2_surf,
__DRIbuffer *buffer)
__DRIimage *image;
int i, name, pitch;
- /* There might be a buffer release already queued that wasn't processed */
- wl_display_dispatch_queue_pending(dri2_dpy-wl_dpy, dri2_dpy-wl_queue);
-
if (dri2_surf-back == NULL) {
- for (i = 0; i ARRAY_SIZE(dri2_surf-color_buffers); i++) {
- /* Get an unlocked buffer, preferrably one with a dri_buffer already
- * allocated. */
-if (dri2_surf-color_buffers[i].locked)
-continue;
- if (dri2_surf-back == NULL)
- dri2_surf-back = dri2_surf-color_buffers[i];
- else if (dri2_surf-back-dri_image == NULL)
- dri2_surf-back = dri2_surf-color_buffers[i];
+ /* There might be a buffer release already queued that wasn't processed
*/
+ wl_display_dispatch_queue_pending(dri2_dpy-wl_dpy, dri2_dpy-wl_queue);
+
+ while (1) {
+ for (i = 0; i ARRAY_SIZE(dri2_surf-color_buffers); i++) {
+/* Get an unlocked buffer, preferrably one with a dri_buffer
already
+ * allocated. */
+if (dri2_surf-color_buffers[i].locked)
+ continue;
+if (dri2_surf-back == NULL)
+ dri2_surf-back = dri2_surf-color_buffers[i];
+else if (dri2_surf-back-dri_image == NULL)
+ dri2_surf-back = dri2_surf-color_buffers[i];
+ }
+
+ if (dri2_surf-back)
+break;
+
+ /* If we make it here then here then all of the buffers are locked.
+ * It wouldn't make sense for the compositor to keep all three
+ * buffers so it must mean that we are rendering too fast without
+ * getting a chance
Re: [PATCH] Revert compositor: Queue buffer.release instead of sending immediately
Hi, On 11 September 2013 17:15, Kristian Høgsberg hoegsb...@gmail.com wrote: On Wed, Sep 11, 2013 at 02:54:47PM -0400, Daniel Stone wrote: Yeah, this is good to me. Media pipelines also really want release events ASAP, so they can reuse hardware-decoder buffers. The problem is that this will introduce an immediate event back to the client after it's done rendering. The goal is to be able to have the client render receive the frame event and input, update its state and render the next frame, attach and commit and then sleep until the next frame event. Sending out the release event immediately will wake it up one more time. Would it work it make mesa send a wl_display.sync request following the commit if swapinterval is 0? Ugh ... given that release isn't (even close to) guaranteed to be generated instantly, that means you'd just have to (periodically) spam the server with syncs until you get your release. The main case I'm thinking of is external media decode engines, which (as with every non-APU access on ARM) require physically-contiguous memory. If you have large enough input and output resolutions, you end up with very few buffers and so recycling them quickly becomes pretty imperative in order to keep your frame rate up. If you don't deliver releases quickly enough, sometimes you end up losing frames as the decoder gets totally starved. Surely you'd have the same issue on 3D rendering with complex scenes (which, given the lamentable state of memory bandwidth on some devices, can equate to 'uses texturing') too, unless (at least) triple-buffering ... ? Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 1/5] tests: always build tests
On Wed, Sep 11, 2013 at 02:38:55PM +0200, sardemff7+wayl...@sardemff7.net wrote: On 11/09/2013 11:12, Sam Spilsbury wrote: Quick thought: there's also an important psychological effect to building the tests on a standard make because it promotes them to the same importance as the rest of your code. They become less of an afterthought and it promotes greater care around how people design the tests (eg, making the tests clean, making sure they run quickly), as well as how the rest of the codebase interacts with the tests. We observed a similar effect at Canonical between the projects which had test building on by default as opposed to those that did not. Then we should definitely fix users (developers) and their workflow, not some arbitrary “problem”, as I said already. arbitrary: Based on random choice or personal whim, rather than any reason or system. libevdev, libwacom, xserver are three projects I know from the top of my head that use this approach, and it has paid off, at last to some degree. xf86-input-wacom doesn't, and right now the tests won't build. it's not an arbitrary 'problem'. It has a distinct set of reasons, even if you can just attribute it to developer lazyness. It all depends on whether or not the tests are there as a basic safety line for managing releases or whether or not tests are used as a tool to iterate and improve quality. In the latter case, building them by default is a very sensible decision indeed. Not at all. They should be *run* by default in this case, not just be built. If their point is to check the code, they must do that, not just build against some headers. See the end of this email. these are not mutually exclusive, and no-one has claimed that because they are building by default means we don't have to run them. Sam (and I) argue that if you always build them you're increasing the likelyhood of someone actually running them. On Wed, Sep 11, 2013 at 4:53 PM, Peter Hutterer wrote: from my experience, every project I've worked on that has a test suite needed this patch eventually, there's always a way to forget to run make check and suddenly you find out that it's been broken for months. (this is largely because test suites have a tendency to become outdated and useless, but...) Again, users need a “patch”. I’ll add a bit on that at the end of this email. how so? TESTS defines what's run during make check. check_* defines what's built, the two are related but not the same. Because building tests means you need their *dependencies*, which should definitely not be needed if you do not want to run them. then make those dependendencies optional? e.g. libevdev will simply print a warning if you don't have check installed at configure time. you can build it, but you won't be able to build (and thus run) the tests. I know the principle, I'm claiming that without this, tests will eventually break unless you find a way to force everyone to run make check. Which, coincidentally, wastes maintainer time too if you get a patch that's fine but breaks make check and you have to get through another revision. So, you have tests that build. Fine. Now what? If nobody run them, they are useless. The best way to force them is the vcs. Just add a git hook that runs them on commit or push, are you are sure you repo will never break. But you should teach people to run them, not force them to do so. again, not mutually exclusive. you can force people to build them, and you should still teach people to run them. having tests fail the build merely elevates their presence. A good example (for me) is Cairo. Their tests are noinst_* already (side note: we have to patch that in Exherbo to avoid circular dependencies on fresh installs). They are known to be broken for ages, but they build fine, sure! not building them by default would have changed this how? at last, if someone now comes along and wants to fix them, they don't have to trudge through months or years of API changes but can focus on the actual test part. Having tests built by default is not a bad idea, it is a bad fix. The developer *will* think “Hey, tests build, everything’s ok!” while his commit just broke them. if it builds it works is a mindset that no amount of tests will be able to fix, and, quite frankly, I'd worry more about this mindset in the main compositor code than in some tests that don't cover much code anyway (atm). If you want to force tests build or run, you can tweak rules a bit: if RUN_TESTS_BY_DEFAULT ifneq ($(ALREADY_INSIDE),yes) all-local: $(MAKE) ALREADY_INSIDE=yes check endif else if BUILD_TESTS_BY_DEFAULT ifneq ($(ALREADY_INSIDE),yes) all-local: $(MAKE) TESTS= ALREADY_INSIDE=yes check endif endif endif It is both more explicit and saner, imo. running make check after every compile is a sure way to turn off developers... As a last note: if tests do not introduce new dependencies *and*
