10:03wlb: weston Issue #873 opened by Debojit Das (new_user_0) Touchscreen not rotating along with display. https://gitlab.freedesktop.org/wayland/weston/-/issues/873
11:25emersion: what should i do about this?
11:25emersion: https://gitlab.freedesktop.org/wlroots/wlroots/-/merge_requests/4529
11:25emersion: that seems… wrong
11:36kennylevinsen: from drm_fourcc.h: "Some Broadcom modifiers take parameters, for example the number of vertical lines in the image. Rseerve the lower 32 bits for modifier type, and the next 24 bits for parameters." - but why would you do that :(
11:37emersion: cc sima daniels ^
11:42pq: did that stuff really make it to kernel upstream?
11:45pq: I'm guessing that kernel reviewers never realized that drm_fourcc.h itself never uses fourcc_mod_broadcom_mod() macro.
11:46pq: or fourcc_mod_broadcom_param()
11:47sima: ok this is hilariously broken and should never have landed
11:47emersion: phew
11:49sima: so from a very quick look at vc4 kernel code it does look fixable
11:49sima: as in, if the param is 0 we use the pitch passed in as metadata and not this funny modifier param business
11:50sima: ideally Someone(tm) would check if we could also rely on the passed-in pitch for when the parameter is set
11:50pq: the doc for these is confusing, but I'm guessing that "vertical lines" is actually the width of "columns", not image width
11:50sima: but yeah good job on tossing 24bits into the dustbin lolz
11:50sima: pq, yeah it's just what addfb calls pitch
11:50sima: or well, should have been
11:50pq: is it? it does not sound like pitch to me
11:51sima: oh I got confused I guess
11:51sima: hm
11:51sima: it's just stored in a register called pitch0 :-)
11:51pq: sounds like there are 4 different valid values for the parameter: 32, 64, 128, and 256
11:52pq: so it shouldn't be any problem to enumerate all accepted modifiers verbatim
11:52sima: hm yeah maybe not sure ...
11:52sima: yeah just enumerate them
11:52sima: and replace the comments above the macros with a "absolutely do not use this in any new code at all"
11:53pq: maybe the kernel patch makes sense, apart from the two unused macros, while the wlroots patch is bogus?
11:53sima: oh normalizing modifiers is complete bogus
11:53sima: I'm just looking for the best way to fix this mess without breaking anything
11:54pq: first patch the kernel to delete the unused macros, and anyone who screams about that is getting modifiers totally wrong in userspace :-)
11:55sima: emersion, I dropped a comment onto that mr
11:55emersion: #define fourcc_mod_broadcom_mod() rand()
11:55sima: pq, it might break the userspace drivers if we delete those
11:56emersion: ty sima
11:56sima: but yeah ideally we ditch them everywhere and replace them with hardcoded enumerations
11:56pq: sima, how could they be right in using those macros?
11:57sima: pq, not right, but also it'd be a build-time regression
11:57pq: "oh, sorry, here's the revert" :trollface:
11:57sima: whether the code is right depends a bit on where they're used
11:58sima: if it's just the modifier for an allocated buffer, then it would work if we switch to "only these explicitly documented combos are correct" world
11:58pq: nope, I don't understand what the bcm modifier doc says.
11:58sima: if the userspace also uses the "canonicalized" values for supported format enumeration purposes, then the userspace code is broken ofc
12:00sima: pq, just looking at the macros the DRM_FORMAT_MOD_*_HEIGHT() that take a parameter seem good
12:00sima: the ones without the parameter are the bogus canonicalized stuff
12:01pq: now I'm thinking the modifier HEIGHT is related to image height
12:01sima: pq, yeah reading the doc the column height seems to be the height of the buffer, rounded up appropriately
12:01sima: pq, yeah
12:01pq: just like stride is related to image width
12:01sima: so the metadata parameter would need to be the alignment value used
12:03sima: or whatever is used to decide how that rounding up works
12:03sima: so yeah I guess the existing modifiers are for the dust bin, and we need new ones
12:04pq: is that SAND format even usable at all? Aren't all APIs everywhere missing "column pitch" from the width,height,stride tuple?
12:05pq: therefore the funny idea of using a format modifier to carry more free parameters
12:06sima: lol c91acda3a380b in the kernel broke this
12:06sima: given no one screamed, we might just be able to toss it outright
12:07sima: this enforces proper "did you list that specific modifier in your plane's modifier list" semantics
12:07sima: which vc4 very much doesn't
12:07pq: \o/
12:07sima: emersion, good news ^^
12:08sima: well only landed in 6.5 but still
12:08pq: but maybe no-one uses these for DRM FBs?
12:08sima: maybe
12:08sima: vc4 has code to support it at least
12:09sima: but yeah quick summary: a) this is totally broken b) we plugged this specific leak at least in kms in 6.5
12:11sima: pq, emersion also I'm thinking that even if someone reports a regression, if we cover the usual values for 720/1080/4k (both i & p) we should be good
12:11sima: to cover regressions, if they pop up
12:11sima: since video
12:11sima: so not super worried about this anymore, but it's still bad
12:11any1: The latest vendor patched kernel for rpi is 6.1
12:11sima: yeeeehaw :-(
12:11sima: they'll notice in time I guess
12:14sima: https://paste.debian.net/hidden/a89e01f3/ example kernel fix if regressions are reported
12:14sima: or whatever magic values are actually used in practice
12:15sima: (I'm guessing in reality is some rounding to dram page size or something like that)
12:24sima: emersion, dropped another comment into the mr, pls ping me since I tend to suck at keeping on top of gitlab pings :-/
12:24emersion: cool ty!
12:25sima: mripard_, https://gitlab.freedesktop.org/wlroots/wlroots/-/merge_requests/4529?commit_id=4e63492f1eea3bbc29f149613f16d4a9d32bcc8f context
12:43Company: so, with compositors doing direct scanout of non-fullscreen windows with this libliftoff work landing everywhere - is it relevant if the application buffers are opaque? Or is it fine to use transparency on overlay planes?
12:43mripard_: sima: thanks
12:44emersion: it's always nice to know when a buffer is opaque
12:45pq: Company, if you can provide guarantees of opaqueness via pixel format or wl_surface opaque region, it does increase the probability of being able use a KMS plane for it.
12:45Company: most application code doesn't bother because it just wasn't an issue
12:45pq: but it's not a strict requirement
12:46Company: and I'm wondering if I should file bugs / add comments for them every time
12:46Company: they just create an AR24 buffer because why not?
12:46pq: I think it would be good to teach people to think about opaqueness.
12:47pq: even if the compositor does not use KMS planes, knowing that a surface is opaque will allow reducing the GPU work.
12:48Company: yeah, the whole opaque region story
12:48pq: isn't that reason enough?
12:49Company: apparently not or everyone would be doing it already...
12:50pq: maybe they didn't know about even that
12:51Company: actually, most of those buffers ended up in GTK and where composited there, that only gets relevant with the recent offloading work
12:52pq: if GTK knew they were opaque, also GTK could probably save in GPU work?
12:52Company: the 2 cases where I recently figured out they use RGBA is the webpage dmabufs used by Epiphany and the videos GStreamer sends to GTK from its GL pipeline
12:52Company: yes, GTK could probably optimize things, too
12:53Company: but it doesn't - even the opaque region is a custom codepath still when we could deduce it from the render tree
12:54Company: I've wondered how much we could gain there - because there's often cases where backgrounds get overdrawn multiple times
12:55Company: like text editors draw the window background first and then draw the text area's background on top
12:55pq: I suppose that depends on the number of polygons painted vs. their area.
12:55Company: but of course, the text area oftenends up with rounded corners and then...
12:57Company: it's 2 huge triangles (or maybe 2 huge and 16 smaller ones if it 9-slices because of rounded corners)
12:57Company: but the problem there is the number of pixels touched
12:57pq: the area, yes
12:58Company: but then, text editors usually aren't limited by draw performance
13:00Company: but I think it could help to do those optimizations to achieve smoother scrolling on lower-powered hardware, like rpi and such
13:00Company: that also tells you how important it is I guess
13:01emersion: could also help battery-powered devices
13:01pq: battery life, yes
13:02Company: yeah, there's probably a lot that could be done there, but it's both hard to measure and there's bigger fish there
13:03Company: nobody is gonna praise longer battery life when they switch from vscode or whatever to vim in a terminal as their editor
13:04Company: and if they did, it'd be because they don't run all the background tasks anymore that are compiling everything on every keypress
13:08sima: Company, yeah reducing wasted memory bandwidth for rendering is a really hard one that needs all the details right
13:09sima: you need incremental drawing with buffer_age and damage rectangles everywhere
13:09sima: that's already a big one depending upon hw
13:20Company: yeah
13:21Company: but the cases that are worth optimizing are always the ones where those features don't work
13:21Company: like scrolling in a text editor turns the damaged region into the full window and all your damage rects don't help you
14:40MrCooper: pq: re https://gitlab.freedesktop.org/wayland/weston/-/issues/871 it's doubtful that compositing would make a big difference with Xorg; in particular, it wouldn't directly affect how XVideo handles colour space conversion and scaling
14:45MrCooper: Company: FWIW, optimizing all those things is particularly important for software rendering, some of the lower-hanging fruit for that
14:45Company: I am well aware of the theory
14:46Company: but I'm also aware that software rendering doesn't even support buffer-age yet
14:47MrCooper: hmm, not even the native Wayland platform?
14:47MrCooper: anyway, should be fixable
14:49Company: LIBGL_ALWAYS_SOFTWARE=1 gtk4-widget-factory
14:49Company: and no damage tracking will happen
14:51Company: all of that is a case of "only 24h in a day" - other stuff is more important, but in theory it'd be cool to have
14:56MrCooper: huh yeah, only GBM platform seems to support buffer age with llvmpipe
14:56zamundaaa[m]: Damage tracking might not happen on the client side, but opaque culling is still done on the compositor side
14:56MrCooper: I suspect it's some kind of silly oversight with the Wayland platform
14:56Company: lavapipe does damage tracking iirc
14:57swick[m]: Company: when you can break API you really should switch any kind of importing buffers to assume the imported image is opaque by default
14:57emersion: that sounds pretty error prone…
14:58Company: swick[m]: people need to specify the buffer format - and they'll always pick RGBA
14:58Company: because RGBA is just less work
14:58Company: see also: cairo_surface_create()
14:58swick[m]: sure, but then just have another HAS_ALPHA flag
14:59swick[m]: and internally switch it to an opaque variant if it's not set
14:59swick[m]: whoever actually needs alpha will notice and turn it on
14:59Company: everyone will turn it on
14:59Company: because otherwise sometimes stuff is black
14:59pq: MrCooper, compositing does not forbid Xv?
14:59Company: people will treat it as _set_not_buffy (TRUE)
14:59Company: *buggy
15:00MrCooper: pq: nope
15:00pq: MrCooper, so there is code to route Xv adapters through a compositing manager?
15:00MrCooper: of course not :)
15:00pq: MrCooper, then how does it work? Breaks into pieces?
15:01swick[m]: Company: dunno man, I think a lot of people just choose RGBA without thinking and if you make them think they will more likely do the right thing
15:01MrCooper: if it's an overlay, the compositing will only affect the colour key, not the video content itself; otherwise it's just a textured blit
15:01swick[m]: but if they always get alpha by accident they will never notice and never have to think
15:01Company: yeah, I know
15:01Company: but your fix doesn't work either
15:02Company: because then they will think for 5s until they find the has_alpha toggle, set it, and continue not thinking about it
15:02pq: MrCooper, sounds like it'll break into pieces then, if it bypasses the compositing manager on its way to screen.
15:02Company: and all that toggle will be known for is the stack overflow posts where people go "oh, it's buggy if you don't set it"
15:03swick[m]: if it's buggy, they need alpha...
15:03MrCooper: pq: an overlay will break if the window isn't displayed at its canonical position, a texture XVideo adapter works fine with compositing
15:04swick[m]: sure, maybe there are cases where only some data contains actual alpha and a lot of other cases don't, but this still seems like a good improvement overall
15:04swick[m]: because you definitely get all those cases where alpha is not required at all
15:04Company: sure, but the way to go there is education
15:05swick[m]: no offense, but you do that by creating APIs that make it hard to do the wrong thing
15:05Company: sure, I'll wait for that API to appear
15:06Company: because for opaqueness the problem is that you need to do deliberate work to check if your stuff is opaque, so the opaque-by-default approach doesn't work
15:06swick[m]: you're basically saying a better API is not better because it can still be misused
15:06swick[m]: and that's really weird to me
15:07pq: MrCooper, interesting. I only have hazy recollections from many years ago, that the coming of compositing managers somehow made Xv break.
15:07Company: I'm saying it's not a better API
15:08swick[m]: it's obvious that it's better. if alpha is not required generally in a call to import a buffer, then the new API gives you opaqueness even if the user mindlessly chooses RGBA
15:09MrCooper: pq: yes, at the time most Xv adaptors were overlay based, and there were issue with compositing; these days Xorg drivers mostly expose only textured adaptors (as does Xwayland) for that reason
15:09Company: no, it's obviously worse
15:09pq: MrCooper, aha, thanks.
15:10Company: because it causes broken results in cases where an alpha-by-default API wouldn't
15:10swick[m]: it breaks by giving and obvious error that can be fixed
15:10Company: yes, but it's giving you an error
15:11swick[m]: yes, and that's a good thing
15:11Company: only if you can guarantee that it is caught during testing
15:11Company: and doesn't end up in users' hands
15:12Company: it's basically why HTML won and XHTML lost
15:13kennylevinsen: no XHTML lost because it added no value
15:13kennylevinsen: strictly XML does not function any different from the non-XML form - it's just aesthetic for the person looking at said markup
15:14Company: it's also much easier to parse spec-compliant XML than it is to write HTML parsers
15:14kennylevinsen: (HTML's deviations from XHTML are now strictly defined, including auto-opening and auto-ending tags)
15:14Company: you can almost use a regexp!
15:15kennylevinsen: Quite the contrary, xml namespaces! dtd you need to download during parsing! :)
15:16kennylevinsen: html just has a handful of rules like "<body> open implicitly ends <head>, body-only tags implicitly open <body>"
15:16Company: xml wants everything to be quoted, too and html can do fine without that
15:17kennylevinsen: that doesn't make it harder to parse, it's just a small syntax difference
15:18Company: I didn't actually check if that's true - it might very well be harder to parse
15:19Company: mostly because I've watched lots of fun talks on how json parsers work
15:19kennylevinsen: yeah, fully spec compliant JSON with all the bells and whistles is... quirky
15:21Company: anyway, the problem with opaqueness is that people will almost always go with transparent and then think they'll "fix it later"
15:21Company: so I think the cairo approach of letting people explicitly say that the want transparency is the best one: It makes them consider if they should care about it, but if they don't want to, they don't end up with brokenness
15:22Company: and that's basically the approach with Wayland/dmabuf formats, too - you have to pick one and it makes you consider which one
15:22ManMower: we should scan client buffers to make sure they actually use alpha, and disconnect them for using opaque ARGB buffers.
15:23Company: kill all the video players!
15:23ManMower: ooi, do they at least set an opaque region?
15:23Company: I don't even know what video players do
15:25kennylevinsen: well, letting people explicitly say that they want transparency vs. defaulting to be opaque kind of sounds like the same thing... :)
15:26Company: *Explicitly say what format they want
15:26Company: so you need to be explict about opaque, too
15:27YaLTeR[m]: ManMower: then game devs will come knocking who set format to xrgb then use alpha channel as internal data storage
15:28Company: like zwp_linux_buffer_params_v1::create() could come without a format and default to XRGB unless you called like zwp_linux_buffer_params_v1::set_format() before
15:28ManMower: YaLTeR[m]: well, 2 things. one is that I was joking, and the other is that I would never suggest scanning an XRGB buffer, because that's a completely legit use case.
15:31Company: you're meant to scan ARGB buffers anyway
15:31kennylevinsen: Those precious 8MB of unused alpha-channel in a 4k buffer...
15:31Company: XRGB is known to be opaque already
15:32kennylevinsen: we're not meant to *scan* ARGB buffers, we're just meant to composite them with blending
15:33kennylevinsen: or offload them if we can
15:33Company: kennylevinsen: it's a fun question if your PNG loader should load into RGB vs XRGB buffers
15:34kennylevinsen: Although I have had a long-standing project idea of a test compositor that actively scanned submitted buffers to report correctness of e.g. damage and opaqueness
15:34ManMower: while I haven't read the entire discussion, this comes to mind as a case where using alpha when not necessary leads to underleveraging available display hardware: https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/1258
15:34Company: with those 20k images, you can end up with almost 100MB of junk bytes
15:34YaLTeR[m]: Company: I think gpus don't like 24-bit formats
15:35Company: YaLTeR[m]: yeah - but GPUs also don't like OOMing
15:35Company: and I think RGB is readable everywhere
15:38Company: also, mpv uses XR30 to play my video, so my GStreamer GTK plugin dissing does not apply to other video players
15:40Company: afaik GStreamer doesn't have an XRGB format though, so I might be able to diss it in general, not just the plugin
15:41Company: but YaLTeR[m] would know that better than me
15:42YaLTeR[m]: when I tried alpha formats from the caps in gst paintable sink, everything broke
15:42YaLTeR[m]: although allegedly it's supposed to work
15:45Company: there's a difference between actually having alpha and making sure to not advertise alpha
15:45Company: and I think GStreamer has no way to have 32bit RGBX buffers
15:45Company: well, unless you use the new dmabuf stuff, where you can just use DRM_FORMAT_XR24
15:46Company: but I was thinking memory RGB buffers
15:47daniels: BGRx and RGBx have always been supported as raw video formats in GSt
15:49Company: good to know - maybe people just forgot to add them to the plugins I looked at
16:14MrCooper: Company: FWIW, non-ancient AMD GPUs don't actually support 3-byte RGB formats, GL_RGB uses 4 bytes per pixel
16:17Company: so it's all faked in the driver again
20:57zamundaaa[m]: Gmail seems to be swallowing some notification emails from fdo gitlab lately... If I don't respond somewhere in time, please feel free to ping me about it here or anywhere else that isn't gitlab
20:58zamundaaa[m]: drakulix: all your responses to my dmabuf MR are missing from my emails for example
20:59zamundaaa[m]: But the ones from Simon seem to be all there
21:00drakulix[m]: zamundaaa[m]: I have had the same problem in reverse. Your answers are missing, but Simons come through.
21:04zamundaaa[m]: Guess we have to check wayland-protocols threads manually for now :/
21:09emersion: or find a better email provider
21:31drakulix[m]: I self-host the address in question and my logs don't show any rejected mails for today. So this isn't necessarily looking like a problem on our side.
21:31emersion: hm
21:49Company: zamundaaa[m]: I've had gmail mark random gitlab.fdo mails as spam
21:54zamundaaa[m]: hmm, nothing in spam
21:54zamundaaa[m]: maybe it's a Gitlab issue
22:01emersion: i tweaked a few knobs, hopefully should be a bit better now