After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 699578 - Implement foreign surface support for stages
Implement foreign surface support for stages
Status: RESOLVED FIXED
Product: clutter
Classification: Platform
Component: wayland
unspecified
Other Linux
: Normal normal
: ---
Assigned To: clutter-maint
clutter-maint
Depends on:
Blocks:
 
 
Reported: 2013-05-03 12:35 UTC by Chris Cummins
Modified: 2013-07-23 17:25 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed implementation for ClutterStage foreign surface surface (3.47 KB, patch)
2013-05-03 12:35 UTC, Chris Cummins
reviewed Details | Review
[1/6] wayland: Add API version (1.58 KB, patch)
2013-05-13 16:59 UTC, Chris Cummins
committed Details | Review
clutter-stage-wayland: Pedantic typo fix (1.06 KB, patch)
2013-05-13 17:00 UTC, Chris Cummins
committed Details | Review
[3/6] docs: Remove empty line before parameter tags (1.83 KB, patch)
2013-05-13 17:01 UTC, Chris Cummins
committed Details | Review
[4/6] clutter-input-device-wayland: Update indentifier name (1.05 KB, patch)
2013-05-13 17:02 UTC, Chris Cummins
committed Details | Review
[5/6] docs: Add clutter-wayland section to reference docs (2.58 KB, patch)
2013-05-13 17:03 UTC, Chris Cummins
committed Details | Review
[6/6] wayland: Add foreign surface support to stage (4.09 KB, patch)
2013-05-13 17:04 UTC, Chris Cummins
accepted-commit_now Details | Review
wayland: Add foreign surface support to stage (4.79 KB, patch)
2013-07-15 17:25 UTC, Rob Bradford
committed Details | Review
wayland: Only create and act on shell_surface for non-foreign surfaces (2.68 KB, patch)
2013-07-15 17:40 UTC, Rob Bradford
committed Details | Review

Description Chris Cummins 2013-05-03 12:35:46 UTC
Created attachment 243177 [details] [review]
Proposed implementation for ClutterStage foreign surface surface

This would allow you to provide a Wayland surface to associate with a ClutterStage, rather than having Cogl create the surface and then associating it
with a toplevel shell surface.
Comment 1 Rob Bradford 2013-05-03 18:37:25 UTC
Review of attachment 243177 [details] [review]:

LGTM. ebassi?
Comment 2 Emmanuele Bassi (:ebassi) 2013-05-03 18:41:20 UTC
Review of attachment 243177 [details] [review]:

looks good, in general.

it still needs to update the API reference bits under doc/reference/clutter.

::: clutter/wayland/clutter-stage-wayland.c
@@ +303,3 @@
+ * platform. Calling this function at any other time has no effect.
+ *
+ * Since: 1.14

Since: 1.16

::: clutter/wayland/clutter-wayland.h
@@ +44,3 @@
 struct wl_shell_surface *clutter_wayland_stage_get_wl_shell_surface (ClutterStage *stage);
 struct wl_surface *clutter_wayland_stage_get_wl_surface (ClutterStage *stage);
+void clutter_wayland_stage_set_wl_surface (ClutterStage *stage, struct wl_surface *surface);

needs the CLUTTER_AVAILABLE_IN_1_16 annotation.

to be fair, all the other functions in the header need an annotation, but that ought to be a separate bug.
Comment 3 Chris Cummins 2013-05-13 16:59:39 UTC
Created attachment 244067 [details] [review]
[1/6] wayland: Add API version

Thanks for the reviews, I've made the suggested modifications. Updating the reference docs spiraled into a small patchset in its own right, the last of which (6/6) is the modified foreign surface implementation.
Comment 4 Chris Cummins 2013-05-13 17:00:25 UTC
Created attachment 244068 [details] [review]
clutter-stage-wayland: Pedantic typo fix

Minor typo fix in gtk-doc directive.
Comment 5 Chris Cummins 2013-05-13 17:01:26 UTC
Created attachment 244069 [details] [review]
[3/6] docs: Remove empty line before parameter tags

These parameter docs were not being picked up due do to empty lines.
Comment 6 Chris Cummins 2013-05-13 17:02:06 UTC
Created attachment 244070 [details] [review]
[4/6] clutter-input-device-wayland: Update indentifier name

This function name had fallen out of date.
Comment 7 Chris Cummins 2013-05-13 17:03:21 UTC
Created attachment 244072 [details] [review]
[5/6] docs: Add clutter-wayland section to reference docs

Adds the stray clutter-wayland section to the reference documentation.
Comment 8 Chris Cummins 2013-05-13 17:04:26 UTC
Created attachment 244073 [details] [review]
[6/6] wayland: Add foreign surface support to stage

Updated proposed implementation for ClutterStage foreign surface support, incorporating Emmanuele's suggested changes.
Comment 9 Emmanuele Bassi (:ebassi) 2013-05-13 18:45:40 UTC
Review of attachment 244067 [details] [review]:

looks good.
Comment 10 Emmanuele Bassi (:ebassi) 2013-05-13 18:46:11 UTC
Review of attachment 244068 [details] [review]:

sure :-)
Comment 11 Emmanuele Bassi (:ebassi) 2013-05-13 18:46:44 UTC
Review of attachment 244069 [details] [review]:

sure.
Comment 12 Emmanuele Bassi (:ebassi) 2013-05-13 18:47:09 UTC
Review of attachment 244070 [details] [review]:

looks good.
Comment 13 Emmanuele Bassi (:ebassi) 2013-05-13 18:47:34 UTC
Review of attachment 244072 [details] [review]:

looks good.
Comment 14 Emmanuele Bassi (:ebassi) 2013-05-13 18:48:17 UTC
Review of attachment 244073 [details] [review]:

looks good.
Comment 15 Rob Bradford 2013-05-14 11:38:46 UTC
I'd like to see a test using the new API but i'll go ahead and merge the documentation fixes to master and 1.16.

When the API change is tested we can pull it in.
Comment 16 Rob Bradford 2013-05-14 11:44:12 UTC
Review of attachment 244067 [details] [review]:

Committed.
Comment 17 Rob Bradford 2013-05-14 11:52:46 UTC
Review of attachment 244068 [details] [review]:

Committed.
Comment 18 Rob Bradford 2013-05-14 11:52:58 UTC
Review of attachment 244069 [details] [review]:

Committed.
Comment 19 Rob Bradford 2013-05-14 11:53:08 UTC
Review of attachment 244070 [details] [review]:

Committed.
Comment 20 Rob Bradford 2013-05-14 11:53:19 UTC
Review of attachment 244072 [details] [review]:

Committed.
Comment 21 Rob Bradford 2013-07-15 17:25:56 UTC
Created attachment 249228 [details] [review]
wayland: Add foreign surface support to stage

This adds support for optionally a providing a foreign Wayland surface
to a ClutterStage before it is first show. Setting a foreign surface
prevents Cogl from allocating a surface and shell surface for the stage
automatically.

v2: add CLUTTER_AVAILABLE_IN_1_16 annotation and API reference docs
    (review from Emmanuele Bassi)
v3: set a boolean to indicate that this stage is using a foreign surface
(Rob Bradford)
Comment 22 Rob Bradford 2013-07-15 17:40:17 UTC
Created attachment 249229 [details] [review]
wayland: Only create and act on shell_surface for non-foreign surfaces

We should not create a shell surface and set the role for that shell
surface if the surface was a foreign one provided through
clutter_wayland_set_wl_surface
Comment 23 Rob Bradford 2013-07-15 17:54:53 UTC
Hi Emmanuele, can you give these two new patches a quick glance to check they're doing it how you'd expect.
Comment 24 Rob Bradford 2013-07-23 11:32:20 UTC
ebassi, do you have any feedback on these two patches.
Comment 25 Emmanuele Bassi (:ebassi) 2013-07-23 12:07:30 UTC
Review of attachment 249228 [details] [review]:

looks good.
Comment 26 Emmanuele Bassi (:ebassi) 2013-07-23 12:08:18 UTC
Review of attachment 249229 [details] [review]:

looks good.
Comment 27 Rob Bradford 2013-07-23 17:25:15 UTC
Attachment 249228 [details] pushed as b6d2232 - wayland: Add foreign surface support to stage
Attachment 249229 [details] pushed as 7153863 - wayland: Only create and act on shell_surface for non-foreign surfaces