Add KWayland virtual desktop protocol
ClosedPublic

Authored by mart on May 11 2018, 9:15 AM.

Details

Summary

Implement the virtual desktop protocol discussed in
T4457 xml protocol, client and server part.

The PlasmaVirtualDesktopManagement interface manages the desktops
instantiation and layout, each desktop is a PlasmaVirtualDesktop
instance which contains unique id, name and position.

PlasmaWindow has new events: plasmaVirtualDesktopEntered
and plasmaVirtualDesktopLeft when a window enters or leaves a desktop,
and desktops as the list of desktops is in. A window can be on
any subset of desktops, if the list is empty, it's considered on all desktops.

Test Plan

Autotest

Diff Detail

Repository
R127 KWayland
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
There are a very large number of changes, so older changes are hidden. Show Older Changes
romangg added a comment.EditedMay 11 2018, 5:06 PM

The file org_kde_plasma_virtual_desktop.xml should be renamed to virtual-desktops.xml or plasma-virtual-desktops.xml to have a similar name format as the other protocol files we have. Also I would in general prefer to just call it "VirtualDesktop..." instaed of "PlasmaVirtualDesktop..." in the class names (file names alike).

Sometimes it says "desktop" instead of "virtual desktop". Although it's cumbersome I would recommend to always use the term "virtual desktop".

Since virtual desktops form a grid I would have thought first at a 2D matrix / 2D array, i.e.: QVector< QVector< PlasmaVirtualDesktopInterface* > > for storing the global information of all Virtual Desktops and their layout in PlasmaVirtualDesktopManagementInterface. You currently use a QList orderedDesktops for all virtual desktops and "holes" when inserting a virtual desktop in the grid in sortDesktops. This seems a bit clunky and error-prone (in particular the calculations to reorder in sortDesktops).

Now a general thought on the layout mechanism: Currently the number of rows is set and together with the number of virtual desktops this results in a column number. This concept is not straightforward. It is also very limiting in the end: a compositor would not be able to set patterns different from a flow left to right, top to bottom. Even the current KCM is already more involved by offering the option to change row and column count.

I propose the following: Remove the layout_position event from org_kde_plasma_virtual_desktop. Instead let event layout of org_kde_plasma_virtual_desktop_management be: layout(wl_array *virtual_desktops, uint32_t columns) with virtual_desktops representing a matrix with columns many columns of all virtual desktops ids from left to right, top to bottom with 0 in cells where no virtual desktop is placed. This way a compositor can create arbitrary 2D patterns, submit a new matrix to KWayland (KWayland then calculates the wl_array and the columns value) and by that it can change the count and position of virtual desktops atomically when sending the layout event (together with added, remove events if necessary) and the done event afterwards.

autotests/client/test_plasma_virtual_desktop.cpp
4

Marco

src/client/plasmavirtualdesktop.h
142

This signal is unneeded when desktopRemoved is there as well.

src/client/protocols/org_kde_plasma_virtual_desktop.xml
5 ↗(On Diff #33979)

2018 and only you Marco?

26 ↗(On Diff #33979)

name="id"

45 ↗(On Diff #33979)

This event is redundant with the current implementation. A client can infer the number of rows and columns from the org_kde_plasma_virtual_desktop objects it receives and their maximum row and column values.

Also a compositor might be interested in setting a custom layout like this: 5 virtual desktops as a cross (one top, 3 in a row mid, one bottom). While one can still say this has in some sense 3 rows and 3 columns, the event in this case loses its intended meaning of defining the "layout".

Look at my review comment for a different approach.

83 ↗(On Diff #33979)

Mention that this event is send on bind and when the position changes.

+ line breaks

src/client/protocols/plasma-window-management.xml
275

Just name it enter_virtual_desktop I would say. For request_leave_virtual_desktop the same.

src/server/plasmavirtualdesktop_interface.cpp
144

indent (below as well)

201

Send done event in the end and flush the client.

src/server/plasmavirtualdesktop_interface.h
78

The compositor does not need to decide upon an id. Let KWayland do this internally (this also means that whenever createDesktop is called it is guaranteed that a new PlasmaVirtualDesktopInterface will be returned.

Or is this needed in regards to Activities down the road? If yes I would also argue that a KWayland internal id would be nicer and KWin should map them then to Activity ids.

104

That the management class is friend to the things it manages (here the PlasmaVirtualDesktopInterface) ok, but the other way around... I would maybe try to work without it.

mart added inline comments.May 14 2018, 12:52 PM
src/client/protocols/org_kde_plasma_virtual_desktop.xml
45 ↗(On Diff #33979)

isn't better the layout to be communicated by the manager? the reasoning was to have it loosely map on models.
the model tells how many rows it has, then the single item on what row it is located.

Also, I think is to be decided now as it influences everything else:
Do we want to allow "holes" in the order? (as you said one in first row, 3 in second etc)

src/client/protocols/plasma-window-management.xml
275

tough is a thing the client asks for which may or may not be granted.. just enter_virtual_desktop looks like it's already decided?

src/server/plasmavirtualdesktop_interface.h
78

what i would like to is to have in the end the ids coming from kactivities, (as i want in wayland desktops===activities) which makes more sense to link and use in kwin, rather than in kwayland

104

yeah, i would need to make things work without needing that ugly ordereddesktops

bshah added a subscriber: bshah.May 14 2018, 1:02 PM
bshah added inline comments.
src/client/registry.h
157

This should be added at end and not in middle and also add @since 5.47(?)

mart added inline comments.May 14 2018, 1:04 PM
src/client/registry.h
157

you're right this breaks ABI

romangg added inline comments.May 14 2018, 1:53 PM
src/client/protocols/org_kde_plasma_virtual_desktop.xml
45 ↗(On Diff #33979)

isn't better the layout to be communicated by the manager?

In the review comment with the "different approach" I propose exactly this. I.e. not remove this (currently redundant) event layout, but even promote it by not only sending the current row and column count, but also directly all positions of existing VDs through a wl_array.

the reasoning was to have it loosely map on models.
the model tells how many rows it has, then the single item on what row it is located.

Thanks, good to know the idea behind it. I see mostly a problem with synchronization between the single virtual desktop objects among themselves as well as with the manager with this approach. For example if the compositor decides to remove a virtual desktop, what reduces the row/column count: Is the layout event first emitted or the layout_position events for the remaining VDs? How in both cases should the client react?

If the client first receives the layout event it still does not really know which VD has been removed. If it first receives all layout_position events plus the removed event it does not need the layout event any more.

But in any case the client would need to hold the VDs with already received layout_position events in some intermediate state until it has received the events for all VDs plus the removed event for the VD, which got removed. Otherwise it might put multiple VDs on the same position.

Do we want to allow "holes" in the order? (as you said one in first row, 3 in second etc)

For this protocol extension I would vote in favor of that, because there are valid use cases and we should write our protocols directly for a larger audience if possible. Here if we use a matrix for representation (with 0 if there is a hole) it wouldn't introduce much more complexity.

Currently on X we only allow full rows and columns I believe, so the protocol implementation in KWin / Plasma could still only use full rows / columns for now.

In general in the long run I imagine a KCM, where a user sees the current VDs in an overview like with the pager and then has buttons around it with plus signs on it the user can click to add another VD in this location. Also with this approach we could later on add "spans" over multiple columns/rows. For example a main VD and below three auxiliary VDs.

mart added a comment.May 16 2018, 11:17 AM

so:

  • the manager just having a maximum_rows event, which tells that's the boundary of rows the kayout should have

or... not even that (then the setting of maximum row/columns becomes completely kwin internal)

  • the location managed completely internally by the virtual desktop so they can have any layout, with holes anywhere
  • this simplifies a lot the code protocol-wise (in fact pretty much revertin the last commit in the branch)

but then... how to manage collisions? allowing to set the same row/column to different desktops and make kwin responsibility of this not happening?

In D12820#263516, @mart wrote:

but then... how to manage collisions? allowing to set the same row/column to different desktops and make kwin responsibility of this not happening?

For a layout change the compositor would give KWayland a complete matrix of all virtual desktops indicating their new positions. So KWin would need to make sure that:

  • every cell in the matrix only has one VD object referenced (this is per definition if the matrix is QVector<QVector <VirtualDesktopInterface*>> ),
  • the matrix contains all available VDs.

The compositor would calculate the new matrix internally (it knows about all available VDs and their current positions through KWayland) and then call:

  1. PlasmaVirtualDesktopManagementInterface::layout(QVector<QVector <VirtualDesktopInterface*>> matrix) containing all VDs,
  2. sendDone().

On the other side when the compositor wants to create a new VD, it would call:

  1. PlasmaVirtualDesktopManagementInterface::createDesktop(),
  2. PlasmaVirtualDesktopManagementInterface::layout(QVector<QVector <VirtualDesktopInterface*>> matrix) containing all VDs including the new one,
  3. sendDone().
graesslin added inline comments.May 16 2018, 3:31 PM
src/client/protocols/org_kde_plasma_virtual_desktop.xml
4 ↗(On Diff #33979)

I doubt I have any copyright on this

45 ↗(On Diff #33979)

I would prefer to not have any layout in the protocol. Setting a layout makes it impossible to have different virtual desktop per output.

81 ↗(On Diff #33979)

like with the other protocol: please no layout information.

mart added inline comments.May 17 2018, 2:59 PM
src/client/protocols/org_kde_plasma_virtual_desktop.xml
45 ↗(On Diff #33979)

Ok, so no layout information at all neither in the manager nor in the single virtual desktops?
I kindof like that as makes things a lot simpler. Tough in this case if the concept of layout becomes completely internal in kwin, how to communicate it to the client? a pager would still need to loosely represent the same layout.

graesslin added inline comments.May 17 2018, 4:07 PM
src/client/protocols/org_kde_plasma_virtual_desktop.xml
45 ↗(On Diff #33979)

A possibility could be a sibling concept. Every desktop could have a reference to the desktop to the left/top/right/bottom.

mart added inline comments.May 17 2018, 4:35 PM
src/client/protocols/org_kde_plasma_virtual_desktop.xml
45 ↗(On Diff #33979)

How you would envision different virtual desktops per output?
to me is a bit of a weird concept, as an output is an actual desktop, not virtual?

45 ↗(On Diff #33979)

A possibility could be a sibling concept. Every desktop could have a reference to the desktop to the left/top/right/bottom.

wouldn't become harder to track and rebuild a visual representation from it?

zzag added a subscriber: zzag.May 17 2018, 7:37 PM
zzag added inline comments.
src/server/plasmavirtualdesktop_interface.cpp
37
class Q_DECL_HIDDEN PlasmaVirtualDesktopInterface::Private

Same for the server part.

graesslin added inline comments.May 18 2018, 3:52 PM
src/client/protocols/org_kde_plasma_virtual_desktop.xml
45 ↗(On Diff #33979)

Have a look at i3. Btw it's a really often requested feature.

mart added inline comments.May 21 2018, 10:11 AM
src/client/protocols/org_kde_plasma_virtual_desktop.xml
45 ↗(On Diff #33979)

the virtualdesktop would then have an output property used for filtering?
or, the layout event could be kept in the manager, but be not just rows and column, but pass the pair output and actual vector containing the virtualdesktop instances?

also if it is per output then two desktops can be on the same row/column, as long as they are on different outputs..
then also all the api for requesting/notifying activate/deactivate would need an output parameter.. /o\

My primary use for this protocol was to make activities==virtual desktops (the quick way would be to just use the activity ids as desktop ids and use kactivitymanager to add/remove, so no need for api to ask kwin to add/remove desktops)
this clashes a bit with different desktops per output, tough could be solved if i then make the activity id a separate property in the Virtualdesktop interface...

graesslin added inline comments.May 21 2018, 1:47 PM
src/client/protocols/org_kde_plasma_virtual_desktop.xml
45 ↗(On Diff #33979)

Please note that your quick way would not work. Activities are disabled in KWin Wayland and I don't see them getting enabled again. Activities resulted in hard freezes several times and I don't trust them due to that. As maintainer I would need very convincing arguments on why activities won't break KWin again. Due to the fact that kamd speaks both dbus and windowing protocol I don't see a chance that KWin can rely on it.

Due to the fact that activities used to block KWin it's an area which is completely not ported to Wayland yet. A Wayland window cannot be put on an activity. So this would require a significant amount of work. This also doesn't match anything we have in KWin to move forward. Activities have been in an unmaintained state since the start and the code is in a really bad state. None of the core KWin developers have touched it in the last year's. It's just a bad copy and paste of virtual desktops without any understanding of the domain. I do not want to see any future code being based on that. In KWin we started to evolve virtual desktops, so that in future it would be possible to implement the feature set of activities in virtual desktops. But for the time being it would mean X11 identifiers, that is numerical desktops.

I put know out an interesting question: do we need layouts?

mart added inline comments.May 24 2018, 9:24 AM
src/client/protocols/org_kde_plasma_virtual_desktop.xml
45 ↗(On Diff #33979)

well, i'm not really talking about the current activity code in kwin, but just a 1:1 mapping for the user, for which i was expecting different code to be written.
if the 1:1 mapping happens on kwin side, or on plasma/kactivities side trying to sync itself to desktops (in this case the protocol would need also adddesktop/removedesktop methods, then kwin wouldn't need to use kactivities directly, the sync can be done client side.

but i don't want the user to see any difference anymore for the two concepts anymore.

mart updated this revision to Diff 34871.May 25 2018, 4:15 PM
  • Revert "virtual diland Virtual desktop protocol"
  • rename the xml file to be consistent with the rest
  • fix copyright
  • replace the concept of layout with neighbours
  • smaller fixes
mart added a comment.May 25 2018, 4:17 PM

different version for a new round of discussion: try to implement the neighbour concept for the desktop that stay near asnother one, so there may be as many grids unconnected with each other as possible

so far there is not integrity check that an added it is actually an existing desktop, but i would really rather add such checks and auto updates as well.

How is a change of neighbors supposed to work for clients already bound to the proxy objects? For example consider virtual desktop grid:

D1 D2
D3 D4

Compositor removes D3 and let D4 flow back to the left:

D1 D2
D4

desktop_removed event is sent and for D1 and D2 bottom neighbor changes, for D4 left and top neighbor change. To have a consistent state on the client at all time, one would need to tell the client when it has fully received all these information coming from multiple interfaces. Maybe it is enough to send the done event of the management interface not after itself has finished all events, but also every desktop interface has sent it. Not sure if this is fine protocol hygiene though. If it is we need to add it to the descriptions of the interfaces.

src/client/protocols/plasma-virtual-desktop.xml
26

This arg name should be id (judging from other example protocols). Find a different arg name for the second argument, for example desktop_id or just desktop. Together:

<arg name="id" type="new_id" interface="org_kde_plasma_virtual_desktop"/>
<arg name="desktop" type="string" summary="Unique identifier string of the desktop"/>
src/client/protocols/plasma-window-management.xml
275

It's a request, so it's intrinsic that it's not yet decided. You can say in the description, that the server might ignore this request, otherwise it sends the virtual_desktop_entered event.

General rule of thumb for any data storage for me is to never have a possible corruptable state.
The current protocol which has the position specified inside the Virtual Desktop object itself, so you can have two in the same place and you can't swap the position of two in an atomic move.
This sibling part makes that worse.

I would prefer to not have any layout in the protocol. Setting a layout makes it impossible to have different virtual desktop per output.

Completely agree, but for completely different reasons.

Layout is not a property of the virtual desktop itself but a property of the relevant view.

In the current (even on X state) I can represent 4 virtual desktops in 1 2x2 grid on the pager so it fits best, yet 4x1 along the edges of the cube, and it wouldn't be unfeasible to display them 1x4 in an activity manager style switcher.

What I think would work best is a shared ordered-list, but the visual representation of that ordered list is entirely up to the view.

Kwin still needs to turn that into a grid in some places for keyboard nav and the slide effect, but that's just the case of a single int config value.

mart added a comment.Jun 8 2018, 2:29 PM

In the current (even on X state) I can represent 4 virtual desktops in 1 2x2 grid on the pager so it fits best, yet 4x1 along the edges of the cube, and it wouldn't be unfeasible to display them 1x4 in an activity manager style switcher.

What I think would work best is a shared ordered-list, but the visual representation of that ordered list is entirely up to the view.

Kwin still needs to turn that into a grid in some places for keyboard nav and the slide effect, but that's just the case of a single int config value.

so...
Remove all layout position and siblings, and just pay attention
plasmaVirtualDesktops() on server, and
desktops() on the client always have the same order.
Would there be some protocol to reorder desktops or would be just a detail internal in kwin?

wether they are on a line, on a cube, on a 3 rows grid, then would just be a config value setting shared between kwin and the pager, not passing trough wayland in any way.

whith this, would still be possible to allow maybe in the future desktops per-output whicj Martin was talking about?

Don't feel the need to change the code unless you and whoever else agree.
I'm not forcing anything, just commenting on my preference.

But yeah, that would be my proposal.

It'd mean reverting work here, but porting the existing plasma pager/effects would go back to being a lot easier.
I don't even know how I'd go about porting it with this.

Would there be some protocol to reorder desktops or would be just a detail internal in kwin?

I think you'd need to communicate the same order with all clients.
That could be just replacing virtual_desktop_added/virtual_desktop_removed/done with set_virtual_desktops(wl_array<string>); or something else.

whith this, would still be possible to allow maybe in the future desktops per-output whicj Martin was talking about?

WRT positioning, probably. You'd need new API, but you wouldn't have redundant API.
WRT which one is active, this code wouldn't cover that as-is.

davidedmundson added inline comments.Jun 8 2018, 2:52 PM
src/client/protocols/plasma-virtual-desktop.xml
57

Code comment:

This interface doesn't have a destructor.

The server can invalidate it on it's end, but ultimately up to the client to destroy the reference which they can't do.

mart added a comment.Jun 11 2018, 9:43 AM

Don't feel the need to change the code unless you and whoever else agree.
I'm not forcing anything, just commenting on my preference.

nono, i'm just trying to thoroughly think about it :)

I think you'd need to communicate the same order with all clients.
That could be just replacing virtual_desktop_added/virtual_desktop_removed/done with set_virtual_desktops(wl_array<string>); or something else.

that may make more difficult to notice what desktop has been added...
what about virtual_desktop_added*id, position)?
that way, mapping it to a model would become really trivial

whith this, would still be possible to allow maybe in the future desktops per-output whicj Martin was talking about?

WRT positioning, probably. You'd need new API, but you wouldn't have redundant API.
WRT which one is active, this code wouldn't cover that as-is.

it would work if added/removed also had an output as parameters, then they can be different independent sets, tough that would force us to think about it right now, which i'm not too sure

We can remove every layout reference in the manager as well as in the VDs. I don't see the advantage then with a list in contrast to events desktopAdded / desktopRemoved and a done event. What is better about having an ordering? It's only relevant for the special case of having a "flow" of VDs over several lines, assumed we additionally share a single integer with the workspace (via config file or DBus).

But if we decide to not have any layout reference in the VD protocol, then we should also not think about enabling this special case. Instead the VDs should be announced unordered and then a layout is shared with workspace (via config file or DBus).

On the other side VDs without a layout are somewhat futile. Also what we achieve is just deferring the syncing step to the client, which still has to sync the Dbus/config change signal with the Wayland protocol events. One could argue it becomes even worse now over multiple channels.

Still it could make sense because we have a minimal protocol this way as a start and can later extend it if we see problems with the Dbus/config side channel.

What is better about having an ordering?
It's only relevant for the special case of having a "flow" of VDs over several

It's relevant in all cases.

You'd want the context menu of kwin's menu and the task manager to be the same.
Same for any kwin effect that shows it. Whether it's a carousel list, or if it was mapped in a 3D way round all 6 sides of a cube.

I think it counts as a property of the data.

On the other side VDs without a layout are somewhat futile.

Not really. The entire basis of my comments is that we currently commonly lay things out different in different UIs.

mart updated this revision to Diff 36052.Jun 12 2018, 4:33 PM
  • remove neighbours, only order
  • store data in a single structure
  • auto activate a new desktop

Cool. I'm happy with the protocol, except for one important comment.

IMHO worth doing the kwin/plasma code before pushing the frameworks code.

src/client/protocols/plasma-virtual-desktop.xml
63

This still isn't deletable, see my comment from last time.

mart updated this revision to Diff 36161.Jun 14 2018, 2:31 PM
  • remove release from the protocol
  • new_id is id
mart added a comment.Jun 22 2018, 2:57 PM

Looking to wire it into KWin:
so, the first thing i note is that abstractclient has already integration api with virtualdesktops with the old assumptions of numerical id and window on either one or all desktops, which is what Client is using on X11.

A virtualdesktopmanager is always created, both in wayland and x11 (in wayland atm to manage xwayland i guess, tough there seems to be partial support for the old virtualdesktop paradigm, both in Shellclient and in the plasmawindow protocol)

What i want to try to do now, is to add usage of the virtual desktop protocol directly in VirtualDesktopManager of virtualdesktops.cpp (all code that would be disable when a waylandserver is not found)

that would be the easiest place to wire up the protocol's virtualdesktopmanagement

mart updated this revision to Diff 36528.Jun 22 2018, 4:39 PM
  • Merge branch 'master' into mart/plasmavirtualdesktop
romangg added inline comments.Jun 26 2018, 4:13 PM
src/client/protocols/plasma-virtual-desktop.xml
56

This needs more description. Other events below as well.

src/server/plasmavirtualdesktop_interface.cpp
182

What happens on an empty QString argument? I wouldn't allow it and abort with nullptr returned directly.

mart updated this revision to Diff 36887.Jun 29 2018, 2:32 PM
  • add desktopActivated signal
  • correct behavior for set on all desktops
  • add new tests for on all desktops
  • make the old isOnallVirtualDesktops api work
mart updated this revision to Diff 36888.Jun 29 2018, 2:40 PM
mart marked an inline comment as done.
  • better comments
mart marked an inline comment as done.Jun 29 2018, 2:41 PM
mart updated this revision to Diff 37149.Jul 4 2018, 3:50 PM
  • add a request remove to the protocol
  • add api call to request removal of a desktop
  • requestCreate
  • remove window management features:
  • active() -> isactive()
  • requestEnterNewVirtualDesktop

The VD protocol was a pure "consumer" protocol before this last revision. Now it gets more complicated by putting requests into it for asking the compositor to change its configuration. I thought this was supposed to go through Dbus. How do you plan to restrict
sandboxed apps in order to not mess with the compositor configuration now?

mart added a comment.Jul 4 2018, 4:10 PM

The VD protocol was a pure "consumer" protocol before this last revision. Now it gets more complicated by putting requests into it for asking the compositor to change its configuration. I thought this was supposed to go through Dbus. How do you plan to restrict

sandboxed apps in order to not mess with the compositor configuration now?

It would be a restricted one, usable only by plasma, like plasmashell protocol and such
Tough is a good point... it may make sense to go trough dbus instead (so the KCM can use this as well)
But.. how would you implement "move this window to a new desktop" in that case?

In D12820#286961, @mart wrote:

But.. how would you implement "move this window to a new desktop" in that case?

This functionality is part of the (amended) plasma window management protocol and not of the new VD protocol.

Btw the current diff is somewhat broken. It incorporates besides your changes other changes from the last few weeks.

mart updated this revision to Diff 37197.Jul 5 2018, 4:18 PM
  • ensure there are never duplicates
hein added a comment.Jul 17 2018, 1:13 PM

I have a problem: When I restart plasmashell, I don't get any desktops again. I think the code may not be re-sending the data when the client goes away and comes a second time.

mart updated this revision to Diff 38017.Jul 18 2018, 2:29 PM
  • fix initial state on createresource
  • send done to new clients
  • desktops shouldn't be deleted when the clients go away
  • iget rid of setActiveDesktop
  • rename the virtual desktop to Unstable
mart added a comment.Jul 18 2018, 2:32 PM

classes renamed to fooUnstableV1, but i need comments, i'm not sure about the naming or how to manage the fact that's unstable..
‎‎Most of the unstable protocols just have the protocol implementation as unstable, but already kinda commit to stability to the c++ interface (as opposed to have a name which has unstable, and an incompatible version would have another class that can be as incompatible as it wants)

‎So, not adjusting the kwin part to it, as i'm not sure at all on what we want yet

mart updated this revision to Diff 38122.Jul 20 2018, 8:47 AM

rever rename

mart updated this revision to Diff 38123.Jul 20 2018, 8:50 AM
  • Revert "rename the virtual desktop to Unstable"
  • Merge branch 'master' into mart/plasmavirtualdesktop
mart added a comment.Jul 25 2018, 3:15 PM

RFC: add back some info for the layout? (in particular: rows and orientation, tought orientation is a bit concerning as is view-spacific)

mart added a comment.Oct 16 2018, 8:54 AM
In D12820#297896, @mart wrote:

RFC: add back some info for the layout? (in particular: rows and orientation, tought orientation is a bit concerning as is view-spacific)

it was decided no, we care only about the order

davidedmundson accepted this revision.Oct 16 2018, 1:45 PM
This revision is now accepted and ready to land.Oct 16 2018, 1:45 PM
This revision was automatically updated to reflect the committed changes.