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 770672 - Display rotation fails on Wayland with stage views enabled
Display rotation fails on Wayland with stage views enabled
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: wayland
3.21.x
Other Linux
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
Depends on:
Blocks: WaylandRelated
 
 
Reported: 2016-08-31 19:45 UTC by Nathaniel McCallum
Modified: 2016-10-19 17:30 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
ClutterStageView: Always blit full texture on full onscreen framebuffer (3.50 KB, patch)
2016-09-09 10:06 UTC, Jonas Ådahl
committed Details | Review
MetaWaylandOutput: Pretend outputs are always untransformed (3.98 KB, patch)
2016-09-09 10:06 UTC, Jonas Ådahl
committed Details | Review
MetaWaylandOutput: Cleanup type declaration (2.01 KB, patch)
2016-09-09 10:06 UTC, Jonas Ådahl
committed Details | Review
clutter: Use non-deprecated pixel reading function when picking (1.37 KB, patch)
2016-09-09 10:06 UTC, Jonas Ådahl
committed Details | Review
clutter: Let the stage window translate dirty pixel according to view (1.92 KB, patch)
2016-09-13 10:22 UTC, Jonas Ådahl
committed Details | Review
ClutterStageCogl: Don't get buffer damage dirty pixel when not supported (1.31 KB, patch)
2016-09-13 10:22 UTC, Jonas Ådahl
committed Details | Review

Description Nathaniel McCallum 2016-08-31 19:45:50 UTC
mutter-3.21.91-1.fc26.x86_64

I used MUTTER_STAGE_VIEWS=1 in order to get the rotate buttons in the UI.

This is my setup:

-----------------------
|   |             |   |
| 2 |             | 3 |
|   |      1      |   |
-----             -----
    |             |
    ---------------

#1 is not rotated. #2 and #3 are rotated. #2 works. #3 does not.
Comment 1 Nathaniel McCallum 2016-08-31 19:46:20 UTC
See https://bugzilla.gnome.org/show_bug.cgi?id=745079 for background.
Comment 2 Jonas Ådahl 2016-09-05 06:28:21 UTC
Under what hardware configuration (gpu etc) does this reproduce?

Does it reproduce even if there are no 4k monitor involved?

Does it only reproduce with 3 or more connected monitors?
Comment 3 Guillaume Ayoub 2016-09-05 09:30:26 UTC
(In reply to Jonas Ådahl from comment #2)
> Under what hardware configuration (gpu etc) does this reproduce?

Intel(R) Iris(TM) Pro Graphics 5200.

> Does it reproduce even if there are no 4k monitor involved?

I can reproduce with only 1920×1080 monitors.

> Does it only reproduce with 3 or more connected monitors?

Same problem with only 2 monitors, as long as the rotated monitor is not at the left and at the top of the whole display area.
Comment 4 Jonas Ådahl 2016-09-09 10:06:09 UTC
Created attachment 335170 [details] [review]
ClutterStageView: Always blit full texture on full onscreen framebuffer

When blitting an offscreen onto an onscreen, the whole offscreen should
always be drawn on the whole onscreen. Thus, don't try to convert
between coordinate spaces, just draw the whole offscreen on the whole
onscreen.
Comment 5 Jonas Ådahl 2016-09-09 10:06:14 UTC
Created attachment 335171 [details] [review]
MetaWaylandOutput: Pretend outputs are always untransformed

Since wl_surface.set_buffer_transform() is not supported, until it is
added, pretend outputs are never transformed, so that clients are less
likely to attach pre-transformed buffers.
Comment 6 Jonas Ådahl 2016-09-09 10:06:19 UTC
Created attachment 335172 [details] [review]
MetaWaylandOutput: Cleanup type declaration

Use G_DECLARE_FINAL_TYPE instead of the set of macros.
Comment 7 Jonas Ådahl 2016-09-09 10:06:25 UTC
Created attachment 335173 [details] [review]
clutter: Use non-deprecated pixel reading function when picking

This wont fix anything, it just one small step away from using
deprecated cogl functions.
Comment 8 Jonas Ådahl 2016-09-09 10:07:41 UTC
Note that those four patches doesn't fix the issue completely. It's still not possible to pick (i.e. no pointer focus or pointer events will work) on the monitors where the issue reproduces. Still investigating that.

Two of the patches are just minor cleanups.
Comment 9 Jonas Ådahl 2016-09-13 10:22:48 UTC
Created attachment 335429 [details] [review]
clutter: Let the stage window translate dirty pixel according to view

Otherwise if buffer age is not supported the dirty pixel would be
shifted potentially in unpaintable position.
Comment 10 Jonas Ådahl 2016-09-13 10:22:53 UTC
Created attachment 335430 [details] [review]
ClutterStageCogl: Don't get buffer damage dirty pixel when not supported

Now with the existance of offscreen view framebuffers the buffer age
damage regions are only valid if the view in question doesn't doesn't
have an intermediate offscreen. So, for views that doesn't have buffer
age, return the dirty pixel (0,0).
Comment 11 Jonas Ådahl 2016-09-13 10:23:49 UTC
With those two it seems to work on my setup. I haven't tested on 4K displays though, because I don't have any.
Comment 12 Carlos Garnacho 2016-09-13 14:48:03 UTC
Comment on attachment 335170 [details] [review]
ClutterStageView: Always blit full texture on full onscreen framebuffer

LGTM. Although tbh I thought we actually wanted to go in the opposite direction, making it possible that we eventually only blit/swap the regions that need updating?

Consider it "accepted commit after freeze" anyway.
Comment 13 Carlos Garnacho 2016-09-13 14:48:12 UTC
Comment on attachment 335171 [details] [review]
MetaWaylandOutput: Pretend outputs are always untransformed

Makes sense to do this at the moment.
Comment 14 Carlos Garnacho 2016-09-13 14:48:21 UTC
Comment on attachment 335172 [details] [review]
MetaWaylandOutput: Cleanup type declaration

Looks good.
Comment 15 Carlos Garnacho 2016-09-13 14:48:28 UTC
Comment on attachment 335173 [details] [review]
clutter: Use non-deprecated pixel reading function when picking

Looks good.
Comment 16 Carlos Garnacho 2016-09-13 14:48:40 UTC
Comment on attachment 335429 [details] [review]
clutter: Let the stage window translate dirty pixel according to view

Oops, nice catch. Makes sense.
Comment 17 Carlos Garnacho 2016-09-13 14:48:50 UTC
Comment on attachment 335430 [details] [review]
ClutterStageCogl: Don't get buffer damage dirty pixel when not supported

Indeed
Comment 18 Jonas Ådahl 2016-09-13 14:57:45 UTC
(In reply to Carlos Garnacho from comment #12)
> Comment on attachment 335170 [details] [review] [review]
> ClutterStageView: Always blit full texture on full onscreen framebuffer
> 
> LGTM. Although tbh I thought we actually wanted to go in the opposite
> direction, making it possible that we eventually only blit/swap the regions
> that need updating?
> 
> Consider it "accepted commit after freeze" anyway.

Right. But don't we so far always pass the full view anyway? Further work in clutter_stage_cogl_redraw_view() is needed to ever only blit only part of the view as far as I can see. 'can_blit_sub_buffer' and 'has_buffer_age' will always be FALSE, thus so will also 'may_use_clipped_redraw', meaning we'll always draw a rotated view fully each frame. That is something that we indeed should fix though.
Comment 19 Carlos Garnacho 2016-09-13 15:51:29 UTC
(In reply to Jonas Ådahl from comment #18)
> Right. But don't we so far always pass the full view anyway? Further work in

Right, must be actually one of the odd places caring about this.

> clutter_stage_cogl_redraw_view() is needed to ever only blit only part of
> the view as far as I can see. 'can_blit_sub_buffer' and 'has_buffer_age'
> will always be FALSE, thus so will also 'may_use_clipped_redraw', meaning
> we'll always draw a rotated view fully each frame. That is something that we
> indeed should fix though.

Indeed, I guess doesn't help that we need to redraw the offscreen before every frame. We'll care about improving performance later.
Comment 20 Jonas Ådahl 2016-09-15 02:33:53 UTC
Attachment 335170 [details] pushed as b78b8c9 - ClutterStageView: Always blit full texture on full onscreen framebuffer
Attachment 335171 [details] pushed as 4c8dd08 - MetaWaylandOutput: Pretend outputs are always untransformed
Attachment 335172 [details] pushed as 22173fd - MetaWaylandOutput: Cleanup type declaration
Attachment 335173 [details] pushed as fb6b0de - clutter: Use non-deprecated pixel reading function when picking
Attachment 335429 [details] pushed as 219d230 - clutter: Let the stage window translate dirty pixel according to view
Attachment 335430 [details] pushed as 342532a - ClutterStageCogl: Don't get buffer damage dirty pixel when not supported
Comment 21 Nathaniel McCallum 2016-09-15 20:33:06 UTC
I can confirm that it all seems to work now (even on 4k monitors).
Comment 22 Scott Palmer 2016-10-19 17:30:57 UTC
What all would I have to compile to get this working on Ubuntu 16.04 running Gnome 3.20 PPA?