Discussion:
[RFC Weston 00/10] Sub-surfaces v2
(too old to reply)
Pekka Paalanen
2013-02-22 15:07:44 UTC
Permalink
Hi all,

this is a new spin of the sub-surfaces Wayland protocol
extension, v2. The v1 was announced in:
http://lists.freedesktop.org/archives/wayland-devel/2012-December/006844.html

Earlier we have already landed some 14 patches of toytoolkit
restructuring, that was done after v1.

The major changes in v2 are:

- Moved protocol from Wayland to Weston. It is easier to handle
in one place, and will be moved into Wayland core once it is
deemed good enough.

- Protocol documentation is updated. The following requests have
been added to the protocol: wl_subcompositor.destroy,
wl_subsurface.destroy, and most importantly
wl_subsurface.set_commit_mode.

- Clients can now choose between two commit behaviours for
sub-surfaces: parent-cached and independent. These are
explained in the protocol file.

- A totally new demo client presenting sub-surfaces, including
Cairo-image rendered window with a pure EGL/GL widget in a
sub-surface, running independently, but still without glitches
on resize (sans bugs).
Loading Image...

The biggest improvement over v1 is that we have some thought-out
commit behaviours. It is possible to resize a window so that all
surfaces stay in sync on screen, and it is also possible to have
sub-surfaces running on their own (i.e. without commit the
parent surface, too).


Open issues:

- Does this commit behaviour satisfy all needs?

- Input events still very much unexplored. The demo just punts
them.

- Clipping and scaling of surface content - this will most
likely be a new orthogonal extension altogether, so not really
relevant here.

- What should wl_subsurface.destroy do? Nothing, or reset the
wl_surface's role? Currently it resets the role, since it was
easy to implement.

- Should sub-surface nesting be allowed? I.e. creation of
sub-sub-surfaces etc. What would be semantics be? I have not
really thought about that. Input dispatching within clients
may be a problem.

- wl_shell and other protocols interact with sub-surfaces, and
all interactions have not been resolved yet.

If I forgot any other design issues, let me know, please.


To do, bugs:

- destroying the wl_surface of a wl_subsurface will prevent
destroying the wl_subsurface, fix this, and adjust the
protocol description accordingly

- double-buffering of sub-surface z-order changes

- a demo client with window decorations stitched from 4
non-overlapping sub-surfaces

- fix frame throttling in subsurfaces demo

- fix eglSwapInterval in Mesa (krh?)

- subsurfaces demo behaves strangely under the DRM backend of
Weston, debug and fix it. A timestamp issue?

- even with three buffers in a pool, subsurfaces may still run
out of free buffers. When that happens, "all buffers are held
by the server" error message appears, and sub-surfaces may be
out of sync. Related to frame throttling.

- glitched rendering of the GL widget in subsurfaces when Up or
Down key held pressed, might be related to frame throttling.

- fix full-surface alpha for surfaces that have sub-surfaces

- in subsurfaces demo, put the GL widget into its own thread, so
it is truely as concurrent as it can get.

- reimplement the "black surface" in shell as a sub-surface


The new code can be browsed in:
http://cgit.collabora.com/git/user/pq/weston.git/log/?h=subsurface-v2

Pekka Paalanen (10):
protocol: add sub-surfaces
compositor: introduce sub-surfaces
tests: add sub-surface protocol tests
compositor: add sub-surfaces to repaint list
shell: keyboard focus and restacking fixes for sub-surfaces
window: implement shm triple-buffering
window: create sub-surfaces
window: implement per-surface redraws
clients: add subsurfaces demo
window: prevent EGL sub-surface deadlock

clients/.gitignore | 3 +
clients/Makefile.am | 12 +
clients/subsurfaces.c | 791 ++++++++++++++++++++++++++++++++++++++++++++++++
clients/window.c | 343 ++++++++++++++++++---
clients/window.h | 14 +
configure.ac | 3 +
protocol/subsurface.xml | 214 +++++++++++++
src/.gitignore | 3 +
src/Makefile.am | 4 +
src/compositor.c | 689 ++++++++++++++++++++++++++++++++++++++++-
src/compositor.h | 59 ++++
src/shell.c | 37 ++-
tests/.gitignore | 3 +
tests/Makefile.am | 9 +-
tests/subsurface-test.c | 327 ++++++++++++++++++++
15 files changed, 2437 insertions(+), 74 deletions(-)
create mode 100644 clients/subsurfaces.c
create mode 100644 protocol/subsurface.xml
create mode 100644 tests/subsurface-test.c


This patch series depends on further patches, so you cannot just
apply this series on top of Weston master. I did not include the
other patches, because they are waiting for a rewrite. Instead,
you can use my Weston branch subsurface-v2 which has everything
in place. You do not need any patches for Wayland, you can just
use master of Wayland upstream.

The following changes since commit a4575634f4a1f63430cfc4da5b7ab757f6c33c1c:

compositor: Rename lock and unlock signals to idle and wake (2013-02-21 21:12:45 -0500)

are available in the git repository at:

git://git.collabora.co.uk/git/user/pq/weston.git subsurface-v2

for you to fetch changes up to 2b3f13a2cd8ddadd87b5329e400fcddcb460a046:

window: prevent EGL sub-surface deadlock (2013-02-22 15:47:03 +0200)

When you build it, you should configure Weston with
--with-cairo-glesv2 regardless of whether your Cairo actually
has GLESv2. Otherwise the new subsurfaces demo might not be
built.


Thanks,
pq
--
1.7.12.4
Pekka Paalanen
2013-02-22 15:07:45 UTC
Permalink
Add protocol for sub-surfaces, wl_subcompositor as the global interface,
and wl_subsurface as the per-surface interface extension.

Changes in v2:

- Rewrite wl_subcompositor.get_subsurface description, and move mapping
and commit details into wl_subsurface description. Check the wording
in wl_subsurface.set_position description.

- Add wl_subsurface.set_commit_mode request, and document it, with the
commit_mode enum. Add bad_value error code for wl_subsurface.

- Moved the protocol into Weston repository so we can land it upstream
sooner for public exposure. It is to be moved into Wayland core later.

- Add destroy requests to both wl_subcompositor and wl_subsurface, and
document them. Experience has showed, that interfaces should always
have a destructor unless there is a good and future-proof reason to not
have it.

Signed-off-by: Pekka Paalanen <ppaalanen at gmail.com>
---
clients/.gitignore | 2 +
clients/Makefile.am | 4 +
clients/window.h | 1 +
protocol/subsurface.xml | 214 ++++++++++++++++++++++++++++++++++++++++++++++++
src/.gitignore | 3 +
src/Makefile.am | 4 +
src/compositor.h | 1 +
tests/.gitignore | 2 +
tests/Makefile.am | 4 +
9 files changed, 235 insertions(+)
create mode 100644 protocol/subsurface.xml

diff --git a/clients/.gitignore b/clients/.gitignore
index dcd4564..16088e8 100644
--- a/clients/.gitignore
+++ b/clients/.gitignore
@@ -20,6 +20,8 @@ simple-egl
simple-shm
simple-touch
smoke
+subsurface-client-protocol.h
+subsurface-protocol.c
tablet-shell-client-protocol.h
tablet-shell-protocol.c
text-client-protocol.h
diff --git a/clients/Makefile.am b/clients/Makefile.am
index 8c9bcd4..5f83acd 100644
--- a/clients/Makefile.am
+++ b/clients/Makefile.am
@@ -81,6 +81,8 @@ libtoytoolkit_la_SOURCES = \
window.h \
text-cursor-position-protocol.c \
text-cursor-position-client-protocol.h \
+ subsurface-protocol.c \
+ subsurface-client-protocol.h \
workspaces-protocol.c \
workspaces-client-protocol.h

@@ -185,6 +187,8 @@ BUILT_SOURCES = \
desktop-shell-protocol.c \
tablet-shell-client-protocol.h \
tablet-shell-protocol.c \
+ subsurface-client-protocol.h \
+ subsurface-protocol.c \
workspaces-client-protocol.h \
workspaces-protocol.c

diff --git a/clients/window.h b/clients/window.h
index c2946d8..815b3f1 100644
--- a/clients/window.h
+++ b/clients/window.h
@@ -27,6 +27,7 @@
#include <wayland-client.h>
#include <cairo.h>
#include "../shared/config-parser.h"
+#include "subsurface-client-protocol.h"

#define ARRAY_LENGTH(a) (sizeof (a) / sizeof (a)[0])

diff --git a/protocol/subsurface.xml b/protocol/subsurface.xml
new file mode 100644
index 0000000..1d148d9
--- /dev/null
+++ b/protocol/subsurface.xml
@@ -0,0 +1,214 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<protocol name="subsurface">
+
+ <copyright>
+ Copyright ? 2012-2013 Collabora, Ltd.
+
+ 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.
+ </copyright>
+
+ <interface name="wl_subcompositor" version="1">
+ <description summary="sub-surface compositing">
+ The global interface exposing sub-surface compositing capabilities.
+ A wl_surface, that has sub-surfaces associated, is called the
+ parent surface. Sub-surfaces cannot be nested.
+
+ A parent surface with its sub-surfaces forms a (compound) window.
+ For window management purposes, this set of wl_surface objects is
+ to be considered as a single window, and it should also behave as
+ such.
+
+ The aim of sub-surfaces is to offload some of the compositing work
+ within a window from clients to the compositor. A prime example is
+ a video player with decorations and video in separate wl_surface
+ objects. This should allow the compositor to pass YUV video buffer
+ processing to dedicated overlay hardware when possible.
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="unbind from the subcompositor interface">
+ Informs the server that the client will not be using this
+ protocol object anymore. This does not affect any other
+ objects, wl_subsurface objects included.
+ </description>
+ </request>
+
+ <enum name="error">
+ <entry name="bad_surface" value="0"
+ summary="the to-be sub-surface is invalid"/>
+ <entry name="bad_parent" value="1"
+ summary="the given parent is a sub-surface"/>
+ </enum>
+
+ <request name="get_subsurface">
+ <description summary="give a surface the role sub-surface">
+ Create a sub-surface interface for the given surface, and
+ associate it with the given parent surface. This turns a
+ plain wl_surface into a sub-surface.
+
+ The parent surface must not be a sub-surface itself.
+ The to-be sub-surface must not already have a dedicated
+ purpose, like any shell surface type, cursor image, drag icon,
+ or sub-surface. It also must not be a parent to further
+ sub-surfaces. Otherwise a protocol error is raised.
+ </description>
+
+ <arg name="id" type="new_id" interface="wl_subsurface"
+ summary="the new subsurface object id"/>
+ <arg name="surface" type="object" interface="wl_surface"
+ summary="the surface to be turned into a sub-surface"/>
+ <arg name="parent" type="object" interface="wl_surface"
+ summary="the parent surface"/>
+ </request>
+ </interface>
+
+ <interface name="wl_subsurface" version="1">
+ <description summary="sub-surface interface to a wl_surface">
+ An additional interface to a wl_surface object, which has been
+ made a sub-surface. A sub-surface has one parent surface.
+
+ A sub-surface becomes mapped, when a non-NULL wl_buffer is applied
+ and the parent surface is mapped. The order of which one happens
+ first is irrelevant. A sub-surface is hidden if the parent becomes
+ hidden, or if a NULL wl_buffer is applied.
+
+ The behaviour of wl_surface.commit request on a sub-surface depends
+ on the commit mode, set by wl_subsurface.set_commit_mode. The commit
+ mode affects operations with the wl_surface state of sub-surfaces.
+
+ If the commit mode is "parent-cached", wl_surface.commit on a
+ sub-surface will accumulate the committed state in a cache,
+ but the state will not be applied and hence will not change the
+ compositor output. The cached state is applied to the sub-surface when
+ wl_surface.commit is called on the parent surface, after the parent
+ surface's own state is applied. This ensures atomic updates of the
+ parent and all its sub-surfaces with "parent-cached" commit mode.
+ Applying the cached state will invalidate the cache, so further
+ parent surface commits do not (re-)apply old state.
+
+ If the commit mode is "independent", wl_surface.commit on a
+ sub-surface will apply the pending state without caching.
+ Calling wl_surface.commit on the parent surface has no effect on the
+ sub-surface state. This mode allows a sub-surface to be updated
+ on its own.
+
+ Sub-surfaces have also other kind of state, which is managed by
+ wl_subsurface requests, as opposed to wl_surface requests. This
+ state includes the sub-surface position relative to the parent
+ surface (wl_subsurface.set_position), and the stacking order of
+ the parent and its sub-surfaces (wl_subsurface.place_above and
+ .place_below). This state is applied on the parent surface's
+ commit, regardless of commit mode. As the exception, changes to
+ commit mode are applied immediately.
+
+ The wl_subsurface object must be destroyed before the wl_surface object
+ it was create for. Note, that destroying either object
+ takes effect immediately. If you need to synchronize the removal
+ of a sub-surface to the parent surface update, unmap the sub-surface
+ first by attaching a NULL wl_buffer, update parent, and then destroy
+ the sub-surface.
+
+ If the parent wl_surface object is destroyed, the sub-surface is
+ unmapped.
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="remove sub-surface interface">
+ The sub-surface interface is removed from the wl_surface object
+ that was turned into a sub-surface with
+ wl_subcompositor.get_subsurface request. The wl_surface's association
+ to the parent is deleted, and the wl_surface loses its role as
+ a sub-surface. The wl_surface is unmapped.
+ </description>
+ </request>
+
+ <enum name="error">
+ <entry name="bad_surface" value="0"
+ summary="wl_surface is not a sibling or the parent"/>
+ <entry name="bad_value" value="1"
+ summary="unknown enum parameter value"/>
+ </enum>
+
+ <request name="set_position">
+ <description summary="reposition the sub-surface">
+ This schedules a sub-surface position change.
+ The sub-surface will be moved so, that its origin (top-left
+ corner pixel) will be at the location x, y of the parent surface.
+
+ The next wl_surface.commit on the parent surface will reset
+ the sub-surface's position to the scheduled coordinates.
+
+ The initial position is 0, 0.
+ </description>
+
+ <arg name="x" type="int" summary="coordinate in the parent surface"/>
+ <arg name="y" type="int" summary="coordinate in the parent surface"/>
+ </request>
+
+ <request name="place_above">
+ <description summary="restack the sub-surface">
+ This sub-surface is taken from the stack, and put back just
+ above the reference surface, changing the z-order of the sub-surfaces.
+ The reference surface must be one of the sibling surfaces, or the
+ parent surface. Using any other surface, including this sub-surface,
+ will cause a protocol error.
+
+ The z-order is double-buffered state, and will be applied on the
+ next commit of the parent surface.
+ See wl_surface.commit and wl_subcompositor.get_subsurface.
+ </description>
+
+ <arg name="sibling" type="object" interface="wl_surface"
+ summary="the reference surface"/>
+ </request>
+
+ <request name="place_below">
+ <description summary="restack the sub-surface">
+ The sub-surface is placed just below of the reference surface.
+ See wl_subsurface.place_above.
+ </description>
+
+ <arg name="sibling" type="object" interface="wl_surface"
+ summary="the reference surface"/>
+ </request>
+
+ <enum name="commit_mode">
+ <entry name="parent_cached" value="1"
+ summary="sub-surface commit only caches state"/>
+ <entry name="independent" value="2"
+ summary="sub-surface commit applies state"/>
+ </enum>
+
+ <request name="set_commit_mode">
+ <description summary="change sub-surface commit behaviour">
+ Change how parent and sub-surface commits apply the pending
+ surface state. This request is effective immediately.
+ See the wl_subsurface description.
+
+ The default commit mode is parent-cached.
+ </description>
+
+ <arg name="mode" type="uint" summary="enum commit_mode"/>
+ </request>
+
+ </interface>
+</protocol>
diff --git a/src/.gitignore b/src/.gitignore
index 3c27953..c1392e1 100644
--- a/src/.gitignore
+++ b/src/.gitignore
@@ -17,3 +17,6 @@ workspaces-protocol.c
workspaces-server-protocol.h
input-method-protocol.c
input-method-server-protocol.h
+subsurface-server-protocol.h
+subsurface-protocol.c
+
diff --git a/src/Makefile.am b/src/Makefile.am
index 2c93a7b..bcc8795 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -34,6 +34,8 @@ weston_SOURCES = \
input-method-server-protocol.h \
workspaces-protocol.c \
workspaces-server-protocol.h \
+ subsurface-protocol.c \
+ subsurface-server-protocol.h \
bindings.c \
animation.c \
gl-renderer.h \
@@ -252,6 +254,8 @@ BUILT_SOURCES = \
input-method-server-protocol.h \
workspaces-server-protocol.h \
workspaces-protocol.c \
+ subsurface-server-protocol.h \
+ subsurface-protocol.c \
git-version.h

CLEANFILES = $(BUILT_SOURCES)
diff --git a/src/compositor.h b/src/compositor.h
index cd21c32..3d197ef 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -31,6 +31,7 @@
#include "version.h"
#include "matrix.h"
#include "config-parser.h"
+#include "subsurface-server-protocol.h"

#define ARRAY_LENGTH(a) (sizeof (a) / sizeof (a)[0])

diff --git a/tests/.gitignore b/tests/.gitignore
index 05bc024..fa19888 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -11,3 +11,5 @@ keyboard-test
event-test
button-test
xwayland-test
+subsurface-client-protocol.h
+subsurface-protocol.c
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 0f81458..9b9351a 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -60,6 +60,8 @@ weston_test_client_src = \
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)
weston_test_client_libs = \
$(SIMPLE_CLIENT_LIBS) \
@@ -109,6 +111,8 @@ endif
EXTRA_DIST = weston-tests-env

BUILT_SOURCES = \
+ subsurface-protocol.c \
+ subsurface-client-protocol.h \
wayland-test-protocol.c \
wayland-test-server-protocol.h \
wayland-test-client-protocol.h
--
1.7.12.4
Pekka Paalanen
2013-02-22 15:07:46 UTC
Permalink
This commit depends on the Wayland commit "protocol: add sub-surfaces".

Implement the basic protocol for sub-surfaces:
- expose wl_subcompositor global interface
- error checking on protocol calls
- associate a parent wl_surface to a sub-surface
- introduce the sub-surface role, which is exclusive
- a rudimentary implementation of the wl_subsurface interface
- proper surface transformation inheritance from parent to sub-surfaces
- two different modes of wl_surface.commit for sub-surfaces

Struct weston_subsurface is dynamically allocated. For sub-surfaces, it
is completely populated.

For parent surfaces, weston_subsurface acts only as a link for stacking
order purposes. The wl_resource is unused, parent_destroy_listener is
not registered, the transform is not linked, etc.

Sub-surfaces are not added directly into layers for display or input.
Instead, a following patch will hook them up via the sub-surface list
present in parent weston_surfaces. This way sub-surfaces are inherently
linked to the parent surface, and cannot be displayed unless the parent
is mapped, too. This also eases restacking, as only the parent will be
in a layer list.

Features still lacking are:
- double-buffering sub-surface z-order changes

Changes in v2:
- fix a bug in surface mapping: commit a sub-surface would cause the
main surface to never be mapped.
- remove debug printfs
- detect attempt of making a surface its own parent
- always zero-alloc weston_subsurface
- apply wl_subsurface.set_position in commit, not immediately
- add weston_surface_to_subsurface()
- implement sub-surface commit modes parent-cached and independent
- implement wl_subcompositor.destroy and wl_subsurface.destroy

Signed-off-by: Pekka Paalanen <ppaalanen at gmail.com>
---
src/compositor.c | 619 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
src/compositor.h | 55 +++++
2 files changed, 672 insertions(+), 2 deletions(-)

diff --git a/src/compositor.c b/src/compositor.c
index ccabb95..9c14423 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -262,6 +262,9 @@ region_init_infinite(pixman_region32_t *region)
UINT32_MAX, UINT32_MAX);
}

+static struct weston_subsurface *
+weston_surface_to_subsurface(struct weston_surface *surface);
+
WL_EXPORT struct weston_surface *
weston_surface_create(struct weston_compositor *compositor)
{
@@ -316,6 +319,8 @@ weston_surface_create(struct weston_compositor *compositor)
region_init_infinite(&surface->pending.input);
wl_list_init(&surface->pending.frame_callback_list);

+ wl_list_init(&surface->subsurface_list);
+
return surface;
}

@@ -951,6 +956,8 @@ destroy_surface(struct wl_resource *resource)
struct weston_compositor *compositor = surface->compositor;
struct weston_frame_callback *cb, *next;

+ assert(wl_list_empty(&surface->subsurface_list));
+
if (weston_surface_is_mapped(surface))
weston_surface_unmap(surface);

@@ -1421,9 +1428,8 @@ weston_surface_has_different_size(struct weston_surface *surface,
}

static void
-surface_commit(struct wl_client *client, struct wl_resource *resource)
+weston_surface_commit(struct weston_surface *surface)
{
- struct weston_surface *surface = resource->data;
pixman_region32_t opaque;

if (surface->pending.sx || surface->pending.sy ||
@@ -1484,6 +1490,31 @@ surface_commit(struct wl_client *client, struct wl_resource *resource)
}

static void
+weston_subsurface_commit(struct weston_subsurface *sub);
+
+static void
+weston_subsurface_parent_commit(struct weston_subsurface *sub);
+
+static void
+surface_commit(struct wl_client *client, struct wl_resource *resource)
+{
+ struct weston_surface *surface = resource->data;
+ struct weston_subsurface *sub = weston_surface_to_subsurface(surface);
+
+ if (sub) {
+ weston_subsurface_commit(sub);
+ return;
+ }
+
+ weston_surface_commit(surface);
+
+ wl_list_for_each(sub, &surface->subsurface_list, parent_link) {
+ if (sub->surface != surface)
+ weston_subsurface_parent_commit(sub);
+ }
+}
+
+static void
surface_set_buffer_transform(struct wl_client *client,
struct wl_resource *resource, int transform)
{
@@ -1602,6 +1633,586 @@ static const struct wl_compositor_interface compositor_interface = {
};

static void
+weston_subsurface_commit_from_cache(struct weston_subsurface *sub)
+{
+ struct weston_surface *surface = sub->surface;
+ pixman_region32_t opaque;
+
+ if (sub->cached.sx || sub->cached.sy ||
+ weston_surface_has_different_size(surface,
+ sub->cached.buffer_ref.buffer,
+ sub->cached.buffer_transform))
+ weston_surface_geometry_dirty(surface);
+
+ /* wl_surface.set_buffer_rotation */
+ surface->buffer_transform = sub->cached.buffer_transform;
+
+ /* wl_surface.attach */
+ if (sub->cached.buffer_ref.buffer || sub->cached.remove_contents)
+ weston_surface_attach(surface, sub->cached.buffer_ref.buffer);
+
+ if (surface->buffer_ref.buffer && surface->configure)
+ surface->configure(surface, sub->cached.sx, sub->cached.sy);
+ sub->cached.sx = 0;
+ sub->cached.sy = 0;
+
+ /* wl_surface.damage */
+ pixman_region32_union(&surface->damage, &surface->damage,
+ &sub->cached.damage);
+ pixman_region32_intersect_rect(&surface->damage, &surface->damage,
+ 0, 0,
+ surface->geometry.width,
+ surface->geometry.height);
+ empty_region(&sub->cached.damage);
+
+ /* wl_surface.set_opaque_region */
+ pixman_region32_init_rect(&opaque, 0, 0,
+ surface->geometry.width,
+ surface->geometry.height);
+ pixman_region32_intersect(&opaque,
+ &opaque, &sub->cached.opaque);
+
+ if (!pixman_region32_equal(&opaque, &surface->opaque)) {
+ pixman_region32_copy(&surface->opaque, &opaque);
+ weston_surface_geometry_dirty(surface);
+ }
+
+ pixman_region32_fini(&opaque);
+
+ /* wl_surface.set_input_region */
+ pixman_region32_fini(&surface->input);
+ pixman_region32_init_rect(&surface->input, 0, 0,
+ surface->geometry.width,
+ surface->geometry.height);
+ pixman_region32_intersect(&surface->input,
+ &surface->input, &sub->cached.input);
+
+ /* wl_surface.frame */
+ wl_list_insert_list(&surface->frame_callback_list,
+ &sub->cached.frame_callback_list);
+ wl_list_init(&sub->cached.frame_callback_list);
+
+ weston_surface_schedule_repaint(surface);
+
+ sub->cached.has_data = 0;
+}
+
+static void
+weston_subsurface_commit_to_cache(struct weston_subsurface *sub)
+{
+ struct weston_surface *surface = sub->surface;
+
+ /*
+ * If this commit would cause the surface to move by the
+ * attach(dx, dy) parameters, the old damage region must be
+ * translated to correspond to the new surface coordinate system
+ * origin.
+ */
+ pixman_region32_translate(&sub->cached.damage,
+ -surface->pending.sx, -surface->pending.sy);
+ pixman_region32_union(&sub->cached.damage, &sub->cached.damage,
+ &surface->pending.damage);
+ empty_region(&surface->pending.damage);
+
+ if (surface->pending.buffer || surface->pending.remove_contents) {
+ sub->cached.remove_contents = surface->pending.remove_contents;
+ weston_buffer_reference(&sub->cached.buffer_ref,
+ surface->pending.buffer);
+ }
+ sub->cached.sx += surface->pending.sx;
+ sub->cached.sy += surface->pending.sy;
+ surface->pending.sx = 0;
+ surface->pending.sy = 0;
+
+ sub->cached.buffer_transform = surface->pending.buffer_transform;
+
+ pixman_region32_copy(&sub->cached.opaque, &surface->pending.opaque);
+
+ pixman_region32_copy(&sub->cached.input, &surface->pending.input);
+
+ wl_list_insert_list(&sub->cached.frame_callback_list,
+ &surface->pending.frame_callback_list);
+ wl_list_init(&surface->pending.frame_callback_list);
+
+ sub->cached.has_data = 1;
+}
+
+static void
+weston_subsurface_commit(struct weston_subsurface *sub)
+{
+ switch (sub->commit_mode) {
+ case WL_SUBSURFACE_COMMIT_MODE_PARENT_CACHED:
+ weston_subsurface_commit_to_cache(sub);
+ break;
+ case WL_SUBSURFACE_COMMIT_MODE_INDEPENDENT:
+ if (sub->cached.has_data) {
+ /* flush accumulated state from cache */
+ weston_subsurface_commit_to_cache(sub);
+ weston_subsurface_commit_from_cache(sub);
+ } else {
+ weston_surface_commit(sub->surface);
+ }
+ break;
+ default:
+ assert(!"bad weston_subsurface::commit_mode");
+ }
+}
+
+static void
+weston_subsurface_parent_commit(struct weston_subsurface *sub)
+{
+ switch (sub->commit_mode) {
+ case WL_SUBSURFACE_COMMIT_MODE_PARENT_CACHED:
+ weston_subsurface_commit_from_cache(sub);
+ break;
+ case WL_SUBSURFACE_COMMIT_MODE_INDEPENDENT:
+ /* no-op */
+ break;
+ default:
+ assert(!"bad weston_subsurface::commit_mode");
+ }
+
+ if (sub->position.set) {
+ weston_surface_set_position(sub->surface,
+ sub->position.x, sub->position.y);
+ sub->position.set = 0;
+ }
+}
+
+static void
+subsurface_configure(struct weston_surface *surface, int32_t dx, int32_t dy)
+{
+ struct weston_compositor *compositor = surface->compositor;
+
+ weston_surface_configure(surface,
+ surface->geometry.x + dx,
+ surface->geometry.y + dy,
+ weston_surface_buffer_width(surface),
+ weston_surface_buffer_height(surface));
+
+ /* No need to check parent mappedness, because if parent is not
+ * mapped, parent is not in a visible layer, so this sub-surface
+ * will not be drawn either.
+ */
+ if (!weston_surface_is_mapped(surface)) {
+ wl_list_init(&surface->layer_link);
+
+ /* Cannot call weston_surface_update_transform(),
+ * because that would call it also for the parent surface,
+ * which might not be mapped yet. That would lead to
+ * inconsistent state, where the window could never be
+ * mapped.
+ *
+ * Instead just assing any output, to make
+ * weston_surface_is_mapped() return true, so that when the
+ * parent surface does get mapped, this one will get
+ * included, too. See surface_list_add().
+ */
+ assert(!wl_list_empty(&compositor->output_list));
+ surface->output = container_of(compositor->output_list.next,
+ struct weston_output, link);
+ }
+}
+
+static struct weston_subsurface *
+weston_surface_to_subsurface(struct weston_surface *surface)
+{
+ if (surface->configure == subsurface_configure)
+ return surface->private;
+
+ return NULL;
+}
+
+static void
+subsurface_set_position(struct wl_client *client,
+ struct wl_resource *resource, int32_t x, int32_t y)
+{
+ struct weston_subsurface *sub = resource->data;
+
+ sub->position.x = x;
+ sub->position.y = y;
+ sub->position.set = 1;
+}
+
+static struct weston_subsurface *
+subsurface_from_surface(struct weston_surface *surface)
+{
+ struct weston_subsurface *sub;
+
+ sub = weston_surface_to_subsurface(surface);
+ if (sub)
+ return sub;
+
+ wl_list_for_each(sub, &surface->subsurface_list, parent_link)
+ if (sub->surface == surface)
+ return sub;
+
+ return NULL;
+}
+
+static struct weston_subsurface *
+subsurface_sibling_check(struct weston_subsurface *sub,
+ struct weston_surface *surface,
+ const char *request)
+{
+ struct weston_subsurface *sibling;
+
+ sibling = subsurface_from_surface(surface);
+
+ if (!sibling) {
+ wl_resource_post_error(&sub->resource,
+ WL_SUBSURFACE_ERROR_BAD_SURFACE,
+ "%s: wl_surface@%d is not a parent or sibling",
+ request, surface->surface.resource.object.id);
+ return NULL;
+ }
+
+ if (sibling->parent != sub->parent) {
+ wl_resource_post_error(&sub->resource,
+ WL_SUBSURFACE_ERROR_BAD_SURFACE,
+ "%s: wl_surface@%d has a different parent",
+ request, surface->surface.resource.object.id);
+ return NULL;
+ }
+
+ return sibling;
+}
+
+static void
+subsurface_place_above(struct wl_client *client,
+ struct wl_resource *resource,
+ struct wl_resource *sibling_resource)
+{
+ struct weston_subsurface *sub = resource->data;
+ struct weston_surface *surface = sibling_resource->data;
+ struct weston_subsurface *sibling;
+
+ sibling = subsurface_sibling_check(sub, surface, "place_above");
+ if (!sibling)
+ return;
+
+ /* TODO: make the list double-buffered, and apply it on parent commit */
+
+ wl_list_remove(&sub->parent_link);
+ wl_list_insert(sibling->parent_link.prev, &sub->parent_link);
+}
+
+static void
+subsurface_place_below(struct wl_client *client,
+ struct wl_resource *resource,
+ struct wl_resource *sibling_resource)
+{
+ struct weston_subsurface *sub = resource->data;
+ struct weston_surface *surface = sibling_resource->data;
+ struct weston_subsurface *sibling;
+
+ sibling = subsurface_sibling_check(sub, surface, "place_below");
+ if (!sibling)
+ return;
+
+ /* TODO: make the list double-buffered, and apply it on parent commit */
+
+ wl_list_remove(&sub->parent_link);
+ wl_list_insert(&sibling->parent_link, &sub->parent_link);
+}
+
+static void
+subsurface_set_commit_mode(struct wl_client *client,
+ struct wl_resource *resource,
+ uint32_t mode)
+{
+ struct weston_subsurface *sub = resource->data;
+
+ switch (mode) {
+ case WL_SUBSURFACE_COMMIT_MODE_PARENT_CACHED:
+ sub->commit_mode = WL_SUBSURFACE_COMMIT_MODE_PARENT_CACHED;
+ break;
+ case WL_SUBSURFACE_COMMIT_MODE_INDEPENDENT:
+ sub->commit_mode = WL_SUBSURFACE_COMMIT_MODE_INDEPENDENT;
+ break;
+ default:
+ wl_resource_post_error(resource,
+ WL_SUBSURFACE_ERROR_BAD_VALUE,
+ "wl_subsurface@%d.set_commit_mode: bad mode %u",
+ resource->object.id, mode);
+ return;
+ }
+}
+
+static void
+subsurface_handle_surface_destroy(struct wl_listener *listener, void *data)
+{
+ struct weston_subsurface *sub =
+ container_of(listener, struct weston_subsurface,
+ surface_destroy_listener);
+ assert(data == &sub->surface->surface.resource);
+
+ if (sub->surface != sub->parent)
+ wl_resource_destroy(&sub->resource);
+ else {
+ /* parent does not have a wl_subsurface resource */
+ assert(sub->resource.destroy == NULL);
+ assert(sub->parent_destroy_listener.notify == NULL);
+ wl_list_remove(&sub->parent_link);
+ free(sub);
+ }
+}
+
+static void
+subsurface_handle_parent_destroy(struct wl_listener *listener, void *data)
+{
+ struct weston_subsurface *sub =
+ container_of(listener, struct weston_subsurface,
+ parent_destroy_listener);
+ assert(data == &sub->parent->surface.resource);
+ assert(sub->surface != sub->parent);
+
+ if (weston_surface_is_mapped(sub->surface))
+ weston_surface_unmap(sub->surface);
+
+ sub->parent = NULL;
+ wl_list_remove(&sub->parent_link);
+ wl_list_remove(&sub->parent_transform.link);
+ wl_list_remove(&sub->parent_dirty_listener.link);
+}
+
+static void
+subsurface_resource_destroy(struct wl_resource *resource)
+{
+ struct weston_subsurface *sub = resource->data;
+ struct weston_frame_callback *cb, *tmp;
+
+ assert(sub);
+ assert(sub->surface);
+ assert(weston_surface_to_subsurface(sub->surface) == sub);
+ assert(sub->parent_destroy_listener.notify ==
+ subsurface_handle_parent_destroy);
+
+ sub->surface->configure = NULL;
+ sub->surface->private = NULL;
+ wl_list_remove(&sub->surface_destroy_listener.link);
+
+ if (sub->parent) {
+ wl_list_remove(&sub->parent_transform.link);
+ wl_list_remove(&sub->parent_dirty_listener.link);
+ wl_list_remove(&sub->parent_link);
+ wl_list_remove(&sub->parent_destroy_listener.link);
+ }
+
+ wl_list_for_each_safe(cb, tmp, &sub->cached.frame_callback_list, link)
+ wl_resource_destroy(&cb->resource);
+
+ weston_buffer_reference(&sub->cached.buffer_ref, NULL);
+ pixman_region32_fini(&sub->cached.damage);
+ pixman_region32_fini(&sub->cached.opaque);
+ pixman_region32_fini(&sub->cached.input);
+
+ free(sub);
+}
+
+static void
+subsurface_destroy(struct wl_client *client, struct wl_resource *resource)
+{
+ wl_resource_destroy(resource);
+}
+
+static void
+subsurface_notify_parent_geometry_dirty(struct wl_listener *listener,
+ void *data)
+{
+ struct weston_subsurface *sub =
+ container_of(listener, struct weston_subsurface,
+ parent_dirty_listener);
+
+ weston_surface_geometry_dirty(sub->surface);
+}
+
+static void
+weston_subsurface_link(struct weston_subsurface *sub,
+ struct weston_surface *surface,
+ struct weston_surface *parent)
+{
+ sub->surface = surface;
+ sub->surface_destroy_listener.notify =
+ subsurface_handle_surface_destroy;
+ wl_signal_add(&surface->surface.resource.destroy_signal,
+ &sub->surface_destroy_listener);
+
+ sub->parent = parent;
+
+ if (sub->surface != sub->parent) {
+ sub->parent_destroy_listener.notify =
+ subsurface_handle_parent_destroy;
+ wl_signal_add(&parent->surface.resource.destroy_signal,
+ &sub->parent_destroy_listener);
+
+ pixman_region32_init(&sub->cached.damage);
+ pixman_region32_init(&sub->cached.opaque);
+ pixman_region32_init(&sub->cached.input);
+ wl_list_init(&sub->cached.frame_callback_list);
+
+ sub->parent_transform.matrix = &parent->transform.matrix;
+ sub->parent_transform.parent = parent;
+ sub->parent_dirty_listener.notify =
+ subsurface_notify_parent_geometry_dirty;
+ wl_signal_add(&parent->transform.dirty_signal,
+ &sub->parent_dirty_listener);
+ wl_list_insert(surface->geometry.transformation_list.prev,
+ &sub->parent_transform.link);
+
+ sub->commit_mode = WL_SUBSURFACE_COMMIT_MODE_PARENT_CACHED;
+ }
+
+ wl_list_insert(&parent->subsurface_list, &sub->parent_link);
+}
+
+static const struct wl_subsurface_interface subsurface_interface = {
+ subsurface_destroy,
+ subsurface_set_position,
+ subsurface_place_above,
+ subsurface_place_below,
+ subsurface_set_commit_mode
+};
+
+static struct weston_subsurface *
+weston_subsurface_create(uint32_t id, struct weston_surface *surface,
+ struct weston_surface *parent)
+{
+ struct weston_subsurface *sub;
+ uint32_t ret;
+
+ sub = calloc(1, sizeof *sub);
+ if (!sub)
+ return NULL;
+
+ sub->resource.destroy = subsurface_resource_destroy;
+ sub->resource.data = sub;
+ sub->resource.object.id = id;
+ sub->resource.object.interface = &wl_subsurface_interface;
+ sub->resource.object.implementation =
+ (void (**)(void))&subsurface_interface;
+
+ ret = wl_client_add_resource(surface->surface.resource.client,
+ &sub->resource);
+ if (ret == 0) {
+ free(sub);
+ return NULL;
+ }
+
+ weston_subsurface_link(sub, surface, parent);
+
+ return sub;
+}
+
+/* Create a dummy subsurface for having the parent itself in its
+ * sub-surface list. Makes stacking order manipulation easy.
+ */
+static struct weston_subsurface *
+weston_subsurface_create_for_parent(struct weston_surface *parent)
+{
+ struct weston_subsurface *sub;
+
+ sub = calloc(1, sizeof *sub);
+ if (!sub)
+ return NULL;
+
+ weston_subsurface_link(sub, parent, parent);
+
+ return sub;
+}
+
+static void
+subcompositor_get_subsurface(struct wl_client *client,
+ struct wl_resource *resource,
+ uint32_t id,
+ struct wl_resource *surface_resource,
+ struct wl_resource *parent_resource)
+{
+ struct weston_surface *surface = surface_resource->data;
+ struct weston_surface *parent = parent_resource->data;
+ struct weston_subsurface *sub;
+ static const char where[] = "get_subsurface: wl_subsurface@";
+
+ if (surface == parent) {
+ wl_resource_post_error(resource,
+ WL_SUBCOMPOSITOR_ERROR_BAD_SURFACE,
+ "%s%d: wl_surface@%d cannot be its own parent",
+ where, id, surface_resource->object.id);
+ return;
+ }
+
+ if (!wl_list_empty(&surface->subsurface_list)) {
+ wl_resource_post_error(resource,
+ WL_SUBCOMPOSITOR_ERROR_BAD_SURFACE,
+ "%s%d: wl_surface@%d is already a parent",
+ where, id, surface_resource->object.id);
+ return;
+ }
+
+ if (weston_surface_to_subsurface(parent)) {
+ wl_resource_post_error(resource,
+ WL_SUBCOMPOSITOR_ERROR_BAD_PARENT,
+ "%s%d: parent wl_surface@%d is already a sub-surface",
+ where, id, parent_resource->object.id);
+ return;
+ }
+
+ if (weston_surface_to_subsurface(surface)) {
+ wl_resource_post_error(resource,
+ WL_SUBCOMPOSITOR_ERROR_BAD_SURFACE,
+ "%s%d: wl_surface@%d is already a sub-surface",
+ where, id, surface_resource->object.id);
+ return;
+ }
+
+ if (surface->configure) {
+ wl_resource_post_error(resource,
+ WL_SUBCOMPOSITOR_ERROR_BAD_SURFACE,
+ "%s%d: wl_surface@%d already has a role",
+ where, id, surface_resource->object.id);
+ return;
+ }
+
+ /* make sure the parent is in its own list */
+ if (wl_list_empty(&parent->subsurface_list)) {
+ if (!weston_subsurface_create_for_parent(parent)) {
+ wl_resource_post_no_memory(resource);
+ return;
+ }
+ }
+
+ sub = weston_subsurface_create(id, surface, parent);
+ if (!sub) {
+ wl_resource_post_no_memory(resource);
+ return;
+ }
+
+ surface->configure = subsurface_configure;
+ surface->private = sub;
+}
+
+static void
+subcompositor_destroy(struct wl_client *client, struct wl_resource *resource)
+{
+ wl_resource_destroy(resource);
+}
+
+static const struct wl_subcompositor_interface subcompositor_interface = {
+ subcompositor_destroy,
+ subcompositor_get_subsurface
+};
+
+static void
+bind_subcompositor(struct wl_client *client,
+ void *data, uint32_t version, uint32_t id)
+{
+ struct weston_compositor *compositor = data;
+
+ wl_client_add_object(client, &wl_subcompositor_interface,
+ &subcompositor_interface, id, compositor);
+}
+
+static void
weston_compositor_dpms(struct weston_compositor *compositor,
enum dpms_enum state)
{
@@ -3053,6 +3664,10 @@ weston_compositor_init(struct weston_compositor *ec,
ec, compositor_bind))
return -1;

+ if (!wl_display_add_global(display, &wl_subcompositor_interface,
+ ec, bind_subcompositor))
+ return -1;
+
wl_list_init(&ec->surface_list);
wl_list_init(&ec->layer_list);
wl_list_init(&ec->seat_list);
diff --git a/src/compositor.h b/src/compositor.h
index 3d197ef..d3c4f2d 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -379,6 +379,55 @@ struct weston_region {
pixman_region32_t region;
};

+struct weston_subsurface {
+ struct wl_resource resource;
+
+ /* guaranteed to be valid and non-NULL */
+ struct weston_surface *surface;
+ struct wl_listener surface_destroy_listener;
+
+ /* can be NULL */
+ struct weston_surface *parent;
+ struct wl_listener parent_destroy_listener;
+ struct wl_list parent_link;
+
+ struct {
+ int32_t x;
+ int32_t y;
+ int set;
+ } position;
+
+ struct {
+ int has_data;
+
+ /* wl_surface.attach */
+ int remove_contents;
+ struct weston_buffer_reference buffer_ref;
+ int32_t sx;
+ int32_t sy;
+
+ /* wl_surface.damage */
+ pixman_region32_t damage;
+
+ /* wl_surface.set_opaque_region */
+ pixman_region32_t opaque;
+
+ /* wl_surface.set_input_region */
+ pixman_region32_t input;
+
+ /* wl_surface.frame */
+ struct wl_list frame_callback_list;
+
+ /* wl_surface.set_buffer_transform */
+ uint32_t buffer_transform;
+ } cached;
+
+ enum wl_subsurface_commit_mode commit_mode;
+
+ struct weston_matrix_pointer parent_transform;
+ struct wl_listener parent_dirty_listener;
+};
+
/* Using weston_surface transformations
*
* To add a transformation to a surface, create a struct weston_matrix_pointer,
@@ -503,6 +552,12 @@ struct weston_surface {
*/
void (*configure)(struct weston_surface *es, int32_t sx, int32_t sy);
void *private;
+
+ /* Parent's list of its sub-surfaces, weston_subsurface:parent_link.
+ * Contains also the parent itself as a dummy weston_subsurface,
+ * if the list is not empty.
+ */
+ struct wl_list subsurface_list;
};

enum weston_key_state_update {
--
1.7.12.4
Pekka Paalanen
2013-02-22 15:07:47 UTC
Permalink
For testing the protocol behaviour only:
- linking a surface to a parent does not fail
- position and placement requests do not fail
- bad linking and arguments do fail
- passing a surface as a sibling from a different set fails
- different destruction sequences do not crash
- setting a surface as its own parent fails

Signed-off-by: Pekka Paalanen <ppaalanen at gmail.com>
---
tests/.gitignore | 1 +
tests/Makefile.am | 5 +-
tests/subsurface-test.c | 327 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 332 insertions(+), 1 deletion(-)
create mode 100644 tests/subsurface-test.c

diff --git a/tests/.gitignore b/tests/.gitignore
index fa19888..fac1f7b 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -13,3 +13,4 @@ button-test
xwayland-test
subsurface-client-protocol.h
subsurface-protocol.c
+subsurface-test
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 9b9351a..b6edbaa 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -9,6 +9,7 @@ weston_tests = \
event-test \
button-test \
text-test \
+ subsurface-test \
$(xwayland_test)

TESTS_ENVIRONMENT = $(SHELL) $(top_srcdir)/tests/weston-tests-env
@@ -82,8 +83,10 @@ text_test_SOURCES = \
$(weston_test_client_src)
text_test_LDADD = $(weston_test_client_libs)

-xwayland_test_SOURCES = xwayland-test.c $(weston_test_client_src)
+subsurface_test_SOURCES = subsurface-test.c $(weston_test_client_src)
+subsurface_test_LDADD = $(weston_test_client_libs)

+xwayland_test_SOURCES = xwayland-test.c $(weston_test_client_src)
xwayland_test_LDADD = $(weston_test_client_libs) $(XWAYLAND_TEST_LIBS)

if ENABLE_XWAYLAND_TEST
diff --git a/tests/subsurface-test.c b/tests/subsurface-test.c
new file mode 100644
index 0000000..2390624
--- /dev/null
+++ b/tests/subsurface-test.c
@@ -0,0 +1,327 @@
+/*
+ * Copyright ? 2012 Collabora, Ltd.
+ *
+ * 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 <string.h>
+
+#include "weston-test-client-helper.h"
+#include "subsurface-client-protocol.h"
+
+#define NUM_SUBSURFACES 3
+
+struct compound_surface {
+ struct wl_subcompositor *subco;
+ struct wl_surface *parent;
+ struct wl_surface *child[NUM_SUBSURFACES];
+ struct wl_subsurface *sub[NUM_SUBSURFACES];
+};
+
+static struct wl_subcompositor *
+get_subcompositor(struct client *client)
+{
+ struct global *g;
+ struct global *global_sub = NULL;
+ struct wl_subcompositor *sub;
+
+ wl_list_for_each(g, &client->global_list, link) {
+ if (strcmp(g->interface, "wl_subcompositor"))
+ continue;
+
+ if (global_sub)
+ assert(0 && "multiple wl_subcompositor objects");
+
+ global_sub = g;
+ }
+
+ assert(global_sub && "no wl_subcompositor found");
+
+ assert(global_sub->version == 1);
+
+ sub = wl_registry_bind(client->wl_registry, global_sub->name,
+ &wl_subcompositor_interface, 1);
+ assert(sub);
+
+ return sub;
+}
+
+static void
+populate_compound_surface(struct compound_surface *com, struct client *client)
+{
+ int i;
+
+ com->subco = get_subcompositor(client);
+
+ com->parent = wl_compositor_create_surface(client->wl_compositor);
+
+ for (i = 0; i < NUM_SUBSURFACES; i++) {
+ com->child[i] =
+ wl_compositor_create_surface(client->wl_compositor);
+ com->sub[i] =
+ wl_subcompositor_get_subsurface(com->subco,
+ com->child[i],
+ com->parent);
+ }
+}
+
+TEST(test_subsurface_basic_protocol)
+{
+ struct client *client;
+ struct compound_surface com1;
+ struct compound_surface com2;
+
+ client = client_create(100, 50, 123, 77);
+ assert(client);
+
+ populate_compound_surface(&com1, client);
+ populate_compound_surface(&com2, client);
+
+ client_roundtrip(client);
+}
+
+TEST(test_subsurface_position_protocol)
+{
+ struct client *client;
+ struct compound_surface com;
+ int i;
+
+ client = client_create(100, 50, 123, 77);
+ assert(client);
+
+ populate_compound_surface(&com, client);
+ for (i = 0; i < NUM_SUBSURFACES; i++)
+ wl_subsurface_set_position(com.sub[i],
+ (i + 2) * 20, (i + 2) * 10);
+
+ client_roundtrip(client);
+}
+
+TEST(test_subsurface_placement_protocol)
+{
+ struct client *client;
+ struct compound_surface com;
+
+ client = client_create(100, 50, 123, 77);
+ assert(client);
+
+ populate_compound_surface(&com, client);
+
+ wl_subsurface_place_above(com.sub[0], com.child[1]);
+ wl_subsurface_place_above(com.sub[1], com.parent);
+ wl_subsurface_place_below(com.sub[2], com.child[0]);
+ wl_subsurface_place_below(com.sub[1], com.parent);
+
+ client_roundtrip(client);
+}
+
+FAIL_TEST(test_subsurface_paradox)
+{
+ struct client *client;
+ struct wl_surface *parent;
+ struct wl_subcompositor *subco;
+
+ client = client_create(100, 50, 123, 77);
+ assert(client);
+
+ subco = get_subcompositor(client);
+ parent = wl_compositor_create_surface(client->wl_compositor);
+
+ /* surface is its own parent */
+ wl_subcompositor_get_subsurface(subco, parent, parent);
+
+ client_roundtrip(client);
+}
+
+FAIL_TEST(test_subsurface_identical_link)
+{
+ struct client *client;
+ struct compound_surface com;
+
+ client = client_create(100, 50, 123, 77);
+ assert(client);
+
+ populate_compound_surface(&com, client);
+
+ /* surface is already a subsurface */
+ wl_subcompositor_get_subsurface(com.subco, com.child[0], com.parent);
+
+ client_roundtrip(client);
+}
+
+FAIL_TEST(test_subsurface_change_link)
+{
+ struct client *client;
+ struct compound_surface com;
+ struct wl_surface *stranger;
+
+ client = client_create(100, 50, 123, 77);
+ assert(client);
+
+ stranger = wl_compositor_create_surface(client->wl_compositor);
+ populate_compound_surface(&com, client);
+
+ /* surface is already a subsurface */
+ wl_subcompositor_get_subsurface(com.subco, com.child[0], stranger);
+
+ client_roundtrip(client);
+}
+
+FAIL_TEST(test_subsurface_nesting)
+{
+ struct client *client;
+ struct compound_surface com;
+ struct wl_surface *stranger;
+
+ client = client_create(100, 50, 123, 77);
+ assert(client);
+
+ stranger = wl_compositor_create_surface(client->wl_compositor);
+ populate_compound_surface(&com, client);
+
+ /* parent is a sub-surface */
+ wl_subcompositor_get_subsurface(com.subco, stranger, com.child[0]);
+
+ client_roundtrip(client);
+}
+
+FAIL_TEST(test_subsurface_nesting_parent)
+{
+ struct client *client;
+ struct compound_surface com;
+ struct wl_surface *stranger;
+
+ client = client_create(100, 50, 123, 77);
+ assert(client);
+
+ stranger = wl_compositor_create_surface(client->wl_compositor);
+ populate_compound_surface(&com, client);
+
+ /* surface is already a parent */
+ wl_subcompositor_get_subsurface(com.subco, com.parent, stranger);
+
+ client_roundtrip(client);
+}
+
+FAIL_TEST(test_subsurface_place_above_stranger)
+{
+ struct client *client;
+ struct compound_surface com;
+ struct wl_surface *stranger;
+
+ client = client_create(100, 50, 123, 77);
+ assert(client);
+
+ stranger = wl_compositor_create_surface(client->wl_compositor);
+ populate_compound_surface(&com, client);
+
+ /* bad sibling */
+ wl_subsurface_place_above(com.sub[0], stranger);
+
+ client_roundtrip(client);
+}
+
+FAIL_TEST(test_subsurface_place_below_stranger)
+{
+ struct client *client;
+ struct compound_surface com;
+ struct wl_surface *stranger;
+
+ client = client_create(100, 50, 123, 77);
+ assert(client);
+
+ stranger = wl_compositor_create_surface(client->wl_compositor);
+ populate_compound_surface(&com, client);
+
+ /* bad sibling */
+ wl_subsurface_place_below(com.sub[0], stranger);
+
+ client_roundtrip(client);
+}
+
+FAIL_TEST(test_subsurface_place_above_foreign)
+{
+ struct client *client;
+ struct compound_surface com1;
+ struct compound_surface com2;
+
+ client = client_create(100, 50, 123, 77);
+ assert(client);
+
+ populate_compound_surface(&com1, client);
+ populate_compound_surface(&com2, client);
+
+ /* bad sibling */
+ wl_subsurface_place_above(com1.sub[0], com2.child[0]);
+
+ client_roundtrip(client);
+}
+
+FAIL_TEST(test_subsurface_place_below_foreign)
+{
+ struct client *client;
+ struct compound_surface com1;
+ struct compound_surface com2;
+
+ client = client_create(100, 50, 123, 77);
+ assert(client);
+
+ populate_compound_surface(&com1, client);
+ populate_compound_surface(&com2, client);
+
+ /* bad sibling */
+ wl_subsurface_place_below(com1.sub[0], com2.child[0]);
+
+ client_roundtrip(client);
+}
+
+TEST(test_subsurface_destroy_protocol)
+{
+ struct client *client;
+ struct compound_surface com;
+
+ client = client_create(100, 50, 123, 77);
+ assert(client);
+
+ populate_compound_surface(&com, client);
+
+ /* not needed */
+ wl_subcompositor_destroy(com.subco);
+
+ /* detach child from parent */
+ wl_subsurface_destroy(com.sub[0]);
+
+ /* destroy: child, parent */
+ wl_surface_destroy(com.child[1]);
+ wl_surface_destroy(com.parent);
+
+ /* destroy: parent, child */
+ wl_surface_destroy(com.child[2]);
+
+ /* destroy: sub, child */
+ wl_surface_destroy(com.child[0]);
+
+ /* leak: wl_subsurface should be destroyed before the wl_surface,
+ * otherwise a protocol error is issued for invalid object.
+ wl_subsurface_destroy(com.sub[2]);
+ wl_subsurface_destroy(com.sub[1]);
+ */
+
+ client_roundtrip(client);
+}
--
1.7.12.4
Pekka Paalanen
2013-02-22 15:07:48 UTC
Permalink
Modify surface list rebuilding in weston_output_repaint() to process
sub-surface lists, if they are non-empty. The sub-surface list always
contains the parent, too, if not empty.

The collection of frame_callback_list is moved to a later loop, to
streamline the surface list rebuild functions. This should not change
anything.

Signed-off-by: Pekka Paalanen <ppaalanen at gmail.com>
---
src/compositor.c | 59 ++++++++++++++++++++++++++++++++++++++++++--------------
1 file changed, 45 insertions(+), 14 deletions(-)

diff --git a/src/compositor.c b/src/compositor.c
index 9c14423..5db255a 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -1120,11 +1120,47 @@ surface_accumulate_damage(struct weston_surface *surface,
}

static void
+surface_list_add(struct weston_compositor *compositor,
+ struct weston_surface *surface)
+{
+ struct weston_subsurface *sub;
+
+ if (wl_list_empty(&surface->subsurface_list)) {
+ weston_surface_update_transform(surface);
+ wl_list_insert(compositor->surface_list.prev, &surface->link);
+ return;
+ }
+
+ wl_list_for_each(sub, &surface->subsurface_list, parent_link) {
+ if (!weston_surface_is_mapped(sub->surface))
+ continue;
+
+ weston_surface_update_transform(sub->surface);
+
+ wl_list_insert(compositor->surface_list.prev,
+ &sub->surface->link);
+ }
+}
+
+static void
+weston_compositor_build_surface_list(struct weston_compositor *compositor)
+{
+ struct weston_surface *surface;
+ struct weston_layer *layer;
+
+ wl_list_init(&compositor->surface_list);
+ wl_list_for_each(layer, &compositor->layer_list, link) {
+ wl_list_for_each(surface, &layer->surface_list, layer_link) {
+ surface_list_add(compositor, surface);
+ }
+ }
+}
+
+static void
weston_output_repaint(struct weston_output *output, uint32_t msecs)
{
struct weston_compositor *ec = output->compositor;
struct weston_surface *es;
- struct weston_layer *layer;
struct weston_animation *animation, *next;
struct weston_frame_callback *cb, *cnext;
struct wl_list frame_callback_list;
@@ -1133,19 +1169,7 @@ weston_output_repaint(struct weston_output *output, uint32_t msecs)
weston_compositor_update_drag_surfaces(ec);

/* Rebuild the surface list and update surface transforms up front. */
- wl_list_init(&ec->surface_list);
- wl_list_init(&frame_callback_list);
- wl_list_for_each(layer, &ec->layer_list, link) {
- wl_list_for_each(es, &layer->surface_list, layer_link) {
- weston_surface_update_transform(es);
- wl_list_insert(ec->surface_list.prev, &es->link);
- if (es->output == output) {
- wl_list_insert_list(&frame_callback_list,
- &es->frame_callback_list);
- wl_list_init(&es->frame_callback_list);
- }
- }
- }
+ weston_compositor_build_surface_list(ec);

if (output->assign_planes && !output->disable_planes)
output->assign_planes(output);
@@ -1158,7 +1182,14 @@ weston_output_repaint(struct weston_output *output, uint32_t msecs)
pixman_region32_fini(&ec->primary_plane.opaque);
pixman_region32_init(&ec->primary_plane.opaque);

+ wl_list_init(&frame_callback_list);
wl_list_for_each(es, &ec->surface_list, link) {
+ if (es->output == output) {
+ wl_list_insert_list(&frame_callback_list,
+ &es->frame_callback_list);
+ wl_list_init(&es->frame_callback_list);
+ }
+
surface_accumulate_damage(es, &opaque);

/* Both the renderer and the backend have seen the buffer
--
1.7.12.4
Pekka Paalanen
2013-02-22 15:07:49 UTC
Permalink
This post might be inappropriate. Click to display it.
Pekka Paalanen
2013-02-22 15:07:50 UTC
Permalink
Increase the maximum number of shm "leaves" to three, and rewrite the
leaf release and pick algorithms. The new algorithms hopefully improve
on buffer re-use while freeing unused buffers.

The goal of the new release algorithm is to always leave one free leaf
with storage allocated, so that the next redraw could start straight on
it.

The new leaf picking algorithm will prefer a free leaf that already has
some storage allocated, instead of just picking the first free leaf that
may need to allocate a new buffer.

Triple-buffering is especially for sub-surfaces, where the compositor
may have one wl_buffer busy on screen, and another wl_buffer busy in the
sub-surface cached state due to the parent-cached commit mode. To be
able to forcibly repaint at that situation for e.g. resize, we need a
third buffer.
---
clients/window.c | 66 +++++++++++++++++++++++++++++++++++---------------------
1 file changed, 41 insertions(+), 25 deletions(-)

diff --git a/clients/window.c b/clients/window.c
index 249ba6f..d17fc2f 100644
--- a/clients/window.c
+++ b/clients/window.c
@@ -818,6 +818,8 @@ shm_surface_leaf_release(struct shm_surface_leaf *leaf)
memset(leaf, 0, sizeof *leaf);
}

+#define MAX_LEAVES 3
+
struct shm_surface {
struct toysurface base;
struct display *display;
@@ -825,7 +827,7 @@ struct shm_surface {
uint32_t flags;
int dx, dy;

- struct shm_surface_leaf leaf[2];
+ struct shm_surface_leaf leaf[MAX_LEAVES];
struct shm_surface_leaf *current;
};

@@ -839,16 +841,32 @@ static void
shm_surface_buffer_release(void *data, struct wl_buffer *buffer)
{
struct shm_surface *surface = data;
+ struct shm_surface_leaf *leaf;
+ int i;
+ int free_found;

- if (surface->leaf[0].data->buffer == buffer)
- surface->leaf[0].busy = 0;
- else if (surface->leaf[1].data->buffer == buffer)
- surface->leaf[1].busy = 0;
- else
- assert(0 && "shm_surface_buffer_release: unknown buffer");
+ for (i = 0; i < MAX_LEAVES; i++) {
+ leaf = &surface->leaf[i];
+ if (leaf->data && leaf->data->buffer == buffer) {
+ leaf->busy = 0;
+ break;
+ }
+ }
+ assert(i < MAX_LEAVES && "unknown buffer released");
+
+ /* Leave one free leaf with storage, release others */
+ free_found = 0;
+ for (i = 0; i < MAX_LEAVES; i++) {
+ leaf = &surface->leaf[i];
+
+ if (!leaf->cairo_surface || leaf->busy)
+ continue;

- if (!surface->leaf[0].busy && !surface->leaf[1].busy)
- shm_surface_leaf_release(&surface->leaf[1]);
+ if (!free_found)
+ free_found = 1;
+ else
+ shm_surface_leaf_release(leaf);
+ }
}

static const struct wl_buffer_listener shm_surface_buffer_listener = {
@@ -862,25 +880,22 @@ shm_surface_prepare(struct toysurface *base, int dx, int dy,
int resize_hint = !!(flags & SURFACE_HINT_RESIZE);
struct shm_surface *surface = to_shm_surface(base);
struct rectangle rect = { 0, 0, width, height };
- struct shm_surface_leaf *leaf;
+ struct shm_surface_leaf *leaf = NULL;
+ int i;

surface->dx = dx;
surface->dy = dy;

- /* See shm_surface_buffer_release() */
- if (!surface->leaf[0].busy && !surface->leaf[1].busy &&
- surface->leaf[1].cairo_surface) {
- fprintf(stderr, "window.c:%s: TODO: release leaf[1]\n",
- __func__);
- }
+ /* pick a free buffer, preferrably one that already has storage */
+ for (i = 0; i < MAX_LEAVES; i++) {
+ if (surface->leaf[i].busy)
+ continue;

- /* pick a free buffer from the two */
- if (!surface->leaf[0].busy)
- leaf = &surface->leaf[0];
- else if (!surface->leaf[1].busy)
- leaf = &surface->leaf[1];
- else {
- fprintf(stderr, "%s: both buffers are held by the server.\n",
+ if (!leaf || surface->leaf[i].cairo_surface)
+ leaf = &surface->leaf[i];
+ }
+ if (!leaf) {
+ fprintf(stderr, "%s: all buffers are held by the server.\n",
__func__);
return NULL;
}
@@ -962,9 +977,10 @@ static void
shm_surface_destroy(struct toysurface *base)
{
struct shm_surface *surface = to_shm_surface(base);
+ int i;

- shm_surface_leaf_release(&surface->leaf[0]);
- shm_surface_leaf_release(&surface->leaf[1]);
+ for (i = 0; i < MAX_LEAVES; i++)
+ shm_surface_leaf_release(&surface->leaf[i]);

free(surface);
}
--
1.7.12.4
Pekka Paalanen
2013-02-22 15:07:51 UTC
Permalink
This patch depends on the wl_subsurface protocol.

The new application API window_add_subsurface() will create a plain
widget that is on a new sub-surface.

The sub-surface position is taken from the surface's root widget
allocation. This way widget allocations are always in the main surface
(i.e. window) coordinates. However, Cairo drawing coordinates will now
be different to widget coordinates for sub-surfaces. Cairo coordinates
are fixed by applying a translation in widget_cairo_create(), so that
widget drawing code can simply use the widget allocation as before.

Sub-surfaces are hooked up into resize, window flush, redraw, and
find_widget. Window maintains a list of sub-surfaces in top-first order.

window: move wl_subsurface to struct surface

window: use sub-surface commit mode

Add a client settable default mode, and toggle the mode when resizing to
guarantee in-sync updates of a window and its sub-surfaces.
---
clients/window.c | 137 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
clients/window.h | 4 ++
2 files changed, 138 insertions(+), 3 deletions(-)

diff --git a/clients/window.c b/clients/window.c
index d17fc2f..e538da9 100644
--- a/clients/window.c
+++ b/clients/window.c
@@ -85,6 +85,7 @@ struct display {
struct wl_display *display;
struct wl_registry *registry;
struct wl_compositor *compositor;
+ struct wl_subcompositor *subcompositor;
struct wl_shell *shell;
struct wl_shm *shm;
struct wl_data_device_manager *data_device_manager;
@@ -189,6 +190,9 @@ struct surface {
struct window *window;

struct wl_surface *surface;
+ struct wl_subsurface *subsurface;
+ enum wl_subsurface_commit_mode subsurface_mode;
+ enum wl_subsurface_commit_mode default_mode;
struct toysurface *toysurface;
struct widget *widget;

@@ -202,6 +206,8 @@ struct surface {
enum wl_output_transform buffer_transform;

cairo_surface_t *cairo_surface;
+
+ struct wl_list link;
};

struct window {
@@ -238,6 +244,9 @@ struct window {

struct frame *frame;

+ /* struct surface::link, contains also main_surface */
+ struct wl_list subsurface_list;
+
void *user_data;
struct wl_list link;
};
@@ -1193,12 +1202,21 @@ window_has_focus(struct window *window)
static void
window_flush(struct window *window)
{
+ struct surface *surface;
+
if (window->type == TYPE_NONE) {
window->type = TYPE_TOPLEVEL;
if (window->shell_surface)
wl_shell_surface_set_toplevel(window->shell_surface);
}

+ wl_list_for_each(surface, &window->subsurface_list, link) {
+ if (surface == window->main_surface)
+ continue;
+
+ surface_flush(surface);
+ }
+
surface_flush(window->main_surface);
}

@@ -1295,11 +1313,15 @@ surface_destroy(struct surface *surface)
if (surface->opaque_region)
wl_region_destroy(surface->opaque_region);

+ if (surface->subsurface)
+ wl_subsurface_destroy(surface->subsurface);
+
wl_surface_destroy(surface->surface);

if (surface->toysurface)
surface->toysurface->destroy(surface->toysurface);

+ wl_list_remove(&surface->link);
free(surface);
}

@@ -1369,7 +1391,16 @@ widget_find_widget(struct widget *widget, int32_t x, int32_t y)
static struct widget *
window_find_widget(struct window *window, int32_t x, int32_t y)
{
- return widget_find_widget(window->main_surface->widget, x, y);
+ struct surface *surface;
+ struct widget *widget;
+
+ wl_list_for_each(surface, &window->subsurface_list, link) {
+ widget = widget_find_widget(surface->widget, x, y);
+ if (widget)
+ return widget;
+ }
+
+ return NULL;
}

static struct widget *
@@ -1419,8 +1450,13 @@ void
widget_destroy(struct widget *widget)
{
struct display *display = widget->window->display;
+ struct surface *surface = widget->surface;
struct input *input;

+ /* Destroy the sub-surface along with the root widget */
+ if (surface->widget == widget && surface->subsurface)
+ surface_destroy(widget->surface);
+
if (widget->tooltip) {
free(widget->tooltip);
widget->tooltip = NULL;
@@ -1494,12 +1530,15 @@ widget_get_cairo_surface(struct widget *widget)
cairo_t *
widget_cairo_create(struct widget *widget)
{
+ struct surface *surface = widget->surface;
cairo_surface_t *cairo_surface;
cairo_t *cr;

cairo_surface = widget_get_cairo_surface(widget);
cr = cairo_create(cairo_surface);

+ cairo_translate(cr, -surface->allocation.x, -surface->allocation.y);
+
return cr;
}

@@ -3216,6 +3255,34 @@ window_move(struct window *window, struct input *input, uint32_t serial)
}

static void
+surface_set_commit_mode_cached(struct surface *surface)
+{
+ if (!surface->subsurface)
+ return;
+
+ if (surface->subsurface_mode == WL_SUBSURFACE_COMMIT_MODE_PARENT_CACHED)
+ return;
+
+ wl_subsurface_set_commit_mode(surface->subsurface,
+ WL_SUBSURFACE_COMMIT_MODE_PARENT_CACHED);
+ surface->subsurface_mode = WL_SUBSURFACE_COMMIT_MODE_PARENT_CACHED;
+}
+
+static void
+surface_set_commit_mode_default(struct surface *surface)
+{
+ if (!surface->subsurface)
+ return;
+
+ if (surface->subsurface_mode == surface->default_mode)
+ return;
+
+ wl_subsurface_set_commit_mode(surface->subsurface,
+ surface->default_mode);
+ surface->subsurface_mode = surface->default_mode;
+}
+
+static void
surface_resize(struct surface *surface)
{
struct widget *widget = surface->widget;
@@ -3237,11 +3304,18 @@ surface_resize(struct surface *surface)
widget->allocation.height,
widget->user_data);

+ if (surface->subsurface &&
+ (surface->allocation.x != widget->allocation.x ||
+ surface->allocation.y != widget->allocation.y)) {
+ wl_subsurface_set_position(surface->subsurface,
+ widget->allocation.x,
+ widget->allocation.y);
+ }
if (surface->allocation.width != widget->allocation.width ||
surface->allocation.height != widget->allocation.height) {
- surface->allocation = widget->allocation;
window_schedule_redraw(widget->window);
}
+ surface->allocation = widget->allocation;

if (widget->opaque)
wl_region_add(surface->opaque_region, 0, 0,
@@ -3252,6 +3326,8 @@ surface_resize(struct surface *surface)
static void
idle_resize(struct window *window)
{
+ struct surface *surface;
+
window->resize_needed = 0;

widget_set_allocation(window->main_surface->widget,
@@ -3261,6 +3337,19 @@ idle_resize(struct window *window)
window->pending_allocation.height);

surface_resize(window->main_surface);
+
+ /* The main surface is in the list, too. Main surface's
+ * resize_handler is responsible for calling widget_set_allocation()
+ * on all sub-surface root widgets, so they will be resized
+ * properly.
+ */
+ wl_list_for_each(surface, &window->subsurface_list, link) {
+ if (surface == window->main_surface)
+ continue;
+
+ surface_set_commit_mode_cached(surface);
+ surface_resize(surface);
+ }
}

void
@@ -3374,17 +3463,23 @@ static void
idle_redraw(struct task *task, uint32_t events)
{
struct window *window = container_of(task, struct window, redraw_task);
+ struct surface *surface;

if (window->resize_needed)
idle_resize(window);

- widget_redraw(window->main_surface->widget);
+ wl_list_for_each(surface, &window->subsurface_list, link)
+ widget_redraw(surface->widget);
+
window->redraw_needed = 0;
wl_list_init(&window->redraw_task.link);

window->frame_cb = wl_surface_frame(window->main_surface->surface);
wl_callback_add_listener(window->frame_cb, &listener, window);
window_flush(window);
+
+ wl_list_for_each(surface, &window->subsurface_list, link)
+ surface_set_commit_mode_default(surface);
}

void
@@ -3633,6 +3728,8 @@ surface_create(struct window *window)
surface->surface = wl_compositor_create_surface(display->compositor);
wl_surface_add_listener(surface->surface, &surface_listener, window);

+ wl_list_insert(&window->subsurface_list, &surface->link);
+
return surface;
}

@@ -3648,6 +3745,7 @@ window_create_internal(struct display *display,
return NULL;

memset(window, 0, sizeof *window);
+ wl_list_init(&window->subsurface_list);
window->display = display;
window->parent = parent;

@@ -3897,6 +3995,32 @@ window_set_buffer_type(struct window *window, enum window_buffer_type type)
window->main_surface->buffer_type = type;
}

+struct widget *
+window_add_subsurface(struct window *window, void *data,
+ enum wl_subsurface_commit_mode default_mode)
+{
+ struct widget *widget;
+ struct surface *surface;
+ struct wl_surface *parent;
+ struct wl_subcompositor *subcompo = window->display->subcompositor;
+
+ if (!subcompo)
+ return NULL;
+
+ surface = surface_create(window);
+ widget = widget_create(window, surface, data);
+ wl_list_init(&widget->link);
+ surface->widget = widget;
+
+ parent = window->main_surface->surface;
+ surface->subsurface = wl_subcompositor_get_subsurface(subcompo,
+ surface->surface,
+ parent);
+ surface->subsurface_mode = WL_SUBSURFACE_COMMIT_MODE_PARENT_CACHED;
+ surface->default_mode = default_mode;
+
+ return widget;
+}

static void
display_handle_geometry(void *data,
@@ -4168,6 +4292,10 @@ registry_handle_global(void *data, struct wl_registry *registry, uint32_t id,
&text_cursor_position_interface, 1);
} else if (strcmp(interface, "workspace_manager") == 0) {
init_workspace_manager(d, id);
+ } else if (strcmp(interface, "wl_subcompositor") == 0) {
+ d->subcompositor =
+ wl_registry_bind(registry, id,
+ &wl_subcompositor_interface, 1);
}

if (d->global_handler)
@@ -4453,6 +4581,9 @@ display_destroy(struct display *display)
fini_egl(display);
#endif

+ if (display->subcompositor)
+ wl_subcompositor_destroy(display->subcompositor);
+
if (display->shell)
wl_shell_destroy(display->shell);

diff --git a/clients/window.h b/clients/window.h
index 815b3f1..30466c0 100644
--- a/clients/window.h
+++ b/clients/window.h
@@ -259,6 +259,10 @@ window_destroy(struct window *window);
struct widget *
window_add_widget(struct window *window, void *data);

+struct widget *
+window_add_subsurface(struct window *window, void *data,
+ enum wl_subsurface_commit_mode default_mode);
+
typedef void (*data_func_t)(void *data, size_t len,
int32_t x, int32_t y, void *user_data);
--
1.7.12.4
Pekka Paalanen
2013-02-22 15:07:52 UTC
Permalink
Add redraw_needed flag to all surfaces, in addition to having one in
window.

widget_schedule_redraw() now schedules the redraw only for the surface,
where the widget is on. window_schedule_redraw() is equivalent to
scheduling a redraw for all (sub-)surfaces of the window.

We still use only one deferred task for all redraws.

surface_redraw() will skip the redraw, if the window does not force a
redraw and the surface does not need a redraw. It will also skip the
redraw, if the frame callback from the previous redraw has not triggered
yet. When the frame callback later arrives, the redraw task will be
scheduled, if the surface still needs a redraw.

If the window forces a redraw, the redraw is executed even if there is a
pending frame callback. This is for resizing: resizing should trigger a
window repaint, as it really wants to update all surfaces in one go, to
apply possible sub-surface size and position changes. Resizing is the
only thing that makes a window force a redraw.

With this change, subsurfaces demo can avoid repainting the cairo
sub-surface while still animating the GL sub-surface.
---
clients/window.c | 80 +++++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 59 insertions(+), 21 deletions(-)

diff --git a/clients/window.c b/clients/window.c
index e538da9..4730c89 100644
--- a/clients/window.c
+++ b/clients/window.c
@@ -195,6 +195,8 @@ struct surface {
enum wl_subsurface_commit_mode default_mode;
struct toysurface *toysurface;
struct widget *widget;
+ int redraw_needed;
+ struct wl_callback *frame_cb;

struct rectangle allocation;
struct rectangle server_allocation;
@@ -220,8 +222,8 @@ struct window {
struct rectangle pending_allocation;
int x, y;
int resize_edges;
- int redraw_scheduled;
int redraw_needed;
+ int redraw_task_scheduled;
struct task redraw_task;
int resize_needed;
int type;
@@ -240,7 +242,6 @@ struct window {

struct surface *main_surface;
struct wl_shell_surface *shell_surface;
- struct wl_callback *frame_cb;

struct frame *frame;

@@ -1307,6 +1308,9 @@ static void frame_destroy(struct frame *frame);
static void
surface_destroy(struct surface *surface)
{
+ if (surface->frame_cb)
+ wl_callback_destroy(surface->frame_cb);
+
if (surface->input_region)
wl_region_destroy(surface->input_region);

@@ -1333,8 +1337,7 @@ window_destroy(struct window *window)
struct window_output *window_output;
struct window_output *window_output_tmp;

- if (window->redraw_scheduled)
- wl_list_remove(&window->redraw_task.link);
+ wl_list_remove(&window->redraw_task.link);

wl_list_for_each(input, &display->input_list, link) {
if (input->pointer_focus == window)
@@ -1361,8 +1364,6 @@ window_destroy(struct window *window)

wl_list_remove(&window->link);

- if (window->frame_cb)
- wl_callback_destroy(window->frame_cb);
free(window->title);
free(window);
}
@@ -1589,10 +1590,14 @@ widget_set_axis_handler(struct widget *widget,
widget->axis_handler = handler;
}

+static void
+window_schedule_redraw_task(struct window *window);
+
void
widget_schedule_redraw(struct widget *widget)
{
- window_schedule_redraw(widget->window);
+ widget->surface->redraw_needed = 1;
+ window_schedule_redraw_task(widget->window);
}

cairo_surface_t *
@@ -3329,6 +3334,7 @@ idle_resize(struct window *window)
struct surface *surface;

window->resize_needed = 0;
+ window->redraw_needed = 1;

widget_set_allocation(window->main_surface->widget,
window->pending_allocation.x,
@@ -3445,14 +3451,14 @@ widget_redraw(struct widget *widget)
static void
frame_callback(void *data, struct wl_callback *callback, uint32_t time)
{
- struct window *window = data;
+ struct surface *surface = data;

- assert(callback == window->frame_cb);
+ assert(callback == surface->frame_cb);
wl_callback_destroy(callback);
- window->frame_cb = 0;
- window->redraw_scheduled = 0;
- if (window->redraw_needed)
- window_schedule_redraw(window);
+ surface->frame_cb = NULL;
+
+ if (surface->redraw_needed || surface->window->redraw_needed)
+ window_schedule_redraw_task(surface->window);
}

static const struct wl_callback_listener listener = {
@@ -3460,6 +3466,29 @@ static const struct wl_callback_listener listener = {
};

static void
+surface_redraw(struct surface *surface)
+{
+ if (!surface->window->redraw_needed && !surface->redraw_needed)
+ return;
+
+ /* Whole-window redraw forces a redraw even if the previous has
+ * not yet hit the screen.
+ */
+ if (surface->frame_cb) {
+ if (!surface->window->redraw_needed)
+ return;
+
+ wl_callback_destroy(surface->frame_cb);
+ }
+
+ surface->frame_cb = wl_surface_frame(surface->surface);
+ wl_callback_add_listener(surface->frame_cb, &listener, surface);
+
+ surface->redraw_needed = 0;
+ widget_redraw(surface->widget);
+}
+
+static void
idle_redraw(struct task *task, uint32_t events)
{
struct window *window = container_of(task, struct window, redraw_task);
@@ -3469,30 +3498,39 @@ idle_redraw(struct task *task, uint32_t events)
idle_resize(window);

wl_list_for_each(surface, &window->subsurface_list, link)
- widget_redraw(surface->widget);
+ surface_redraw(surface);

window->redraw_needed = 0;
wl_list_init(&window->redraw_task.link);
+ window->redraw_task_scheduled = 0;

- window->frame_cb = wl_surface_frame(window->main_surface->surface);
- wl_callback_add_listener(window->frame_cb, &listener, window);
window_flush(window);

wl_list_for_each(surface, &window->subsurface_list, link)
surface_set_commit_mode_default(surface);
}

-void
-window_schedule_redraw(struct window *window)
+static void
+window_schedule_redraw_task(struct window *window)
{
- window->redraw_needed = 1;
- if (!window->redraw_scheduled) {
+ if (!window->redraw_task_scheduled) {
window->redraw_task.run = idle_redraw;
display_defer(window->display, &window->redraw_task);
- window->redraw_scheduled = 1;
+ window->redraw_task_scheduled = 1;
}
}

+void
+window_schedule_redraw(struct window *window)
+{
+ struct surface *surface;
+
+ wl_list_for_each(surface, &window->subsurface_list, link)
+ surface->redraw_needed = 1;
+
+ window_schedule_redraw_task(window);
+}
+
int
window_is_fullscreen(struct window *window)
{
--
1.7.12.4
MoD
2013-02-23 18:21:20 UTC
Permalink
Presently every toolkit using wayland-cursor hard-codes or has its own configuration for cursor size, and there is no scheme for coordination. Weston documents the XCURSOR_SIZE environment variable as affecting Wayland programs. This change brings that functionality into wayland-cursor so consumers can just pass 0 for size to do the right thing. The logic is duplicated in xwayland/window-manager.c, as I'm hesitant to alter semantics of X-namespaced functions without confirmation; I'm not sure of the design considerations for that part of libwayland-cursor.
---
cursor/wayland-cursor.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/cursor/wayland-cursor.c b/cursor/wayland-cursor.c
index 25e51c2..e4a7452 100644
--- a/cursor/wayland-cursor.c
+++ b/cursor/wayland-cursor.c
@@ -324,7 +324,8 @@ load_callback(XcursorImages *images, void *data)
*
* \param name The name of the cursor theme to load. If %NULL, the default
* theme will be loaded.
- * \param size Desired size of the cursor images.
+ * \param size Desired size of the cursor images. If zero, the default size
+ * will be used.
* \param shm The compositor's shm interface.
*
* \return An object representing the theme that should be destroyed with
@@ -335,6 +336,7 @@ WL_EXPORT struct wl_cursor_theme *
wl_cursor_theme_load(const char *name, int size, struct wl_shm *shm)
{
struct wl_cursor_theme *theme;
+ char *v = NULL;

theme = malloc(sizeof *theme);
if (!theme)
@@ -344,6 +346,16 @@ wl_cursor_theme_load(const char *name, int size, struct wl_shm *shm)
name = "default";

theme->name = strdup(name);
+
+ if (!size)
+ {
+ v = getenv ("XCURSOR_SIZE");
+ if (v)
+ size = atoi(v);
+ if (!size)
+ size = 32;
+ }
+
theme->size = size;
theme->cursor_count = 0;
theme->cursors = NULL;
--
1.8.1.4
MoD
2013-02-23 18:21:31 UTC
Permalink
After wayland-cursor changes, this allows users to configure cursor size for toytoolkit with the XCURSOR_SIZE environment variable as documented in weston's man page.
---
clients/window.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clients/window.c b/clients/window.c
index 249ba6f..29201fe 100644
--- a/clients/window.c
+++ b/clients/window.c
@@ -1108,7 +1108,7 @@ create_cursors(struct display *display)
parse_config_file(config_file, cs, ARRAY_LENGTH(cs), NULL);
free(config_file);

- display->cursor_theme = wl_cursor_theme_load(theme, 32, display->shm);
+ display->cursor_theme = wl_cursor_theme_load(theme, 0, display->shm);
display->cursors =
malloc(ARRAY_LENGTH(cursors) * sizeof display->cursors[0]);
--
1.8.1.4
Pekka Paalanen
2013-02-22 15:07:53 UTC
Permalink
Add a demo program with:
- a main surface (green)
- a Cairo-image sub-surface (red)
- a raw GLESv2 widget (triangle)

Sub-surface input region is set empty to avoid problems in toytoolkit.

If Cairo links to libGL, then we will end up with also libGLESv2 linked
to subsurfaces program, and both libs getting really used, which leads
to disaster.

Do not build subsurfaces demo, if Cairo links to libGL and cairo-egl is
usable.

The GL rendering loop is not tied to the toytoolkit or the widget, but
runs directly from its own frame callback. Therefore it runs
independent of the rest of the application. This also relies on one of
two things:
- eglSwapInterval(0) is implemented, and therefore eglSwapBuffers never
blocks indefinitely, or
- toytoolkit has a workaround, that guarantees that eglSwapBuffers will
return soon, when we force a repaint on resize.
Otherwise the demo will deadlock.

The code is separated into three sections:

1. The library component, using only EGL, GLESv2, and libwayland-client
APIs, and not aware of any toolkit details of the parent application.
This runs independently until the parent application tells otherwise.

2. The glue code: a toytoolkit application widget, who has its own
rendering machinery.

3. The application written in toytoolkit.

This patch also adds new toytoolkit interfaces:
- widget_get_wl_surface()
- widget_get_last_time()
- widget_input_region_add()

Toytoolkit applications have not had a possibility to change the input
region. The frame widget (decorations) set the input region on its own
when used, otherwise the default input region of everything has been
used. If a window does not have a frame widget, it can now use
widget_input_region_add() to set a custom input region.

These are not a window methods, because a widget may lie on a different
wl_surface (sub-surface) than the window.
---
clients/.gitignore | 1 +
clients/Makefile.am | 8 +
clients/subsurfaces.c | 791 ++++++++++++++++++++++++++++++++++++++++++++++++++
clients/window.c | 30 ++
clients/window.h | 9 +
configure.ac | 3 +
6 files changed, 842 insertions(+)
create mode 100644 clients/subsurfaces.c

diff --git a/clients/.gitignore b/clients/.gitignore
index 16088e8..f925ce6 100644
--- a/clients/.gitignore
+++ b/clients/.gitignore
@@ -22,6 +22,7 @@ simple-touch
smoke
subsurface-client-protocol.h
subsurface-protocol.c
+subsurfaces
tablet-shell-client-protocol.h
tablet-shell-protocol.c
text-client-protocol.h
diff --git a/clients/Makefile.am b/clients/Makefile.am
index 5f83acd..d360174 100644
--- a/clients/Makefile.am
+++ b/clients/Makefile.am
@@ -64,6 +64,7 @@ clients_programs = \
clickdot \
transformed \
calibrator \
+ $(subsurfaces) \
$(full_gl_client_programs)

desktop_shell = weston-desktop-shell
@@ -131,6 +132,13 @@ transformed_LDADD = libtoytoolkit.la
calibrator_SOURCES = calibrator.c ../shared/matrix.c ../shared/matrix.h
calibrator_LDADD = libtoytoolkit.la

+if BUILD_SUBSURFACES_CLIENT
+subsurfaces = subsurfaces
+subsurfaces_SOURCES = subsurfaces.c
+subsurfaces_CPPFLAGS = $(AM_CPPFLAGS) $(SIMPLE_EGL_CLIENT_CFLAGS)
+subsurfaces_LDADD = libtoytoolkit.la $(SIMPLE_EGL_CLIENT_LIBS) -lm
+endif
+
if HAVE_PANGO
pango_programs = editor
editor_SOURCES = \
diff --git a/clients/subsurfaces.c b/clients/subsurfaces.c
new file mode 100644
index 0000000..eec5a3e
--- /dev/null
+++ b/clients/subsurfaces.c
@@ -0,0 +1,791 @@
+/*
+ * Copyright ? 2010 Intel Corporation
+ * Copyright ? 2011 Benjamin Franzke
+ * Copyright ? 2012-2013 Collabora, Ltd.
+ *
+ * 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 <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <cairo.h>
+#include <math.h>
+#include <assert.h>
+
+#include <linux/input.h>
+#include <wayland-client.h>
+
+#include <wayland-egl.h>
+#include <GLES2/gl2.h>
+#include <EGL/egl.h>
+
+#include "window.h"
+
+#if 0
+#define DBG(fmt, ...) \
+ fprintf(stderr, "%d:%s " fmt, __LINE__, __func__, ##__VA_ARGS__)
+#else
+#define DBG(...) do {} while (0)
+#endif
+
+static int32_t option_red_mode = 2;
+static int32_t option_triangle_mode = 2;
+static int32_t option_no_triangle;
+static int32_t option_help;
+
+static const struct weston_option options[] = {
+ { WESTON_OPTION_INTEGER, "red-mode", 'r', &option_red_mode },
+ { WESTON_OPTION_INTEGER, "triangle-mode", 't', &option_triangle_mode },
+ { WESTON_OPTION_BOOLEAN, "no-triangle", 'n', &option_no_triangle },
+ { WESTON_OPTION_BOOLEAN, "help", 'h', &option_help },
+};
+
+static enum wl_subsurface_commit_mode
+int_to_mode(int32_t i)
+{
+ switch (i) {
+ case 1:
+ return WL_SUBSURFACE_COMMIT_MODE_PARENT_CACHED;
+ case 2:
+ return WL_SUBSURFACE_COMMIT_MODE_INDEPENDENT;
+ default:
+ fprintf(stderr, "error: %d is not a valid commit mode.\n", i);
+ exit(1);
+ }
+}
+
+static const char help_text[] =
+"Usage: %s [options]\n"
+"\n"
+" -r, --red-mode=MODE\t\tthe commit mode for the red sub-surface (2)\n"
+" -t, --triangle-mode=MODE\tthe commit mode for the GL sub-surface (2)\n"
+" -n, --no-triangle\t\tDo not create the GL sub-surface.\n"
+"\n"
+"The MODE is the wl_subsurface commit mode used by default for the\n"
+"given sub-surface. Valid values are the integers:\n"
+" 1\tfor parent-cached\n"
+" 2\tfor independent, i.e. free-running\n"
+"\n"
+"This program demonstrates sub-surfaces with the toytoolkit.\n"
+"The main surface contains the decorations, a green canvas, and a\n"
+"green spinner. One sub-surface is red with a red spinner. These\n"
+"are rendered with Cairo. The other sub-surface contains a spinning\n"
+"triangle rendered in EGL/GLESv2, without Cairo, i.e. it is a raw GL\n"
+"widget.\n"
+"\n"
+"The GL widget animates on its own. The spinners follow wall clock\n"
+"time and update only when their surface is repainted, so you see\n"
+"which surfaces get redrawn. The red sub-surface animates on its own,\n"
+"but can be toggled with the spacebar.\n"
+"\n"
+"Even though the sub-surfaces attempt to animate on their own, they\n"
+"are subject to the commit mode. If commit mode is parent-cached,\n"
+"they will need a commit on the main surface to actually display.\n"
+"You can trigger a main surface repaint, without a resize, by\n"
+"hovering the pointer over the title bar buttons.\n"
+"\n"
+"Resizing will temporarily toggle the commit mode of all sub-surfaces\n"
+"to guarantee synchronized rendering on size changes. It also forces\n"
+"a repaint of all surfaces.\n"
+"\n"
+"Using -t1 -r1 is especially useful for trying to catch inconsistent\n"
+"rendering and deadlocks, since free-running sub-surfaces would\n"
+"immediately hide the problem.\n"
+"\n"
+"Key controls:\n"
+" space - toggle red sub-surface animation loop\n"
+" up - step window size shorter\n"
+" down - step window size taller\n"
+"\n";
+
+struct egl_state {
+ EGLDisplay dpy;
+ EGLContext ctx;
+ EGLConfig conf;
+};
+
+struct triangle_gl_state {
+ GLuint rotation_uniform;
+ GLuint pos;
+ GLuint col;
+};
+
+struct triangle {
+ struct egl_state *egl;
+
+ struct wl_surface *wl_surface;
+ struct wl_egl_window *egl_window;
+ EGLSurface egl_surface;
+ int width;
+ int height;
+
+ struct triangle_gl_state gl;
+
+ struct widget *widget;
+ uint32_t time;
+ struct wl_callback *frame_cb;
+};
+
+/******** Pure EGL/GLESv2/libwayland-client component: ***************/
+
+static const char *vert_shader_text =
+ "uniform mat4 rotation;\n"
+ "attribute vec4 pos;\n"
+ "attribute vec4 color;\n"
+ "varying vec4 v_color;\n"
+ "void main() {\n"
+ " gl_Position = rotation * pos;\n"
+ " v_color = color;\n"
+ "}\n";
+
+static const char *frag_shader_text =
+ "precision mediump float;\n"
+ "varying vec4 v_color;\n"
+ "void main() {\n"
+ " gl_FragColor = v_color;\n"
+ "}\n";
+
+static void
+egl_print_config_info(struct egl_state *egl)
+{
+ EGLint r, g, b, a;
+
+ printf("Chosen EGL config details:\n");
+
+ printf("\tRGBA bits");
+ if (eglGetConfigAttrib(egl->dpy, egl->conf, EGL_RED_SIZE, &r) &&
+ eglGetConfigAttrib(egl->dpy, egl->conf, EGL_GREEN_SIZE, &g) &&
+ eglGetConfigAttrib(egl->dpy, egl->conf, EGL_BLUE_SIZE, &b) &&
+ eglGetConfigAttrib(egl->dpy, egl->conf, EGL_ALPHA_SIZE, &a))
+ printf(": %d %d %d %d\n", r, g, b, a);
+ else
+ printf(" unknown\n");
+
+ printf("\tswap interval range");
+ if (eglGetConfigAttrib(egl->dpy, egl->conf, EGL_MIN_SWAP_INTERVAL, &a) &&
+ eglGetConfigAttrib(egl->dpy, egl->conf, EGL_MAX_SWAP_INTERVAL, &b))
+ printf(": %d - %d\n", a, b);
+ else
+ printf(" unknown\n");
+}
+
+static struct egl_state *
+egl_state_create(struct wl_display *display)
+{
+ struct egl_state *egl;
+
+ 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, 1,
+ EGL_RENDERABLE_TYPE, EGL_OPENGL_ES2_BIT,
+ EGL_NONE
+ };
+
+ EGLint major, minor, n;
+ EGLBoolean ret;
+
+ egl = calloc(1, sizeof *egl);
+ assert(egl);
+
+ egl->dpy = eglGetDisplay(display);
+ assert(egl->dpy);
+
+ ret = eglInitialize(egl->dpy, &major, &minor);
+ assert(ret == EGL_TRUE);
+ ret = eglBindAPI(EGL_OPENGL_ES_API);
+ assert(ret == EGL_TRUE);
+
+ ret = eglChooseConfig(egl->dpy, config_attribs, &egl->conf, 1, &n);
+ assert(ret && n == 1);
+
+ egl->ctx = eglCreateContext(egl->dpy, egl->conf,
+ EGL_NO_CONTEXT, context_attribs);
+ assert(egl->ctx);
+ egl_print_config_info(egl);
+
+ return egl;
+}
+
+static void
+egl_state_destroy(struct egl_state *egl)
+{
+ /* Required, otherwise segfault in egl_dri2.c: dri2_make_current()
+ * on eglReleaseThread(). */
+ eglMakeCurrent(egl->dpy, EGL_NO_SURFACE, EGL_NO_SURFACE,
+ EGL_NO_CONTEXT);
+
+ eglTerminate(egl->dpy);
+ eglReleaseThread();
+ free(egl);
+}
+
+static void
+egl_make_swapbuffers_nonblock(struct egl_state *egl)
+{
+ EGLint a = EGL_MIN_SWAP_INTERVAL;
+ EGLint b = EGL_MAX_SWAP_INTERVAL;
+
+ if (!eglGetConfigAttrib(egl->dpy, egl->conf, a, &a) ||
+ !eglGetConfigAttrib(egl->dpy, egl->conf, b, &b)) {
+ fprintf(stderr, "warning: swap interval range unknown\n");
+ } else
+ if (a > 0) {
+ fprintf(stderr, "warning: minimum swap interval is %d, "
+ "while 0 is required to not deadlock on resize.\n", a);
+ }
+
+ /*
+ * We rely on the Wayland compositor to sync to vblank anyway.
+ * We just need to be able to call eglSwapBuffers() without the
+ * risk of waiting for a frame callback in it.
+ */
+ if (!eglSwapInterval(egl->dpy, 0)) {
+ fprintf(stderr, "error: eglSwapInterval() failed.\n");
+ }
+}
+
+static GLuint
+create_shader(const char *source, GLenum shader_type)
+{
+ GLuint shader;
+ GLint status;
+
+ shader = glCreateShader(shader_type);
+ assert(shader != 0);
+
+ glShaderSource(shader, 1, (const char **) &source, NULL);
+ glCompileShader(shader);
+
+ glGetShaderiv(shader, GL_COMPILE_STATUS, &status);
+ if (!status) {
+ char log[1000];
+ GLsizei len;
+ glGetShaderInfoLog(shader, 1000, &len, log);
+ fprintf(stderr, "Error: compiling %s: %*s\n",
+ shader_type == GL_VERTEX_SHADER ? "vertex" : "fragment",
+ len, log);
+ exit(1);
+ }
+
+ return shader;
+}
+
+static void
+triangle_init_gl(struct triangle_gl_state *trigl)
+{
+ GLuint frag, vert;
+ GLuint program;
+ GLint status;
+
+ frag = create_shader(frag_shader_text, GL_FRAGMENT_SHADER);
+ vert = create_shader(vert_shader_text, GL_VERTEX_SHADER);
+
+ program = glCreateProgram();
+ glAttachShader(program, frag);
+ glAttachShader(program, vert);
+ glLinkProgram(program);
+
+ glGetProgramiv(program, GL_LINK_STATUS, &status);
+ if (!status) {
+ char log[1000];
+ GLsizei len;
+ glGetProgramInfoLog(program, 1000, &len, log);
+ fprintf(stderr, "Error: linking:\n%*s\n", len, log);
+ exit(1);
+ }
+
+ glUseProgram(program);
+
+ trigl->pos = 0;
+ trigl->col = 1;
+
+ glBindAttribLocation(program, trigl->pos, "pos");
+ glBindAttribLocation(program, trigl->col, "color");
+ glLinkProgram(program);
+
+ trigl->rotation_uniform = glGetUniformLocation(program, "rotation");
+}
+
+static void
+triangle_draw(const struct triangle_gl_state *trigl, uint32_t time)
+{
+ static const GLfloat verts[3][2] = {
+ { -0.5, -0.5 },
+ { 0.5, -0.5 },
+ { 0, 0.5 }
+ };
+ static const GLfloat colors[3][3] = {
+ { 1, 0, 0 },
+ { 0, 1, 0 },
+ { 0, 0, 1 }
+ };
+ GLfloat angle;
+ GLfloat rotation[4][4] = {
+ { 1, 0, 0, 0 },
+ { 0, 1, 0, 0 },
+ { 0, 0, 1, 0 },
+ { 0, 0, 0, 1 }
+ };
+ static const int32_t speed_div = 5;
+
+ angle = (time / speed_div) % 360 * M_PI / 180.0;
+ rotation[0][0] = cos(angle);
+ rotation[0][2] = sin(angle);
+ rotation[2][0] = -sin(angle);
+ rotation[2][2] = cos(angle);
+
+ glUniformMatrix4fv(trigl->rotation_uniform, 1, GL_FALSE,
+ (GLfloat *) rotation);
+
+ glClearColor(0.0, 0.0, 0.0, 0.5);
+ glClear(GL_COLOR_BUFFER_BIT);
+
+ glVertexAttribPointer(trigl->pos, 2, GL_FLOAT, GL_FALSE, 0, verts);
+ glVertexAttribPointer(trigl->col, 3, GL_FLOAT, GL_FALSE, 0, colors);
+ glEnableVertexAttribArray(trigl->pos);
+ glEnableVertexAttribArray(trigl->col);
+
+ glDrawArrays(GL_TRIANGLES, 0, 3);
+
+ glDisableVertexAttribArray(trigl->pos);
+ glDisableVertexAttribArray(trigl->col);
+}
+
+static void
+triangle_frame_callback(void *data, struct wl_callback *callback,
+ uint32_t time);
+
+static const struct wl_callback_listener triangle_frame_listener = {
+ triangle_frame_callback
+};
+
+static void
+triangle_frame_callback(void *data, struct wl_callback *callback,
+ uint32_t time)
+{
+ struct triangle *tri = data;
+
+ DBG("%stime %u\n", callback ? "" : "artificial ", time);
+ assert(callback == tri->frame_cb);
+ tri->time = time;
+
+ if (callback)
+ wl_callback_destroy(callback);
+
+ glViewport(0, 0, tri->width, tri->height);
+
+ triangle_draw(&tri->gl, tri->time);
+
+ tri->frame_cb = wl_surface_frame(tri->wl_surface);
+ wl_callback_add_listener(tri->frame_cb, &triangle_frame_listener, tri);
+
+ eglSwapBuffers(tri->egl->dpy, tri->egl_surface);
+}
+
+static void
+triangle_create_egl_surface(struct triangle *tri, int width, int height)
+{
+ EGLBoolean ret;
+
+ tri->wl_surface = widget_get_wl_surface(tri->widget);
+ tri->egl_window = wl_egl_window_create(tri->wl_surface, width, height);
+ tri->egl_surface = eglCreateWindowSurface(tri->egl->dpy,
+ tri->egl->conf,
+ tri->egl_window, NULL);
+
+ ret = eglMakeCurrent(tri->egl->dpy, tri->egl_surface,
+ tri->egl_surface, tri->egl->ctx);
+ assert(ret == EGL_TRUE);
+
+ egl_make_swapbuffers_nonblock(tri->egl);
+ triangle_init_gl(&tri->gl);
+}
+
+/********* The widget code interfacing the toolkit agnostic code: **********/
+
+static void
+triangle_resize_handler(struct widget *widget,
+ int32_t width, int32_t height, void *data)
+{
+ struct triangle *tri = data;
+
+ DBG("to %dx%d\n", width, height);
+ tri->width = width;
+ tri->height = height;
+
+ if (tri->egl_surface) {
+ wl_egl_window_resize(tri->egl_window, width, height, 0, 0);
+ } else {
+ triangle_create_egl_surface(tri, width, height);
+ triangle_frame_callback(tri, NULL, 0);
+ }
+}
+
+static void
+triangle_redraw_handler(struct widget *widget, void *data)
+{
+ struct triangle *tri = data;
+ int w, h;
+
+ wl_egl_window_get_attached_size(tri->egl_window, &w, &h);
+
+ DBG("previous %dx%d, new %dx%d\n", w, h, tri->width, tri->height);
+
+ /* If size is not changing, do not redraw ahead of time.
+ * That would risk blocking in eglSwapbuffers().
+ */
+ if (w == tri->width && h == tri->height)
+ return;
+
+ if (tri->frame_cb) {
+ wl_callback_destroy(tri->frame_cb);
+ tri->frame_cb = NULL;
+ }
+ triangle_frame_callback(tri, NULL, tri->time);
+}
+
+static void
+set_empty_input_region(struct widget *widget, struct display *display)
+{
+ struct wl_compositor *compositor;
+ struct wl_surface *surface;
+ struct wl_region *region;
+
+ compositor = display_get_compositor(display);
+ surface = widget_get_wl_surface(widget);
+ region = wl_compositor_create_region(compositor);
+ wl_surface_set_input_region(surface, region);
+ wl_region_destroy(region);
+}
+
+static struct triangle *
+triangle_create(struct window *window, struct egl_state *egl)
+{
+ struct triangle *tri;
+
+ tri = calloc(1, sizeof *tri);
+
+ tri->egl = egl;
+ tri->widget = window_add_subsurface(window, tri,
+ int_to_mode(option_triangle_mode));
+ widget_set_resize_handler(tri->widget, triangle_resize_handler);
+ widget_set_redraw_handler(tri->widget, triangle_redraw_handler);
+
+ set_empty_input_region(tri->widget, window_get_display(window));
+
+ return tri;
+}
+
+static void
+triangle_destroy(struct triangle *tri)
+{
+ if (tri->egl_surface)
+ eglDestroySurface(tri->egl->dpy, tri->egl_surface);
+
+ if (tri->egl_window)
+ wl_egl_window_destroy(tri->egl_window);
+
+ widget_destroy(tri->widget);
+ free(tri);
+}
+
+/************** The toytoolkit application code: *********************/
+
+struct demoapp {
+ struct display *display;
+ struct window *window;
+ struct widget *widget;
+ struct widget *subsurface;
+
+ struct egl_state *egl;
+ struct triangle *triangle;
+
+ int animate;
+};
+
+static void
+draw_spinner(cairo_t *cr, const struct rectangle *rect, uint32_t time)
+{
+ double cx, cy, r, angle;
+ unsigned t;
+
+ cx = rect->x + rect->width / 2;
+ cy = rect->y + rect->height / 2;
+ r = (rect->width < rect->height ? rect->width : rect->height) * 0.3;
+ t = time % 2000;
+ angle = t * (M_PI / 500.0);
+
+ cairo_set_line_width(cr, 4.0);
+
+ if (t < 1000)
+ cairo_arc(cr, cx, cy, r, 0.0, angle);
+ else
+ cairo_arc(cr, cx, cy, r, angle, 0.0);
+
+ cairo_stroke(cr);
+}
+
+static void
+sub_redraw_handler(struct widget *widget, void *data)
+{
+ struct demoapp *app = data;
+ cairo_t *cr;
+ struct rectangle allocation;
+ uint32_t time;
+
+ widget_get_allocation(app->subsurface, &allocation);
+
+ cr = widget_cairo_create(widget);
+ cairo_set_operator(cr, CAIRO_OPERATOR_SOURCE);
+
+ cairo_set_source_rgba(cr, 0.9, 0.0, 0.9, 1.0);
+ cairo_paint(cr);
+
+ cairo_rectangle(cr,
+ allocation.x,
+ allocation.y,
+ allocation.width,
+ allocation.height);
+ cairo_clip(cr);
+
+ cairo_set_source_rgba(cr, 0.8, 0, 0, 0.8);
+ cairo_paint(cr);
+
+ time = widget_get_last_time(widget);
+ cairo_set_source_rgba(cr, 1.0, 0.5, 0.5, 1.0);
+ draw_spinner(cr, &allocation, time);
+
+ cairo_destroy(cr);
+
+ if (app->animate)
+ widget_schedule_redraw(app->subsurface);
+ DBG("%dx%d @ %d,%d, last time %u\n",
+ allocation.width, allocation.height,
+ allocation.x, allocation.y, time);
+}
+
+static void
+sub_resize_handler(struct widget *widget,
+ int32_t width, int32_t height, void *data)
+{
+ DBG("%dx%d\n", width, height);
+ widget_input_region_add(widget, NULL);
+}
+
+static void
+redraw_handler(struct widget *widget, void *data)
+{
+ struct demoapp *app = data;
+ cairo_t *cr;
+ struct rectangle allocation;
+ uint32_t time;
+
+ widget_get_allocation(app->widget, &allocation);
+
+ cr = widget_cairo_create(widget);
+ cairo_set_operator(cr, CAIRO_OPERATOR_SOURCE);
+ cairo_rectangle(cr,
+ allocation.x,
+ allocation.y,
+ allocation.width,
+ allocation.height);
+ cairo_set_source_rgba(cr, 0, 0.8, 0, 0.8);
+ cairo_fill(cr);
+
+ time = widget_get_last_time(widget);
+ cairo_set_source_rgba(cr, 0.5, 1.0, 0.5, 1.0);
+ draw_spinner(cr, &allocation, time);
+
+ cairo_destroy(cr);
+
+ DBG("%dx%d @ %d,%d, last time %u\n",
+ allocation.width, allocation.height,
+ allocation.x, allocation.y, time);
+}
+
+static void
+resize_handler(struct widget *widget,
+ int32_t width, int32_t height, void *data)
+{
+ struct demoapp *app = data;
+ struct rectangle area;
+ int side, h;
+
+ widget_get_allocation(widget, &area);
+
+ side = area.width < area.height ? area.width / 2 : area.height / 2;
+ h = area.height - side;
+
+ widget_set_allocation(app->subsurface,
+ area.x + area.width - side,
+ area.y,
+ side, h);
+
+ if (app->triangle) {
+ widget_set_allocation(app->triangle->widget,
+ area.x + area.width - side,
+ area.y + h,
+ side, side);
+ }
+
+ DBG("green %dx%d, red %dx%d, GL %dx%d\n",
+ area.width, area.height, side, h, side, side);
+}
+
+static void
+keyboard_focus_handler(struct window *window,
+ struct input *device, void *data)
+{
+ struct demoapp *app = data;
+
+ window_schedule_redraw(app->window);
+}
+
+static void
+key_handler(struct window *window, struct input *input, uint32_t time,
+ uint32_t key, uint32_t sym,
+ enum wl_keyboard_key_state state, void *data)
+{
+ struct demoapp *app = data;
+ struct rectangle winrect;
+
+ if (state == WL_KEYBOARD_KEY_STATE_RELEASED)
+ return;
+
+ switch (sym) {
+ case XKB_KEY_space:
+ app->animate = !app->animate;
+ window_schedule_redraw(window);
+ break;
+ case XKB_KEY_Up:
+ window_get_allocation(window, &winrect);
+ winrect.height -= 100;
+ if (winrect.height < 150)
+ winrect.height = 150;
+ window_schedule_resize(window, winrect.width, winrect.height);
+ break;
+ case XKB_KEY_Down:
+ window_get_allocation(window, &winrect);
+ winrect.height += 100;
+ if (winrect.height > 600)
+ winrect.height = 600;
+ window_schedule_resize(window, winrect.width, winrect.height);
+ break;
+ case XKB_KEY_Escape:
+ display_exit(app->display);
+ break;
+ }
+}
+
+static struct demoapp *
+demoapp_create(struct display *display)
+{
+ struct demoapp *app;
+
+ app = calloc(1, sizeof *app);
+ if (!app)
+ return NULL;
+
+ app->egl = egl_state_create(display_get_display(display));
+
+ app->display = display;
+ display_set_user_data(app->display, app);
+
+ app->window = window_create(app->display);
+ app->widget = frame_create(app->window, app);
+ window_set_title(app->window, "Wayland Sub-surface Demo");
+
+ window_set_key_handler(app->window, key_handler);
+ window_set_user_data(app->window, app);
+ window_set_keyboard_focus_handler(app->window, keyboard_focus_handler);
+
+ widget_set_redraw_handler(app->widget, redraw_handler);
+ widget_set_resize_handler(app->widget, resize_handler);
+
+ app->subsurface = window_add_subsurface(app->window, app,
+ int_to_mode(option_red_mode));
+ widget_set_redraw_handler(app->subsurface, sub_redraw_handler);
+ widget_set_resize_handler(app->subsurface, sub_resize_handler);
+
+ if (app->egl && !option_no_triangle)
+ app->triangle = triangle_create(app->window, app->egl);
+
+ /* minimum size */
+ widget_schedule_resize(app->widget, 100, 100);
+
+ /* initial size */
+ widget_schedule_resize(app->widget, 400, 300);
+
+ app->animate = 1;
+
+ return app;
+}
+
+static void
+demoapp_destroy(struct demoapp *app)
+{
+ if (app->triangle)
+ triangle_destroy(app->triangle);
+
+ if (app->egl)
+ egl_state_destroy(app->egl);
+
+ widget_destroy(app->subsurface);
+ widget_destroy(app->widget);
+ window_destroy(app->window);
+ free(app);
+}
+
+int
+main(int argc, char *argv[])
+{
+ struct display *display;
+ struct demoapp *app;
+
+ parse_options(options, ARRAY_LENGTH(options), &argc, argv);
+ if (option_help) {
+ printf(help_text, argv[0]);
+ return 0;
+ }
+
+ display = display_create(&argc, argv);
+ if (display == NULL) {
+ fprintf(stderr, "failed to create display: %m\n");
+ return -1;
+ }
+
+ app = demoapp_create(display);
+
+ display_run(display);
+
+ demoapp_destroy(app);
+ display_destroy(display);
+
+ return 0;
+}
diff --git a/clients/window.c b/clients/window.c
index 4730c89..ca6fb69 100644
--- a/clients/window.c
+++ b/clients/window.c
@@ -197,6 +197,7 @@ struct surface {
struct widget *widget;
int redraw_needed;
struct wl_callback *frame_cb;
+ uint32_t last_time;

struct rectangle allocation;
struct rectangle server_allocation;
@@ -1543,6 +1544,33 @@ widget_cairo_create(struct widget *widget)
return cr;
}

+struct wl_surface *
+widget_get_wl_surface(struct widget *widget)
+{
+ return widget->surface->surface;
+}
+
+uint32_t
+widget_get_last_time(struct widget *widget)
+{
+ return widget->surface->last_time;
+}
+
+void
+widget_input_region_add(struct widget *widget, const struct rectangle *rect)
+{
+ struct wl_compositor *comp = widget->window->display->compositor;
+ struct surface *surface = widget->surface;
+
+ if (!surface->input_region)
+ surface->input_region = wl_compositor_create_region(comp);
+
+ if (rect) {
+ wl_region_add(surface->input_region,
+ rect->x, rect->y, rect->width, rect->height);
+ }
+}
+
void
widget_set_resize_handler(struct widget *widget,
widget_resize_handler_t handler)
@@ -3457,6 +3485,8 @@ frame_callback(void *data, struct wl_callback *callback, uint32_t time)
wl_callback_destroy(callback);
surface->frame_cb = NULL;

+ surface->last_time = time;
+
if (surface->redraw_needed || surface->window->redraw_needed)
window_schedule_redraw_task(surface->window);
}
diff --git a/clients/window.h b/clients/window.h
index 30466c0..ccba1cf 100644
--- a/clients/window.h
+++ b/clients/window.h
@@ -390,6 +390,15 @@ widget_get_user_data(struct widget *widget);
cairo_t *
widget_cairo_create(struct widget *widget);

+struct wl_surface *
+widget_get_wl_surface(struct widget *widget);
+
+uint32_t
+widget_get_last_time(struct widget *widget);
+
+void
+widget_input_region_add(struct widget *widget, const struct rectangle *rect);
+
void
widget_set_redraw_handler(struct widget *widget,
widget_redraw_handler_t handler);
diff --git a/configure.ac b/configure.ac
index 74eb892..a7a0430 100644
--- a/configure.ac
+++ b/configure.ac
@@ -261,6 +261,9 @@ AM_CONDITIONAL(HAVE_PANGO, test "x$have_pango" = "xyes")
AM_CONDITIONAL(BUILD_FULL_GL_CLIENTS,
test x$cairo_modules = "xcairo-gl" -a "x$have_cairo_egl" = "xyes" -a "x$enable_egl" = "xyes")

+AM_CONDITIONAL(BUILD_SUBSURFACES_CLIENT,
+ [test '(' "x$have_cairo_egl" != "xyes" -o "x$cairo_modules" = "xcairo-glesv2" ')' -a "x$enable_simple_egl_clients" = "xyes"])
+
AM_CONDITIONAL(ENABLE_DESKTOP_SHELL, true)

AC_ARG_ENABLE(tablet-shell,
--
1.7.12.4
Pekka Paalanen
2013-02-22 15:07:54 UTC
Permalink
This post might be inappropriate. Click to display it.
Giulio Camuffo
2013-02-23 11:05:20 UTC
Permalink
Hi
Post by Pekka Paalanen
- a demo client with window decorations stitched from 4
non-overlapping sub-surfaces
I tought i may write here the problems i've seen when doing window
decorations this way in QtWayland:
(http://qt.gitorious.org/~giucam/qt/giucam-qtwayland/commits/csd-subsurface-collage
warning: very very WIP code, will rebase.)

- you click on the window decoration to move the shell surface, but
the shell surface you're trying to move has the main surface as its
surface so the condition "|| ws->seat.pointer->focus ==
&shsurf->surface->surface" in weston's shell_surface_move() in
src/shell.c will fail, and you won't be able to move the window. Same
thing for resizing.
solution: make it check if ws->seat.pointer->focus is a subsurface of
shsurf->surface->surface?

- if you ask the shell to maximize the shell_surface it will use the
main surface as the size, so if you apply blindly the width and height
given by the configure the decorations will be outside the wanted
perimeter. You can account for the decorations size in the client's
configure and reduce the width and height, but the main surface
position will still be at 0,0 so the top decoration will still be
below the panel, the left one will be outside the screen and you will
have empty space below and on the right of the window.
solution: make it use the bounding box of the surface + subsurfaces
instead of the surface's size?


Giulio
Pekka Paalanen
2013-02-23 20:41:31 UTC
Permalink
On Sat, 23 Feb 2013 12:05:20 +0100
Post by Giulio Camuffo
Hi
Post by Pekka Paalanen
- a demo client with window decorations stitched from 4
non-overlapping sub-surfaces
I tought i may write here the problems i've seen when doing window
(http://qt.gitorious.org/~giucam/qt/giucam-qtwayland/commits/csd-subsurface-collage
warning: very very WIP code, will rebase.)
- you click on the window decoration to move the shell surface, but
the shell surface you're trying to move has the main surface as its
surface so the condition "|| ws->seat.pointer->focus ==
&shsurf->surface->surface" in weston's shell_surface_move() in
src/shell.c will fail, and you won't be able to move the window. Same
thing for resizing.
solution: make it check if ws->seat.pointer->focus is a subsurface of
shsurf->surface->surface?
Yeah, I guess we should always look up the main surface, when
something happens wrt. to a sub-surface.
Post by Giulio Camuffo
- if you ask the shell to maximize the shell_surface it will use the
main surface as the size, so if you apply blindly the width and height
given by the configure the decorations will be outside the wanted
perimeter. You can account for the decorations size in the client's
configure and reduce the width and height, but the main surface
position will still be at 0,0 so the top decoration will still be
below the panel, the left one will be outside the screen and you will
have empty space below and on the right of the window.
solution: make it use the bounding box of the surface + subsurfaces
instead of the surface's size?
Oh yes, that is a missing feature: a function to compute the window
rectangle from the set of wl_surfaces. And then use that in
positioning computations in shell.

I did expect to hit these kinds of problems, when I would start
experimenting with the stitched decorations.


Thanks,
pq
Pekka Paalanen
2013-02-25 14:36:21 UTC
Permalink
On Fri, 22 Feb 2013 17:07:44 +0200
Post by Pekka Paalanen
- destroying the wl_surface of a wl_subsurface will prevent
destroying the wl_subsurface, fix this, and adjust the
protocol description accordingly
- double-buffering of sub-surface z-order changes
- a demo client with window decorations stitched from 4
non-overlapping sub-surfaces
- fix eglSwapInterval in Mesa (krh?)
- fix full-surface alpha for surfaces that have sub-surfaces
- in subsurfaces demo, put the GL widget into its own thread, so
it is truely as concurrent as it can get.
- reimplement the "black surface" in shell as a sub-surface
http://cgit.collabora.com/git/user/pq/weston.git/log/?h=subsurface-v2
The above todo still stands, while...
Post by Pekka Paalanen
- fix frame throttling in subsurfaces demo
- subsurfaces demo behaves strangely under the DRM backend of
Weston, debug and fix it. A timestamp issue?
- even with three buffers in a pool, subsurfaces may still run
out of free buffers. When that happens, "all buffers are held
by the server" error message appears, and sub-surfaces may be
out of sync. Related to frame throttling.
- glitched rendering of the GL widget in subsurfaces when Up or
Down key held pressed, might be related to frame throttling.
these I think I nailed with two new patches that I already pushed to
subsurface-v2 branch:

compositor: fix buffer reference leak in commit_from_cache
window: throttle resizing to the main surface

I have tested it even with early buffer release hacked out from Weston.


Thanks,
pq
Pekka Paalanen
2013-02-27 10:15:15 UTC
Permalink
On Fri, 22 Feb 2013 17:07:44 +0200
Post by Pekka Paalanen
- What should wl_subsurface.destroy do? Nothing, or reset the
wl_surface's role? Currently it resets the role, since it was
easy to implement.
...
Post by Pekka Paalanen
- destroying the wl_surface of a wl_subsurface will prevent
destroying the wl_subsurface, fix this, and adjust the
protocol description accordingly
I'll open up this problem a bit.

The nearest correspondence to wl_subsurface we have in the current
upstream protocol is wl_shell_surface. They are both defined as
additional interfaces to a wl_surface, and provide a wl_surface role.
Surface roles are always exclusive. A wl_surface can be at most one of
the following:
- a sub-surface
- a shell surface, e.g. a top-level window, tooltip, ...
- a pointer cursor
- a drag icon

Sub-surface and shell surface have a protocol interface associated with
them, cursor and drag icons do not.

In weston's code, a role is identified by what is plugged into struct
weston_surface::private and ::configure fields. In practice, a surface
without a role is never visible and does not take part in input
dispatching.

There are currently different kinds of implementations for removing a
role from a wl_surface. When a pointer cursor or a drag icon wl_surface
stops being used as such, it is returned to the no-role state, allowing
re-use for another purpose if the client wants to. However, I believe
the practice is to not re-use wl_surfaces, since they are cheap to
create and destroy protocol-wise.

wl_shell_surface role on the other hand is permanent. The role is
assigned by creating a wl_shell_surface object. Destroying the
wl_shell_surface object is a no-op, because it has no protocol for
destroy. Therefore there is no way to remove the role. Also, the
wl_shell_surface object is implicitly destroyed in the server, when the
wl_surface is destroyed. The specification requires wl_shell_surface to
be destroyed before the wl_surface, although I think doing it in the
opposite order does not directly lead to a protocol error. Doing it in
the specified order however prevents sending requests to a destroyed
object.

(I now believe that not defining protocol for wl_shell_surface.destroy
was a mistake, made by me.)

In sub-surfaces RFC v2, wl_subsurface object is implicitly destroyed
when the wl_surface object is destroyed. As wl_subsurface.destroy has
protocol defined, destroying the wl_surface first and the wl_subsurface
second will lead to an immediate protocol error, since the destroy
request is being sent to an already destroyed object. Therefore
wl_subsurface must be destroyed first. When wl_subsurface is destroyed,
the wl_surface returns to the no-role state. (Btw. this allows
reparenting of sub-surfaces.)

As you see, there is quite some inconsistency here.

When looking at what we have, cursor surfaces and drag icons are part
of the Wayland core protocol, and wl_subsurface will be a core
extension. Among these we can have consistency: the role can be
dismissed, and the wl_surface returns to the no-role state, ready to be
re-purposed if a client so wants.

wl_shell_surface is known to have design problems anyway, so we can
just punt it.

Finally, I think I'm going to lift the restriction that wl_subsurface
must always be destroyed before the wl_surface. If the wl_surface is
destroyed before the wl_subsurface, the wl_subsurface will become
"inert".

Inert is a term we will be employing with global Wayland
interfaces/objects. An inert protocol object exists, but ignores all
requests except destroy, and does not pass any events. It is only
waiting to be destroyed. With globals, this will be used when a global
is going away: all objects created from the global interface will
become inert. The purpose of inert objects is to avoid races. Before a
client gets to process the removal of a global interface, it may send
requests to objects created from it. These requests racing with the
destruction should not cause protocol errors but be silently ignored.

For the implementation in Weston, supporting inert objects means we
will better detach wl_resource from the corresponding weston data
structure. wl_resource will be a pointer than can be NULL, instead of
embedded. I think that will be nice and tidy, and we need to do that
with weston_surface one day, too, to support window closing animations
and server-only surfaces better.

It seems we have a Wayland protocol design pattern emerging here,
extending object's interface:
- a global for creating an extension (wl_subcompositor, wl_shell)
- an extension object (wl_subsurface, wl_shell_surface)
- destroy semantics (wl_surface.destroy makes the wl_subsurface inert)

Another design pattern is one-shot reply: wl_callback for
wl_display.sync and wl_surface.frame, and wl_probe_result for
wl_shell_surface.probe_area [1].

And the most generic design pattern, or rather a rule of thumb: always
define protocol for destroy. Only in very exceptional cases, like
one-shot reply objects, the destroy request will do more harm than good.

We also have Wayland protocol usage patterns:
- glitch-free maximization and fullscreening [2]
- glitch-free resizing of a window with sub-surfaces

I feel we should document these somewhere. :-)

Comments?


Thanks,
pq

[1] http://lists.freedesktop.org/archives/wayland-devel/2013-February/007340.html
[2] http://lists.freedesktop.org/archives/wayland-devel/2013-February/007650.html
Daniel Stone
2013-03-06 20:58:58 UTC
Permalink
Hi,
Post by Pekka Paalanen
The biggest improvement over v1 is that we have some thought-out
commit behaviours. It is possible to resize a window so that all
surfaces stay in sync on screen, and it is also possible to have
sub-surfaces running on their own (i.e. without commit the
parent surface, too).
One thing I noticed is that a subsurface is defined to be mapped as soon as
it has a valid buffer attached, and its parent is mapped. Presumably this
means that if:
- the parent window is already mapped
- the child is brought into being, and attaches a buffer as part of this
- a subsurface is created for the child

then the subsurface will be mapped before the parent has had a chance to
either repaint itself, or even call set_position. So maybe that's
something we need to look at for the embed (as opposed to decoration) case,
either with an explicit map request, or just adding 'and set_position has
been called' to the list of map conditions.

It's also undefined what the commit_mode is for a new wl_subsurface object
- and having the commit mode initially set to parent-cached might be enough
to get around the map issue. Also, if the commit mode is parent-cached,
changes are made which are put into the cache, and commit mode is then
changed to immediate before the parent is committed, will the cached
changes be committed? It's about the only semantics I can think of which
really make sense for this.

This might all be fixed in the implementation, but it's not clear from the
spec.

- Input events still very much unexplored. The demo just punts
Post by Pekka Paalanen
them.
At the very least, we're going to need flags on enter/leave events to note
when the pointer left towards a subsurface.

Cheers,
Daniel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20130306/e708e4fb/attachment.html>
Pekka Paalanen
2013-03-07 10:32:14 UTC
Permalink
On Wed, 6 Mar 2013 12:58:58 -0800
Post by Daniel Stone
Hi,
Post by Pekka Paalanen
The biggest improvement over v1 is that we have some thought-out
commit behaviours. It is possible to resize a window so that all
surfaces stay in sync on screen, and it is also possible to have
sub-surfaces running on their own (i.e. without commit the
parent surface, too).
One thing I noticed is that a subsurface is defined to be mapped as soon as
it has a valid buffer attached, and its parent is mapped. Presumably this
- the parent window is already mapped
- the child is brought into being, and attaches a buffer as part of this
- a subsurface is created for the child
then the subsurface will be mapped before the parent has had a chance to
either repaint itself, or even call set_position. So maybe that's
something we need to look at for the embed (as opposed to decoration) case,
either with an explicit map request, or just adding 'and set_position has
been called' to the list of map conditions.
I think the parent-cached commit mode, which also happens to be the
default, takes care of that:
1. main surface A created and mapped, as usual, is on screen
2. create wl_surface B
3. make B a sub-surface, with A as its parent, gets parent-cached
commit mode
4. start feeding frames continously to wl_surface B, calling B.commit
and all
5. send wl_subsurface.set_position for B
6. update A, and A.commit

You can do steps 3-5 in any possible order, and it is guaranteed that
the surface B will be mapped only at step 6.

To have an effective attach (on B), you have to B.commit, but what
B.commit does depends on the commit mode, so it is not effective
until the parent is committed while in parent-cached mode.

Also, a wl_surface without any role will not get mapped under desktop
shell. Is it ok to rely on the shell like this, or should we support
shells that map "anonymous" surfaces?

OTOH, is it really too much to require, that you create the
wl_subsurface before committing the first buffer to the
sub-surface's wl_surface?
Post by Daniel Stone
It's also undefined what the commit_mode is for a new wl_subsurface object
- and having the commit mode initially set to parent-cached might be enough
to get around the map issue. Also, if the commit mode is parent-cached,
changes are made which are put into the cache, and commit mode is then
changed to immediate before the parent is committed, will the cached
changes be committed? It's about the only semantics I can think of which
really make sense for this.
Yes, I flush the cache always, when a commit-to-current-state happens.
I should add a mention about that.

I'm very happy you saw that corner case from the spec ;-)
Post by Daniel Stone
This might all be fixed in the implementation, but it's not clear from the
spec.
Default commit mode is mentioned here:
http://cgit.collabora.com/git/user/pq/weston.git/tree/protocol/subsurface.xml?h=subsurface-v2#n207

But maybe better move it to the general description.
Post by Daniel Stone
- Input events still very much unexplored. The demo just punts
Post by Pekka Paalanen
them.
At the very least, we're going to need flags on enter/leave events to note
when the pointer left towards a subsurface.
Why exactly?

The client will receive an enter for the sub-surface as the very next
thing.

If it is about the race of an app repainting between leave and enter,
we have the same problem elsewhere, too, and should solve it globally,
and I'd like krh's input on that. I can imagine we will hit the same
problem in some shell use cases, too, e.g. keyboard focus entering a
menu or tooltip surface.


Thanks,
pq
Bill Spitzak
2013-03-07 21:26:52 UTC
Permalink
Post by Pekka Paalanen
Post by Daniel Stone
At the very least, we're going to need flags on enter/leave events to note
when the pointer left towards a subsurface.
Why exactly?
The client will receive an enter for the sub-surface as the very next
thing.
If it is about the race of an app repainting between leave and enter,
we have the same problem elsewhere, too, and should solve it globally,
and I'd like krh's input on that. I can imagine we will hit the same
problem in some shell use cases, too, e.g. keyboard focus entering a
menu or tooltip surface.
I think this is another example that can be fixed by getting rid of the
goal of making the server send exactly matching enter/leave events.

If the old and new surface belong to the same client, the server sends
only the "enter" event to the new surface. The client can figure out
exactly what happened and at no time is it unsure whether the cursor is
pointing at one of it's surfaces.

Full proposal:

All pointer motion produces one of three events, somewhat misleadingly
labelled MOVE, ENTER, LEAVE. All of them have the pointer x/y position
in the coordinate space of the target surface. MOVE has an accurate
timestamp as it always matches an actual device event. The others may
have inaccurate information as they are spontaneously generated by other
actions by clients.

When a pointer moves, a MOVE event is sent to exactly one surface.
Normally this is the top-most one with a non-transparent pixel under the
cursor, but there are grabs and other state that can send it to a
different surface.

The compositor also knows of all kinds of other reasons a pointer may
suddenly be "in" a surface even without movement. This includes
reordering and creation/destruction of surfaces, creation of pointers,
and changes in grab. In this case it sends an ENTER event to the same
surface that would have gotten the MOVE event if the movement ended at
the current pointer position. Compositors can spuriously send ENTER at
any time so that they do not have to track huge amounts of state, and
clients must be prepared to handle any number of ENTER events.

The compositor keeps track of the surface it last sent an ENTER or MOVE
to, and when sending an ENTER or MOVE to a different surface, and they
belong to different clients, it sends a LEAVE event to the previous
surface. A LEAVE may also be sent if a pointer is destroyed.

Kai-Uwe Behrmann
2013-03-07 06:25:17 UTC
Permalink
Hello Pekka,
Post by Pekka Paalanen
- A totally new demo client presenting sub-surfaces, including
Cairo-image rendered window with a pure EGL/GL widget in a
sub-surface, running independently, but still without glitches
on resize (sans bugs).
http://people.collabora.com/~pq/subsurface-2.png
The above example shows non overlapping regions. For a more throughout
test, I would expect overlapping regions with and without alpha to test
Z-order behaviour.
Post by Pekka Paalanen
The biggest improvement over v1 is that we have some thought-out
commit behaviours. It is possible to resize a window so that all
surfaces stay in sync on screen, and it is also possible to have
sub-surfaces running on their own (i.e. without commit the
parent surface, too).
What is the default behaviour of repainting in regards to Z-order? Draw
all surfaces? I other words, is the possibility to have sub-surfaces
running on their own only optional?

kind regards
Kai-Uwe
Pekka Paalanen
2013-03-07 10:44:02 UTC
Permalink
On Thu, 07 Mar 2013 07:25:17 +0100
Post by Kai-Uwe Behrmann
Hello Pekka,
Post by Pekka Paalanen
- A totally new demo client presenting sub-surfaces, including
Cairo-image rendered window with a pure EGL/GL widget in a
sub-surface, running independently, but still without glitches
on resize (sans bugs).
http://people.collabora.com/~pq/subsurface-2.png
The above example shows non overlapping regions. For a more throughout
test, I would expect overlapping regions with and without alpha to test
Z-order behaviour.
There's a huuuuge amount of test code that could be written. Z-order
code is largely untested, and actually still incomplete even in the
implementation (place_above/below are effective immediately instead of
on parent commit like they should be).
Post by Kai-Uwe Behrmann
Post by Pekka Paalanen
The biggest improvement over v1 is that we have some thought-out
commit behaviours. It is possible to resize a window so that all
surfaces stay in sync on screen, and it is also possible to have
sub-surfaces running on their own (i.e. without commit the
parent surface, too).
What is the default behaviour of repainting in regards to Z-order? Draw
all surfaces? I other words, is the possibility to have sub-surfaces
running on their own only optional?
Sorry, I don't understand the question.

In v2 proposal, which does not support nesting, all sub-surfaces are
siblings. They have no effect on each other's rendering. Also, as they
are all siblings, Z-order is a completely orthogonal attribute to
commit mode or rendering.

Nesting support is coming, though.

Could you rephrase the question?


Thanks,
pq
Kai-Uwe Behrmann
2013-03-07 11:32:21 UTC
Permalink
Post by Pekka Paalanen
On Thu, 07 Mar 2013 07:25:17 +0100
Post by Kai-Uwe Behrmann
Post by Pekka Paalanen
The biggest improvement over v1 is that we have some thought-out
commit behaviours. It is possible to resize a window so that all
surfaces stay in sync on screen, and it is also possible to have
sub-surfaces running on their own (i.e. without commit the
parent surface, too).
What is the default behaviour of repainting in regards to Z-order? Draw
all surfaces? I other words, is the possibility to have sub-surfaces
running on their own only optional?
Sorry, I don't understand the question.
In v2 proposal, which does not support nesting, all sub-surfaces are
siblings. They have no effect on each other's rendering. Also, as they
are all siblings, Z-order is a completely orthogonal attribute to
commit mode or rendering.
Nesting support is coming, though.
Could you rephrase the question?
I will try.

Statement:
The flattened output of overlapping transparent regions depend on the
intented order of painting the graph during compositing.

Question:
Does your following annotation has a side effect on the painting(Z)
order of transparent content?
Post by Pekka Paalanen
Post by Kai-Uwe Behrmann
Post by Pekka Paalanen
and it is also possible to have
sub-surfaces running on their own (i.e. without commit the
parent surface, too)
kind regards
Kai-Uwe
Pekka Paalanen
2013-03-07 11:56:44 UTC
Permalink
On Thu, 07 Mar 2013 12:32:21 +0100
Post by Kai-Uwe Behrmann
Post by Pekka Paalanen
On Thu, 07 Mar 2013 07:25:17 +0100
Post by Kai-Uwe Behrmann
Post by Pekka Paalanen
The biggest improvement over v1 is that we have some thought-out
commit behaviours. It is possible to resize a window so that all
surfaces stay in sync on screen, and it is also possible to have
sub-surfaces running on their own (i.e. without commit the
parent surface, too).
What is the default behaviour of repainting in regards to Z-order? Draw
all surfaces? I other words, is the possibility to have sub-surfaces
running on their own only optional?
Sorry, I don't understand the question.
In v2 proposal, which does not support nesting, all sub-surfaces are
siblings. They have no effect on each other's rendering. Also, as they
are all siblings, Z-order is a completely orthogonal attribute to
commit mode or rendering.
Nesting support is coming, though.
Could you rephrase the question?
I will try.
The flattened output of overlapping transparent regions depend on the
intented order of painting the graph during compositing.
Does your following annotation has a side effect on the painting(Z)
order of transparent content?
Post by Pekka Paalanen
Post by Kai-Uwe Behrmann
Post by Pekka Paalanen
and it is also possible to have
sub-surfaces running on their own (i.e. without commit the
parent surface, too)
Hmm, no. Whether you commit or not on a parent surface, has no effect on
the Z-order of sub-surfaces on its own. How could it have? The parent
surface is still there in the same state it was before, if you don't
commit it.

Sub-surfaces running on their own simply means, that a
wl_surface.commit on such a sub-surface will be effective immediately,
instead of waiting for the parent surface to be committed. This
allows a widget in a sub-surface to animate on its own, without
requiring other surfaces to be committed every time it updates.

Committing new state to surfaces, and compositor's repainting are quite
orthogonal. The compositor is free to repaint as often or seldom as it
wants. Committing does not affect the Z-order or repainting order,
unless you explicitly change the Z-order with a place_above/below
request.

I guess I still cannot imagine what side-effects there could possibly
be.


Thanks,
pq
Kai-Uwe Behrmann
2013-03-07 13:51:27 UTC
Permalink
Post by Pekka Paalanen
On Thu, 07 Mar 2013 12:32:21 +0100
Post by Kai-Uwe Behrmann
The flattened output of overlapping transparent regions depend on the
intented order of painting the graph during compositing.
Does your following annotation has a side effect on the painting(Z)
order of transparent content?
Post by Pekka Paalanen
and it is also possible to have
sub-surfaces running on their own (i.e. without commit the
parent surface, too)
Committing new state to surfaces, and compositor's repainting are quite
orthogonal. The compositor is free to repaint as often or seldom as it
wants. Committing does not affect the Z-order or repainting order,
unless you explicitly change the Z-order with a place_above/below
request.
Ok, I now understand the difference between paint and commit. That makes
sense and answers my question.

thanks and kind regards
Kai-Uwe
Continue reading on narkive:
Loading...