08:08wlb: weston/main: Daniel Stone * 7 commits https://gitlab.freedesktop.org/wayland/weston/compare/914be30bc0eb8a26e1ce0ab7e91fa3129d5ba51b...3100d3635f03df0d7db7d75d9bb00cb5e9a64791
08:08wlb: weston Merge request !1277 merged \o/ (Be conservative about dirtying paint nodes https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/1277)
08:38wlb: weston Merge request !1279 opened by Philipp Zabel (pH5) backend-wayland: fix error path in wayland_backend_create https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/1279
09:12wlb: weston Merge request !1280 opened by Philipp Zabel (pH5) libweston: move weston_compositor_shutdown call out of backends https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/1280
10:38wlb: weston Merge request !1273 merged \o/ (libweston: prefer active, high refresh rate outputs during surface assignment https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/1273)
10:38wlb: weston/main: Philipp Zabel * libweston: prefer active, high refresh rate outputs during surface assignment https://gitlab.freedesktop.org/wayland/weston/commit/38fea7c3b4eb libweston/compositor.c
12:26swick[m]: relative-pointer has unaccelerated pointer movements, the app can then just draw its own cursor
12:27swick[m]: inputfd is really for devices which are not covered by other wayland protocols (i.e. not pointers, touch, keyboards and drawing tablets)
12:49pq: swick[m], where are we at with color-management, are you waiting for anything specific from me?
12:50wlb: weston Merge request !1281 opened by Daniel Stone (daniels) build: Run tests with leak-sanitizer suppressions https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/1281 [Testing]
12:51pq: swick[m], I want to concentrate on the Weston side a bit, and using libdisplay-info for color. I feel I need a break from *driving* protocol design.
12:56swick[m]: not waiting on anything right now
12:56pq: cool, as I've kinda lost track :-)
12:57swick[m]: still pondering the whole luminance anchoring stuff but that's basically everything
12:57pq: yeah
12:57swick[m]: the feedback mechanism can be added in version 2 easily
12:58swick[m]: clarification on the mastering display stuff is blocked
12:59swick[m]: from my pov everything is in a state that it could be implemented and potentially merged
12:59pq: I have the same feeling.
13:01pq: implemented yes, not sure about merging quite as is
13:01pq: The plan for weston is to copy the XML and prefix all interface names to avoid conflicts with the final form.
13:13daniels: hmm, can someone remind me if attach/damage/commit -> damage/commit is supposed to provoke repaint with the same buffer? or do we need attach/damage/commit -> attach(same)/damage/commit?
13:14emersion: i'm not sure it's clearly defined
13:14emersion: in general front-buffer rendering is not clearly defined iirc
13:15emersion: wlroots will repaint in both cases
13:15emersion: i believe (haven't checked)
13:15emersion: hmmm
13:16emersion: except it might not invalidate the GL texture…
13:16emersion: (it will re-upload shm buffers though)
13:17emersion: the last time we discussed this was in the context of the tearing proto iirc
13:17pq: daniels, if the wl_buffer was released in between, the compositor cannot re-read it.
13:17pq: unless it is explicitly re-attached
13:17emersion: ah, so this isn't about front-buffer rendering?
13:18emersion:jumped the gun a bit
13:18emersion: shm with early release?
13:18pq: it could be, if the compositor didn't release
13:18pq: yeah
13:18pq: both
13:19pq: but given that a compositor *might* release in between, any client using damage+commit without attach is provoking undefined behavior
13:19emersion: https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/65#note_916945
13:19emersion: https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/65#note_1535440
13:19emersion: ^ for front-buffer rendering
13:21zubzub: if the compositor releases the buffer, then the surface has no buffer attached anymore and the surface is no longer painted no?
13:21pq: you can do front-buffer rendering "just fine" if you attach every commit, right? So front-buffer rendering is no excuse to not attach.
13:22pq: zubzub, no. If the compositor releases the buffer, it means the compositor retains the contents some other way. Like as a copy in a GL texture.
13:23zubzub: right
13:23pq: the contents are on the wl_surface, and whether the compositor needs to hold the wl_buffer is just a detail of it
13:23zubzub: yeah the compositor can't decide what the contents of a surface are
13:25emersion: pq, though front-buffer rendering without release violates the spec
13:25pq: yes, but clients can get away with it
13:26emersion: so it means the compositor is free to not update the surface contents in that case
13:26pq: hence the quotes around "just fine"
13:26emersion: okay, the meaning of the quotes were not clear to me
13:26emersion: :P
13:26emersion: i agree that attaching is a good thing to do anyways
13:26kennylevinsen: hmm, I really should at some point look at my "buffer/damage validation" server project again
13:28pq: daniels, me and emersion agree :-o
13:29emersion: tbh i don't know how you get that idea that we always disagree from ;_;
13:29daniels: haha, so the conclusion is that attach is required?
13:29emersion: from my pov, we agree more often than we disagree
13:29pq: attach is required if a client wants it to work
13:29daniels: (the context for this is doing a bunch of dirty tracking etc within Weston, not working on some wild exotic new protocol everyone's going to love)
13:29daniels: kk
13:32daniels: btw, https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/1281 is a pretty trivial QoL improvement if anyone's interested - unfortunately there is still a race-induced leak in drm-smoke - we can try to tear down with page_flip_pending and don't wait around long enough for the destroy, in which case we leak the output
13:32pH5: daniels: wl_surface::damage(_buffer) both explicitly refer to the "pending buffer" and wl_surface::attach states that there is no pending buffer after commit. so expecting wl_surface::commit to applying damage to the already committed buffer instead of the non-existent pending buffer seems wrong, even if the behaviour in that case is not well defined.
13:32daniels: leandrohrb5: ^ did you have something which fixed that or was that stuck behind other stuff?
13:32daniels: pH5: wow, someone who reads the spec more thoroughly than pq :) that sounds very clear to me, thanks!
13:33pq: emersion, negative things are easier to remember so there is that bias. Also if you don't say that you agree to something I've argued, I won't know that you agree.
13:33emersion: yeah, true
13:34pq: ...I hope I say when I agree.
13:34emersion: pH5: maybe there should be a protocol error for damaging without attaching a new buffer then…
13:34emersion: i should try to speak up more when i do agree
13:34daniels: pq: damn, all this time I've been assuming that you completely agree with me when you say nothing :(
13:35daniels: (for clarity, the above is a joke)
13:36ManMower: an especially useful paradigm when pq is on vacation
13:37emersion: :D
13:37daniels: can't wait to re-do colour management in a couple of weeks then
13:37pq: Usually silence from me means: I can't bother to study enough to make my mind, or I don't want to get involved. Or maybe I never noticed the question.
13:38pq: four weeks!
13:38pq: you'll have free reign after four weeks, for three weeks :-)
13:39pq: emersion, a protocol error for damage without attach is a good idea, but I fear it might require a version bump, because of course someone is doing it.
13:39emersion: yea
13:40emersion: let's just record that idea for now
13:41pq: Once a year I think of a "nag" protocol extension. It should be a conduit for compositors to post their complaints to application logs, without crashing the app like protocol errors do.
13:41ManMower: if there's no pending buffer, damage without attach should technically unmap the surface and reset any xdg-shell state, just like attaching a null buffer, shouldn't it? ;)
13:42wlb: wayland Issue #388 opened by Simon Ser (emersion) Add a protocol error when a surface is damaged without a new buffer https://gitlab.freedesktop.org/wayland/wayland/-/issues/388 [Protocol]
13:43pq: ManMower, I suspect I may have written protocol wording to stop that.
13:43pq: because commits can happen without damage, they must be ok without damage, and I don't recall special-casing damage
13:47ManMower: yeah, I think there's a clear distincting between no pending buffer, and attaching a NULL pending buffer.
13:47ManMower: *distinction
13:56pH5: emersion: sounds good, that would have certainly stopped us from wondering.
14:19leandrohrb5: daniels: do you mean the MR that fixes the behavior to leak the output on purpose?
14:20daniels: leandrohrb5: right, but I mean was there an MR which let us hang around to not leak the output?
14:21daniels: ah yes, https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/641
14:21daniels: looks like it just needs some tiny reworks if you have half an hour - since we've turned leak checking on in CI, this does occasionally break innocent MRs
14:22leandrohrb5: sure, I'll take a look
14:23leandrohrb5: for some reason that I don't remember I was not being able to test output hot-plugging (even without this series)
14:24jadahl: pq: a "shame" protocol maybe, where compositors do http post requests whining about situations where it wanted to error out clients to some public website
14:29daniels: leandrohrb5: ahh, thanks
14:32daniels: another fun spec question: what's the expected interaction between wl_subsurface.set_position(child, x1, y1), and wl_surface.commit(x2, y2) on the child?
14:33daniels: for example, if you do wl_subsurface.set_position(child, 1, 1); wl_surface.commit(); wl_surface@child.offset(20, 20); wl_surface.commit(); wl_subsurface.set_position(child, 3, 3);
14:33kennylevinsen: I assume you mean wl_surface.offset(x2, y2)?
14:33daniels: yeah
14:34kennylevinsen: hmm, offset only really made sense in the scope of a toplevel
14:34daniels: in Weston atm, the child's relative pos (assuming no positions) will be (1,1) at the first commit, (21,21) at the second commit, and (3,3) at the second commit
14:34daniels: *at the third commit
14:34kennylevinsen: for some broken definition of "sense"
14:34daniels: yeah, I'd be happy with a conclusion of 'don't offset subsurfs'
14:35daniels: just wondering what others do - do you apply the offset but then stomp it with the next set_pos like we do? retain the offset separately from the set_pos? ignore offset?
14:36emersion: i think we ignore it in wlr, but not sure
14:36emersion: vyivel might know more
14:36daniels: jadahl: ?
14:36kennylevinsen: I touched that a loooong time ago in sway, too long to remember
14:37kennylevinsen: but I have a tingling feeling that sway also explicitly ignored it as it did foo things
14:37emersion: the offset is a delta, i don't think it ever makes sense to replace the position with the offset for any role
14:37jadahl: daniels: good question, i have no recollection of how subsurfaces consumes the dx/dy
14:37daniels: emersion: right, the offset accumulates into the position - but then the next position is a reset which removes any accumulated offset
14:39emersion: right
14:39emersion: mind, the position is part of the parent state
14:39emersion: so i'd be wary of updating the position as part of a child commit
14:40daniels: 'Sub-surfaces also have another kind of state, which is managed by wl_subsurface requests, as opposed to wl_surface requests. This state includes the sub-surface position relative to the parent surface (wl_subsurface.set_position), [...]'
14:40daniels: so that would imply that we should probably just ignore it?
14:40jadahl: looks like it ignores dx/dy for subsurfaces, at least since the transaction infrastructure was introduced
14:43vyivel: emersion: wlroots ignores the surface offset for subsurfaces yeah
14:43emersion: ty
14:43daniels: cool, I'd be inclined to introduce an error for trying
14:43emersion: yeah…
14:44jadahl: errors make me nervous
14:44MrCooper: one issue with ignoring the offset for sub-surfaces is that there's no way to atomically change the size and position of unsynchronized sub-surfaces, since sub-surface position changes are always applied only when the parent is committed
14:44emersion: or at least document _something_ in the spec
14:44daniels: MrCooper: that's literally why the synchronised mode exists tho
14:44emersion: daniels: he means size of the sub-surface
14:44daniels: jadahl: better than it displaying wrong?
14:44MrCooper: it's for consistency with the parent, not self-consistency?
14:45jadahl: daniels: my concerns with introducing new errors is that buggy clients that glitched ever so slightly now will crash
14:45emersion: basically, resize a sub-surface from the top-left corner, without having the parent involved
14:45daniels: yeah, it's the last 5 words that I object to :P
14:45jadahl: there are a couple of places with "this will crash [some toolkit] if we're strict, so lets not" because of it
14:45daniels: the point of sync mode is that you flip it into sync mode, wait for the child to come with a newly-sized buffer, then commit on the parent with set_position as well
14:46emersion: daniels, the use-case would be to resize in desync mode
14:46emersion: yeah, in sync mode, it doesn't make sense
14:46MrCooper: daniels: the parent is irrelevant for what I described, it's about the position & size of the sub-surface always being consistent with each other
14:47daniels: MrCooper: I understand, I'm just saying that trying to do parent-unaware resize + reposition is never really going to work
14:47MrCooper: "every frame is perfect" and all that
14:48jadahl: every frame is only perfect if you use subsurfaces in sync mode
14:48daniels: and in fact in mutter trying that with a top-left resize is just going to result in your window acting as if it were being resized from the bottom right
14:48emersion: i think we should document that it's ignored given all current impls, and decide whether MrCooper's use-case is enough to warrant version bump + new behavior
14:48emersion: if not, version bump + new error
14:48daniels: what would the new behaviour be?
14:48emersion: allow offset in desync mode
14:49emersion: i don't even know if there are users of desync mode
14:50wlb: wayland Issue #389 opened by Daniel Stone (daniels) How should wl_surface.offset interact with subsurfaces? https://gitlab.freedesktop.org/wayland/wayland/-/issues/389 [Protocol]
14:50MrCooper: there's weston-subsurfaces :)
14:51daniels: GSt uses desync in the way it was intended
14:51daniels: specifically, that you start in synchronised mode until you've got everything set up, commit and flip into desync for the duration you're running in steady state, then when you need to resize flip briefly back to sync mode
14:52daniels: that's why you can change it dynamically with requests, as opposed to the mode being immutable
14:56emersion: there are niche use-cases where i think offset+desync can be used while being frame-perfect
14:57jadahl: emersion: gtk3 popups goes desync
14:57emersion: for instance, let's say my sub-surface is a spinner which moves back and forth between two points
14:57jadahl: or popovers, rather
14:57jadahl: (they don't use offsets though, iirc)
15:00daniels: emersion: sure - how much code do you want to write to make that possible though? :)
15:01emersion: yeah, then there's the question of whether we care about the niche use-case or not
15:08MrCooper: I'd be surprised if took a lot of code to make that work in mutter, I just missed this in !1880
15:14daniels: MrCooper: it's not much code at all for us, just a matter of making sure it actually continues to usefully work, and also how much more complicated we really want our subsurface code to be :P
15:14daniels: (bearing in mind that offset co-ords are in subsurf co-ord space whereas set_pos is in parent co-ord space ...)
15:15MrCooper: you mean if they use different scaling?
15:15emersion: it's a delta, so doesn't matter in what space
15:16daniels: yeah you're right, scale/xform only apply to buffer->surface, so the offset would still move in logical pixels anyway
18:32KALT: Hi, is transparency broken?
18:35kennylevinsen: KALT: no, transparency is not broken. If you are asking about a specific application, wayland server or desktop environment that you see being broken, you should ask in their respective forums
18:40KALT: Odd, I'm having difficulty setting up transparency using Vulkan & Wayland. My test application works fine under X11. Under Wayland I've tested various conditions Gnome/KDE/Winit/GLFW/Nvidia(dedicated)/AMD(integrated) with the same result.
18:40KALT: https://user-images.githubusercontent.com/2294209/247638538-0442bbf5-9c2f-4db6-8dcd-598cf33b596d.png
18:41kennylevinsen: what does your test application do?
18:41KALT: Render 3 white squares, on the top left corner, overlapping.
18:41KALT: With a transparent swapchain.
18:42kennylevinsen: what about the rest of the window?
18:43KALT: Nothing, but, I also tried using vkCmdClearColorImage to clear the swapchain to no avail.
18:46KALT: There is some interesting behavior, when using GLFW passing a smaller resolution with a higher resolution swapchain causes expected transparency for the part outside of the GLFW resolution.
18:47KALT: https://user-images.githubusercontent.com/2294209/247640278-fab37991-37c8-41f5-991d-db63267fcc7d.png
18:49kennylevinsen: run your program with WAYLAND_DEBUG=1 set and share the output in a pastebin
18:49kennylevinsen: that'll show what your program is doing on the wire to Wayland
18:50kennylevinsen: it won't show the buffer content, but it'll show what it requests
19:01KALT: I've got a program Vulkan program with transparency that does work and I've taken the logs of both.
19:01KALT: My test program:
19:01KALT: https://sebsauvage.net/paste/?61fc6e88cd5c9912#I4U098v/5eNJ1JyiCdQo4tLE2NElX6SZQ4ZN5QTuJ9Q=
19:01KALT: Vulkan program with working transparency:
19:01KALT: https://sebsauvage.net/paste/?7929eed0809aa37b#IgqAPvp9o84qcw4Uuc9Xo/EI54oJfZjlNlAkkCrzd5s=
19:03KALT: I noticed that "wl_surface@7.frame(new id wl_callback@18)" is missing in my log. What does wl_surface@frame do?
19:04kennylevinsen: it asks the server for a notification next time the server thinks that this surface (window) should start to draw again
19:07kennylevinsen: btw, it's always best to share full logs instead of snippets when getting help
19:08kennylevinsen: for example, I can see that your program invalidates a region that is 1024x1024, but without the rest I have no idea if that is the whole buffer, and if not, if the rest ever got invalidated correctly
19:10bl4ckb0ne: have you tried other compositors?
19:11KALT: Sorry logs were long, https://sebsauvage.net/paste/?145a0708c3c8416d#tt0UTBgsFKI9+7ppysgYH79Nq3n3/jj4AlTCBmgbdA4=
19:11KALT: I've tried both Gnome & Wayland.
19:12KALT: Then again, this is probably not a Wayland issue as there is a working Vulkan demo with transparency.
19:13KALT: Gnome & KDE*
19:13KALT: On Wayland.
19:14kennylevinsen: wayland logs seem good, 1024x1024 surface with 3 GPU buffers and full damage on each commit. I wonder if this could be a gpu quirk if the rest of the buffer hasn't been rendered properly, not super familiar with Vulkan.
19:17KALT: Then it's most likely, me having inproperly set-up Vulkan, thanks for your help.
19:18bl4ckb0ne: seems like alpha composing issue
19:19kennylevinsen: KALT: oh!
19:20KALT: ?
19:20kennylevinsen: You have an opaque region that covers the entire surface
19:20KALT: That's in the logs?
19:20kennylevinsen: yeah, an opaque region with MAX_INT32 dimensions
19:21kennylevinsen: that tells the Wayland server that you will not have any transparency whatsoever so that it can jump corners on compositing
19:21kennylevinsen: which, well, glitches things out if you then actually need it
19:21KALT: Awesome, thanks kennylevinsen. This also explains the behavior or mismatching window & swapchain resolutions.
19:22kennylevinsen: MAX_INT32 is perfectly valid as dimensions on the wire, it just gets capped to the actual surface size
19:23kennylevinsen: but something makes your Vulkan WSI set that opaque region up which you don't want
19:23kennylevinsen: this bit for reference: > wl_region@32.add(0, 0, 2147483647, 2147483647) \n -> wl_surface@26.set_opaque_region(wl_region@32) \n
19:25KALT: Yes! I guess it's window handling library bug.
19:25KALT: https://github.com/rust-windowing/winit/blob/7929999c1c9b9a3ad8966f36663fc4514b517216/src/platform_impl/linux/wayland/window/state.rs#L482
19:25KALT: I just removed that line and it works.
19:26KALT: Again thanks for your help kennylevinsen.
19:28kennylevinsen: KALT: https://github.com/rust-windowing/winit/blob/7929999c1c9b9a3ad8966f36663fc4514b517216/src/platform_impl/linux/wayland/window/state.rs#L93
19:28kennylevinsen: maybe you just need to set that
19:28KALT: I've already set it to true.