00:45 <robert_mader> Hi. I shortly wanted to ask for an opinion concerning submitted buffer. IIUC it's safe to assume that Wayland compositors don't change the content of such buffers. So looking at different ways to implement partial damage with double/n-buffering, does anything speak against blitting from a already commited and not yet released buffer back to the next buffer in preparation? It's likely not possible with the wayland EGL platform, but I sup

00:45 <robert_mader> pose it is when using dmabufs/shm.

00:46 <robert_mader> phrased differently, the Wayland spec only forbids writing to "locked" buffers, not reading, right?

06:35 <emersion> hm, i wonder. wayland buffer submission is non-destructive, but that doesn't necessarily mean that the buffer contents are untouched

06:36 <emersion> one use-case would be to change the buffer tiling in-place (this is called a "transition", e.g. to allow direct scanout)

06:36 <emersion> and then change it back before releasing the buffer to the client

06:36 <emersion> what is the wording used in the protocol?

07:19 <daniels> emersion: if you did that, you'd have to ensure that you returned -1 as buffer_age to the client to tell its contents were trashed

07:20 <daniels> we can't specifically preclude it in wl_buffer because sometimes they get used for screenshots or whatever, but perhaps wl_buffer.attach() could carry an explicit 'no we won't destructively modify anything'

07:20 <emersion> daniels: not if the compositor transitions the buffer back before returning it to the client?

07:22 <daniels> that sounds like it'd break multiple attach

07:22 <emersion> it… depends

07:23 <emersion> transition on first surface commit, transition back when the last surface commit has done using the buffer…

07:24 <emersion> it'd definitely be weird, and no compositor does that so far

07:26 <jadahl> only time I've seen a buffer changing behind the scene was a i915 bug where trying to glReadPixel() from a texture from a wl_buffer would accidentally discard modifier flags without changing the actual buffer content

07:26 <jadahl> resulting in fun graphical glitches

07:26 <emersion> also, with dmabuf-hints, it's better if the client does the transition itself

07:32 <daniels> srsly

07:33 <daniels> if you want to do anything destructive, do a copy?

07:37 <emersion> the whole point of doing it in-place is to make it Fast™ and avoid any copy

07:44 <daniels> but the speed slope is: making a copy <<<< in-place transformation <<<<<< client just doing what you wanted it to in the first place, right :P

07:45 <emersion> yup

07:45 <emersion> with dmabuf-hints, the client can just do the right thing

07:46 <emersion> so, maybe it's fine to spec wl_surface.attach as read-only?

07:55 <daniels> I would ack the bejesus out of that

08:49 <emersion> jadahl, seems like we have some sed work to do in the wayland and w-p readme/docs

08:56 <pq> emersion, I have always, from the very start, assumed that wl_surface.attach does not allow the compositor to modify the buffers in any way, at any time. So robert_mader's suggestion is ok. Has reality made this false already?

08:56 <pq> If not, then yes, let's make it very explicit in the spec if it's not already.

08:57 <emersion> pq, not yet, but James Jones had patches pretty close from completion to make this false

08:57 <pq> yikes

08:57 <emersion> see XDC 2019, IIRC

08:57 <emersion> OTOH, it was discussed there that clients should do transitions

08:57 <emersion> not compositors

08:57 <pq> I remember hearing about transitions, but I never connected here

08:57 <jadahl> emersion: yea

08:58 <pq> clients do it, definitely

08:58 <pq> good

08:58 <emersion> pq, so yeah i think it's fine to clarify the spec this way

08:58 <pq> yup

09:00 <pq> I think every request that takes a wl_buffer should be very explicit on the expectations on *both* sides - like sync: can you read or write, and if not then when can you

09:00 <pq> sides: compositor and client

09:01 <pq> also whether the compositor is "allowed" to detect and act on a client violating the rules

09:02 <pq> like we cannot have a compositor disconnecting a client because it is writing into a wl_buffer reserved by wl_surface.attach+commit, since there could already be clients that do that and run "fine"

09:02 <pq> e.g. old Xwayland

09:03 <emersion> hm right

10:11 <daniels> emersion: I mean Streams was all kind of spec violating

10:11 <daniels> I don't think transitions make that any better or worse

10:11 <daniels> it's already a mystical black box of 'errrr dunno it works for us'

11:25 <pq> swick, btw. I don't I saw any copyright notices from you in the CM&HDR code, would you want some?

11:25 <pq> *I don't think

11:31 <robert_mader> emersion: "Committing a pending wl_buffer allows the compositor to read the pixels in the wl_buffer. The compositor may access the pixels at any time after the wl_surface.commit request." - the first sentence indicates that the compositor is not free to *write* to the buffer

11:32 <emersion> robert_mader, i'd explicitly indicate that mutating the buffer's underlying storage is forbidden

11:33 <emersion> also, nothing says a wl_buffer has pixels in it :>

11:33 <emersion> there can be some "inventive" wl_buffer types

11:34 <emersion> like solid color, mirror of another surface, stereoscopic 3D content, etc

11:34 <pq> svg... :-P

11:35 <emersion> yeah, that's a good one as well…

11:35 <robert_mader> :D

11:35 <emersion> so i'd prefer to refer to "buffer contents" or "underlying buffer storage" instead

11:35 <robert_mader> But in tendency, would you say back-blitting should be avoided?

11:36 <emersion> why do you need to back-blit?

11:36 <emersion> not possible to accumulate damage like in the GL path?

11:36 <pq> what' back-blitting?

11:37 <emersion> pq, submit a buffer, realize you need a new one to draw, blit the buffer you've submitted to the new buffer

11:37 <pq> well, copying from the buffer you just submitted allows you to not track damage for further than just one frame, and it also allows you to simply blit instead of re-render.

11:38 <robert_mader> yeah apparently the Firefox compositor integration gl paths do something like that on CoreAnimation

11:38 <emersion> robert_mader, do you always unconditionally blit the whole buffer?

11:38 <emersion> or just a region of it?

11:38 <emersion> pq, blitting when allocating a new buffer sounds like an unnecessary optimization

11:39 <emersion> allocating a new buffer should be pretty rare

11:39 <robert_mader> I'd say only a region - which could be calculated using buffer age and the damage region from previous commits

11:39 <emersion> ok, good

11:39 <pq> emersion, it's not about when allocating, but when starting every frame.

11:40 <emersion> i think there are cases where blitting is a bad idea

11:40 <emersion> let's say the whole buffer is updated each frame, e.g. solid color fill changing at every frame

11:41 <emersion> blitting is a bad idea in this case because the blitted region gets over-written when drawing the new content

11:41 <pq> sure, but I think this more towards the cases where you have tons of geometry to render, like a web page?

11:42 <emersion> if you know what changed since the last frame, you can blit (accumulated_buffer_age_damage - next_frame_damage)

11:42 <emersion> then render next_frame_damage

11:42 <pq> yes

11:42 <robert_mader> well, tracking some damage regions over multiple frames is apparently much easier than making the whole gl stack aware of that possibility

11:42 <robert_mader> so selective blitting should be easy

11:43 <emersion> so yeah, i think blitting is a good idea if you don't blit the region you're going to re-render, robert_mader

11:43 <emersion> and only blit what's necessary according to the buffer age

11:44 <robert_mader> emersion: cool, thanks. Then I'll consider that option

11:44 <emersion> i think dnkl has been thinking about things like this as well

11:45 <emersion> https://github.com/swaywm/wlroots/issues/2705#issuecomment-835252396

11:45 <gitlab-bot> swaywm issue 2705 in wlroots "Add support for GL_EXT_robustness" [Enhancement, Renderer, Open]

11:45 <emersion> eh, the bot does GitHub issues as well? interesting

11:46 <emersion> the numbers aren't necessarily directly applicable to firefox, because the rendering workload will be different

11:47 <emersion> thanks for working on this, robert_mader :)

11:52 <robert_mader> ah right, the software backend on compositors that hold shm buffers also falls into this category. And sure :)

11:52 <dnkl> I haven't been following this discussion in detail, but does sound familiar :) If (one of) the question is "should the compositor be allowed to modify a non-released buffer", then I'll just mention that foot will break if a) compositor doesn't do immediate releases, and b) the compositor modifies the unreleased buffer, and c) the buffer we're rendering to is "too old" - in this case

11:53 <dnkl> foot memcpy:es/blits the last buffer (currently in-use by the compositor) to the new buffer

11:56 <emersion> dnkl, (a) is pretty bad, it means foot breaks on software-rendered compositors

11:56 <robert_mader> emersion: by the way, you'd do me a huge favor if you (or somebody else) could have a look at https://github.com/swaywm/wlroots/issues/1865 - I think it's the only blocker for the "delegated compositing" firefox backend, and sway is the last one to not support it ;)

11:56 <gitlab-bot> swaywm issue 1865 in wlroots "wl_subsurface_place_above ignores parent" [Bug, Surface, Open]

11:56 <emersion> robert_mader, oh ok, will prioritize this!

11:56 <dnkl> emersion: sorry, meant that foot breaks if all three conditions are met

11:57 <emersion> oh i see

11:57 <robert_mader> thanks!

11:57 <emersion> dnkl, so with the protocol spec'ed so that compositors can't mutate buffers, we should be good

12:00 <dnkl> emersion: yup. And if it's spec:ed to *allow* it, I'll update foot to re-render instead of copy. As long as we're only switching between two buffers we'll never hit the memcpy path

15:43 <swick> pq: wouldn't mind

19:06 <robert_mader> emersion, daniels: talking about shm buffer handling, has one of you looked into https://software.intel.com/content/www/us/en/develop/articles/zero-copy-texture-uploads-in-chrome-os.html before? Looks like on intel and similar we could avoid texture uploads from shm altogether

19:08 <daniels> robert_mader: we can't really avoid uploads from shm, because you need to alloc a BO from vgem - it doesn't give you a way to wrap vaddr+len into a BO

19:08 <daniels> so clients can do that today if they alloc using vgem and send via dmabuf

19:09 <robert_mader> oh cool - so all compositors support this in theory?

19:16 <jadahl> daniels: sounds like something gtk3 could benefit from

19:16 <jadahl> as it'll likely never see any hw acceleration

19:17 <hesperos66> is it okay to call something like i.e. `weston_view_geometry_dirty` from a different thread than the one running `wl_display_run`?

19:17 <daniels> hesperos66: afraid not

19:17 <daniels> robert_mader: yeah, as long as your driver can import from vgem, which is ... not all of them

19:17 <daniels> jadahl: EFL does this too

19:18 <hesperos66> daniels: so more or less the compositor has to be single threaded?

19:19 <hesperos66> is there a way to post an operation like `weston_view_geometry_dirty` to event_loop from another thread?

19:22 <daniels> yeah, it pretty much has to be single-threaded - you can spawn helper threads to do background things if you like, but all your interactions with weston API & structures can only be in the main thread

19:23 <daniels> the usual pattern is to create a pipe(), add it as a wl_event_source, and use that to post messages between your main & helper threads

19:24 <jadahl> daniels: so you effectively need a per driver allow-list then?

19:24 <jadahl> in gtk

19:33 <robert_mader> jadahl: or a small check?

19:34 <robert_mader> because gkt should also be able to import, right?

19:39 <emersion> i don't see vgem loaded often

19:40 <daniels> a lot of people don't ship it because it will generally claim card0 and annoy dumb userspace

19:41 <daniels> also it's an unguarded interface which allows userspace to allocate memory outwith ulimit

19:43 <emersion> allocations not counted towards the allocating process?

19:44 <daniels> right, they don't accrue towards your memory limit

19:45 <daniels> (insert opinion about practical usefulness of ulimit here)

19:52 <emersion> :S

19:53 <emersion> oom-killer already random enough, no need to make it even more random

19:55 <robert_mader> hm ok that's the crux of the matter