[Differential] [Request, 645 lines] D1191: Reworking subsurface support

Mon Mar 21 15:17:34 UTC 2016

graesslin created this revision.
graesslin added a subscriber: Plasma.
graesslin set the repository for this revision to rKWAYLAND KWayland.
graesslin added a project: Plasma.
Herald added a subscriber: plasma-devel.

REVISION SUMMARY
  This is a commit series consisting of the following individual commits. If needed I can create reviews for the individual commits.

  commit 710f9eaf042f65799518e3987de82018cedcfbde
  Author: Martin Gräßlin <mgraesslin at kde.org>
  Date:   Fri Mar 18 09:11:28 2016 +0100

    [tests] Add a sub-surface test application

    The test application creates a sub-surface tree consisting of overall
    three surfaces:
    * blue main surface
    * red sub surface
    * green sub surface to the red sub surface

    All surfaces are in synchronized mode. There is a timer to turn the
    green surface into yellow after five seconds.

  commit c1db900ad8621b6364b22a7889370423a72154cc
  Author: Martin Gräßlin <mgraesslin at kde.org>
  Date:   Fri Mar 18 09:13:27 2016 +0100

    [server] Send frameRendered to all sub-surfaces

    If a surface got rendered it implies that all sub-surfaces also got
    rendered. So pass the frameRendered to the complete sub-surface tree.

  commit 43b7e10e1a31f1e09b0a86d8f9c32a8f155bfc53
  Author: Martin Gräßlin <mgraesslin at kde.org>
  Date:   Fri Mar 18 10:41:56 2016 +0100

    [server] Add a bool SubSurfaceInterface::isSynchronized() const

    The mode is not sufficient to determine whether a SubSurface is in
    synchronized mode.

    Quoting spec:
    "Even if a sub-surface is in desynchronized mode, it will behave as in
    synchronized mode, if its parent surface behaves as in synchronized mode.
    This rule is applied recursively throughout the tree of surfaces.
    This means, that one can set a sub-surface into synchronized mode, and
    then assume that all its child and grand-child sub-surfaces are
    synchronized, too, without explicitly setting them."

  commit 43b7e10e1a31f1e09b0a86d8f9c32a8f155bfc53
  Author: Martin Gräßlin <mgraesslin at kde.org>
  Date:   Fri Mar 18 10:41:56 2016 +0100

    [server] Add a bool SubSurfaceInterface::isSynchronized() const

    The mode is not sufficient to determine whether a SubSurface is in
    synchronized mode.

    Quoting spec:
    "Even if a sub-surface is in desynchronized mode, it will behave as in
    synchronized mode, if its parent surface behaves as in synchronized mode.
    This rule is applied recursively throughout the tree of surfaces.

    When committing the state of a sub-surface, the state should not
    be immediately applied if the sub-surface is in synchronized mode.
    Instead it should be cached and only applied after the parent surface's
    state is applied.

    To implement this the Surface::Private has now a third cached state
    buffer. When committing the state is either swapped between pending and
    current or pending and subSurfacePending. Once the parent state is
    applied the state is swapped between subSurfacePending and current.

    The logic for applying state changes is changed. Instead of copying the
    complete state object, the individual state changes are now copied and the
    source gets completely reset to default values. Only the children tree is
    copied back, as that list needs to be modified.

  commit c61ad393e0b3a7ec44ac5f301a77137233c9feba
  Author: Martin Gräßlin <mgraesslin at kde.org>
  Date:   Fri Mar 18 09:57:46 2016 +0100

    [server] Cache the state of synchronized sub surfaces

    When committing the state of a sub-surface, the state should not
    be immediately applied if the sub-surface is in synchronized mode.
    Instead it should be cached and only applied after the parent surface's
    state is applied.

    To implement this the Surface::Private has now a third cached state
    buffer. When committing the state is either swapped between pending and
    current or pending and subSurfacePending. Once the parent state is
    applied the state is swapped between subSurfacePending and current.

    The logic for applying state changes is changed. Instead of copying the
    complete state object, the individual state changes are now copied and the
    source gets completely reset to default values. Only the children tree is
    copied back, as that list needs to be modified.

  commit f616dc48b548580a1fe831b5d1e47e50e09d96b6
  Author: Martin Gräßlin <mgraesslin at kde.org>
  Date:   Mon Mar 21 14:32:31 2016 +0100

    [server] Add a subSurfaceTreeChanged signal to SurfaceInterface

    The idea behind this signal is to notify whenever the tree of sub
    surfaces changes in a way that a repaint of the Surface is required.
    Possible situations are:
    * surface damaged
    * surface unmapped
    * subsurface added/removed
    * subsurface moved (position changed)
    * subsurface stacking changed

    Ideally it would be possible to provide the actual area which needs
    repainting, but due to the possible complexity of the tree, synced
    and desynced changes this doesn't look worth the effort. A user of
    the signal might trigger too many repaints with it, but if it really
    wants to be only notified about the actual changes, it can just track
    the individual sub-surfaces.

  commit 045f2b6de1462961004919d6ab0935c7b435571a
  Author: Martin Gräßlin <mgraesslin at kde.org>
  Date:   Mon Mar 21 15:53:13 2016 +0100

    [server] Don't double buffer adding/removing of sub-surfaces

    QtWayland doesn't commit the parent surface when creating a sub-surface.
    This results in a QtWayland application to freeze as it renders to the
    surface and waits for the frame rendered, which it will never get as the
    Compositor waits for the commit on the parent prior to mapping the
    sub-surface.

    To work around this behavior, we apply the adding/removing directly.
    The behavior around this is actually not fully documented, so QtWayland
    is not wrong per se. See:

    https://lists.freedesktop.org/archives/wayland-devel/2016-March/027540.html

    Once this is properly clarified and implemented in the Client, we should
    revert this change.

TEST PLAN
  Most of the testing is done with the added test case in f616dc48b548580a1fe831b5d1e47e50e09d96b6

REPOSITORY
  rKWAYLAND KWayland

BRANCH
  subsurface-fixes

REVISION DETAIL
  https://phabricator.kde.org/D1191

AFFECTED FILES
  autotests/client/test_wayland_subsurface.cpp
  src/server/subcompositor_interface.cpp
  src/server/subcompositor_interface.h
  src/server/surface_interface.cpp
  src/server/surface_interface.h
  src/server/surface_interface_p.h
  tests/CMakeLists.txt
  tests/subsurfacetest.cpp

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: graesslin
Cc: plasma-devel, Plasma
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160321/e8b21b7e/attachment-0001.html>