Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transactions attempt #2 #1704

Merged
merged 39 commits into from
Jun 26, 2023
Merged

Transactions attempt #2 #1704

merged 39 commits into from
Jun 26, 2023

Conversation

ammen99
Copy link
Member

@ammen99 ammen99 commented Jan 23, 2023

A second attempt at bringing transactions to Wayfire, as the previous attempts were not successful due to the lack of scenegraph. Plan:

  • Update the transactions API with a simpler interface, the old one was overengineered.
  • Rewrite xdg-shell views with the new API (without changing view_interface_t)
  • Rewrite layer-shell views with the new API (without changing view_interface_t)
  • Rewrite xwayland views with the new API (without changing view_interface_t)
  • Split view_interface_t into view_t and toplevel_t and update all usages in the codebase

@ammen99 ammen99 changed the title Transactions attempt #2 [WIP] Transactions attempt #2 May 11, 2023
@ammen99 ammen99 force-pushed the transactions-yet-again branch 2 times, most recently from 7d29de9 to 8723322 Compare May 21, 2023 12:13
@ammen99 ammen99 mentioned this pull request Jun 7, 2023
21 tasks
@ammen99 ammen99 force-pushed the transactions-yet-again branch 2 times, most recently from 0c30f15 to a631b61 Compare June 11, 2023 13:15
@ammen99 ammen99 added the api-breaking API breaking change label Jun 11, 2023
@ammen99
Copy link
Member Author

ammen99 commented Jun 11, 2023

Major breaking change no. 1: #1309. Explanation:

The current view_interface_t (most commonly used as observer_ptr, i.e wayfire_view) combines multiple view types, which however do not all have the same structure. For example, layer-shell panels don't have minimized, fullscreen, etc. states, neither can/should they be moved or resized by plugins. The view API however did not reflect this.

This PR splits the view_interface_t in two classes: view_interface_t and toplevel_view_interface_t, the latter being a subclass of the former. view_interface_t and wayfire_view now support only querying their app-id/title, view role and adding/removing transformers - operations common for all views.

The derived interface toplevel_view_interface_t and the corresponding wayfire_toplevel_view type contain methods specific to toplevel views, e.g. getting/setting geometry, minimized/tiled/fullscreen state, view children, etc.

Updating plugins is usually relatively straightforward, as most plugins do not care about non-toplevel views anyway:

  • Replace wayfire_view with wayfire_toplevel_view in case the plugin deals with toplevel views only.
  • Use toplevel_cast to try and convert a wayfire_view to a wayfire_toplevel_view (returns null for non-toplevel views) where needed, for example when querying the active view on an output, since this view might not necessarily be a toplevel view.
  • view::get_output_geometry() has been completely removed, because it is usually not the geometry plugins need. In the context of transformers, it is usually better to use the scenegraph bounding box functions, for toplevel views we generally want the wm_geometry. Should the information about the position of the top-left corner of the main surface of a view really be needed, it can be computed as view->get_surface_root_node()->to_global({0, 0}).

@ammen99
Copy link
Member Author

ammen99 commented Jun 16, 2023

(Relatively) Minor breaking change no. 2: The *_request functions (move_request, tile_request, etc.) have been moved from the toplevel view interface to a new helper class window-manager, which is meant to accumulate window-management operations and requests over time. Adapting plugins is quite straightforward, simply change for example view->move_request() to wf::get_core().default_wm->move_request(view).

@mark-herbert42
Copy link

applied the patchset - compiled well finally. the main issue i see so far - animation on close window works terrible. Openin is fine ad smooth, but when closing it closes like skipping the frames - and finally get stuck. A small window (zoom) or transparent window (fade) remains on screen until some activity like mouse move happens. Fire is the only one working OK as the close animation.

So far, core has kept a copy of the unmapped view's contents in an
offscreen buffer, so that it can be drawn when the view is unmapped.
With scenegraph, the animate plugin can make and draw this copy itself,
so there is no need to put this plugin-specific functionality in core.
It will be removed and demoted to an internal detail.
It was used only by xdg-popup now, so the necessary functionality was
just merged into it.
view_node_t has been replaced by specific view-node-types for each view
implementation.
Potentially fixes #1799, have to do a few more tests.
This fixes a bug introduced by #1803, where dropping causes
wl_pointer.enter to be sent before the drag has ended, resulting in
weird behavior in clients.
@ammen99
Copy link
Member Author

ammen99 commented Jun 17, 2023

applied the patchset - compiled well finally. the main issue i see so far - animation on close window works terrible. Openin is fine ad smooth, but when closing it closes like skipping the frames - and finally get stuck. A small window (zoom) or transparent window (fade) remains on screen until some activity like mouse move happens. Fire is the only one working OK as the close animation.

@mark-herbert42 The problem should be fixed with the latest commits.

@ammen99
Copy link
Member Author

ammen99 commented Jun 17, 2023

Major breaking change no. 3: Transactions are finally here. This means some changes in how toplevel view state is handled:

  • Toplevel views have a toplevel object associated with them, which can be accessed with toplevel_view_interface_t::toplevel(). Toplevels are transactional objects, that is, they have two states: current and pending. The pending state is that which the compositor wants to apply next, current is what the client has provided the compositor with so far. The pending state is applied atomically and becomes current when the client submits a new buffer. Example:

    • Toplevel is with geometry 100, 100 500x500.
    • Compositor wants to fullscreen it. Pending state of the toplevel is marked as fullscreen with geometry 0, 0 1920x1080. The compositor starts a new transaction. The client is notified of the change, but the current state remains the same.
    • Clients later on submits a new buffer with the requested state. The toplevel object becomes ready (in terms of transactions), and the transaction is applied. Current toplevel state now is also 0, 0 1920x1080 and fullscreend.
  • Most plugins only change the state of a single view, for these cases the helper functions move(), resize(), set_geometry() are still available, but they simply set the pending attributes and start a new transaction.

  • Plugins that want to change multiple view states at once can also do that by manually creating a transaction and adding all participating toplevels to it.

  • Important breaking changes: get_wm_geometry() is to be replaced with either get_geometry() (gets the currently configured geometry of the toplevel) or get_pending_geometry(). fullscreen and tiled_edges can be accessed either directly via view->toplevel().{current,pending}().{fullscreen, tiled_edges} or with the helpers view->pending_{fullscreen, tiled_edges}(). The latter functions work as fullscreen and tiled_edges were used previously.

@ammen99 ammen99 marked this pull request as ready for review June 17, 2023 14:19
@ammen99 ammen99 changed the title [WIP] Transactions attempt #2 Transactions attempt #2 Jun 17, 2023
@mark-herbert42
Copy link

now woks perfectly for me, thank you!

@mark-herbert42
Copy link

found one issue - when making server-decorated window fullscreen it is full-screened with decorations. Decorations themselves do not show up - but the space is taken and look transparent. last 2 patches in the series did nod fix it so far.

@ammen99
Copy link
Member Author

ammen99 commented Jun 20, 2023

found one issue - when making server-decorated window fullscreen it is full-screened with decorations. Decorations themselves do not show up - but the space is taken and look transparent. last 2 patches in the series did nod fix it so far.

Thanks for the heads up, I will have to rework decorations (which I will do soon).

@mark-herbert42
Copy link

I've dropped using tilda in favor of xfce4-terminal which suits wayland better. But still some windows opening like tiny squares - for example skype. This comes with transaction patches - before that all worked.

@mark-herbert42
Copy link

Modified get_margins() in deco-subsurface. I do no feel qualified enough to commit working code as it looks ugly and i bet it is not optimal - but it fixes fullscreen with server decorations.

virtual wf::decoration_margins_t get_margins() override
{
if (!view->pending_fullscreen()) {
return wf::decoration_margins_t{
.left = deco->current_thickness,
.right = deco->current_thickness,
.bottom = deco->current_thickness,
.top = deco->current_titlebar,
}; } else {
return wf::decoration_margins_t{
.left = 0,
.right = 0,
.bottom = 0,
.top = 0,
};
}
}

With transactions, managing the decoration state gets a bit more
difficult, because we need to know the margins individually for each
state. Therefore, the decorator_t interface is no longer needed.
@ammen99
Copy link
Member Author

ammen99 commented Jun 24, 2023

@mark-herbert42 I pushed a few updates, can you tell me which of the problems are still reproducible? I hope that decorations at least are fixed now.

@ammen99
Copy link
Member Author

ammen99 commented Jun 24, 2023

Breaking change no. 4: The decorations API has been changed completely to work with transactions. Key differences:

  • There is no more view->set_decoration(), as it was superfluous.
  • Decorators are encouraged to listen for new transactions, and if there are toplevels in them, update the margins in the pending state. This way core will know how to resize the windows for this transaction.
  • The visual part of the decorations remains as before, since decorations can be simple scenegraph nodes attached when mapping the view.

@mark-herbert42
Copy link

Just a question - id it possible in new decoration design wrap decoration it another "decoration" like was done in window shadows plugin? I do not uderstand this transaction topic at all so may have stupid questions... That design was interesting as it allowed to draw shadows over CSD decorated windows as well like telegram-desktop, which is decorated terribly ugly in Wayland (in Xord it can be server-side decorated).

@ammen99
Copy link
Member Author

ammen99 commented Jun 25, 2023

Just a question - id it possible in new decoration design wrap decoration it another "decoration" like was done in window shadows plugin? I do not uderstand this transaction topic at all so may have stupid questions... That design was interesting as it allowed to draw shadows over CSD decorated windows as well like telegram-desktop, which is decorated terribly ugly in Wayland (in Xord it can be server-side decorated).

It should be possible to decorate a window twice, even though I don't think the results will be very good .. but you could try patching the decoration plugin to ignore the should_be_decorated() property.

@mark-herbert42
Copy link

so looks that the best way is to look at should_be_decorated() and if should - do full decoration, else - only paint shadow. So only one decoration (CSD is not a "real" decoration ) will be created, and the appearance will be handled by renderer. Still not sure if i ever be able to code such thing...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-breaking API breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants