[01:36] WTF Linux? [01:37] If I get interrupted in the middle of recvmsg while receiving file descriptors do you _really_ dup() all those descriptors and give them to me twice? [01:44] RAOF: dup would mean different fd values... ? [02:00] duflu: Precisely. [02:00] Umm kay [02:00] YES [02:01] This is, obviously, somewhat awkward. === chihchun_afk is now known as chihchun [04:09] RAOF: Could you take a glance at this? https://code.launchpad.net/~mir-team/mir/fix-1339700-take2/+merge/226534 [04:16] duflu: Sure. [04:23] duflu: Why do you think that “while(count > 0) cv.wait(lock);” would be more efficient than “cv.wait(lock, [](){ count > 0 });”? [04:24] RAOF: Because there's no lambda required. No callback into one, no inlining, and no extra work for the compiler. So even if it could be made as efficient as the "while" syntax it would still require a lot more work by the compiler to do so [04:26] Not to mention it's easier to read. There's always a loop to wait on a condition, so it's nicer to get people to read it as "while (something)" instead of "do_stuff(until not cond)" [04:34] I don't know; wait(for_thing) pretty easy to understand. [04:39] After optimizations it should generate the same code. But the while syntax is even easier to read, and requires no compiler optimization effort to generate optimal code [04:42] Roughly the inverse code, but pretty much, yeah. [04:43] However, I think the lambda code is easier to read. [04:43] So, personal taste :) [04:45] I did notice at some point in the past (Mir) developers preferred reading all logic lef-to-right in long lines. Prior to that most developers read top-to-bottom [04:46] At a guess, that'd be the influence of functional programming.. [04:49] The lambda asserts state, the loop describes how to get that state.. [05:30] RAOF: I think "while()" is a more reliable assertion in C-anything. But that's just me I guess [05:31] It's also clearer that the condition is tested while locked (and must be) [05:32] It's not really an assertion, though; it's a proceedure. [05:32] RAOF: Knowing bare minimum about the C language it is a logical assertion :) [05:33] while (A) B; means A is no longer true after [05:33] And then, lunch === chihchun is now known as chihchun_afk [06:28] Of *course* I can't reproduce that bug now that I've got some debugging in place. How stupid of me to presume I might be able to.. === chihchun_afk is now known as chihchun === chihchun is now known as chihchun_afk [12:05] alf_: hey, nested servers GL contexts - they are not getting alpha buffers are they? [12:08] alf_: no they can, it just seems I'm not configuring things correctly to get a gl context with alpha [12:09] a nested server has the RGBA of the nested gl context decided based on the current format of the display output. [12:09] it seems the format QtComp is getting does not have alpha enabled [12:11] Qt also reports that the GL context it is getting has no depth of stencil buffers either - which is odd, as I would expect rendering errors as a result, but visually it looks ok [12:12] depth _or_ stencil [12:13] greyback: Are you referring to a context you get with mg::Display::create_gl_context() or the one that is current when you make_current() [12:13] alf_: an excellent question [12:16] alf_: yeah I'm referring to mg::Display::create_gl_context() [12:17] greyback: Ok, I see that's broken, it always returns zero alpha [12:17] greyback: (broken for nested) [12:17] alf_: ok, that was confusing me. Thanks! [12:18] greyback: what behavior do you need? [12:20] alf_: I just want to read the RGBA/depth/stencil buffer sizes into Qt. But I was doing that for a temporary context I created with mg::Display::create_gl_context - so Qt thought it had no alpha/depth/stencil buffer [12:21] but instead I can just read that from the current context when I call make_current() [12:21] greyback: do you get alpha for the context you get from make_current()? [12:21] alf_: I need to check, but I think so yes [12:27] greyback: as you mentioned, by default the surface/context RGBA a nested server uses matches the format of the real output framebuffer (in the system compositor), which is probably RGB. You can override this with the_display_configuration_policy(). Not sure if unity8 already does this. For depth/stencil you need to override the_gl_config() [12:28] alf_: we override the_gl_config() anyway [12:28] greyback: that's only for depth/stencil though [12:28] alf_: indeed. So you recommend we override the_display_configuration_policy too? In unity8? [12:29] greyback: question is, why do you need alpha for unity8's surface? [12:30] alf_: we don't actually. Qt does complain if depth & stencil missing though [12:30] greyback: ok, but you set that with the_gl_config(), so I am confused about the actual problem... [12:32] alf_: there may not be an actual problem, aside from what is returned by mg::Display::create_gl_context not being what I expected === olli_ is now known as olli === greyback is now known as greyback|lunch === greyback|lunch is now known as greyback [14:06] mterry: hey, are you available for some help/thots on debian control file changes? [14:06] kgunn, sure [14:06] mterry: Hi! I noticed that in USC trunk we are waiting for two frames to be posted before marking a session as ready. Is this a workaround for some unity8 issue? [14:06] kgunn, is this more ABI breakage stuff? :) [14:07] alf_, it was a workaround for *something* -- without that we'd get black frames first. Either Qt or Mir or some driver code was posting black frames [14:07] mterry: nope, more like shuffling some libraries around to make them shared libraries [14:07] https://code.launchpad.net/~mir-team/mir/libmirshared/+merge/226670 [14:08] mterry: ok, I guess I should keep it in the refactoring then :) [14:09] mterry: ...oh, and alan_g brot up this bug...so maybe it is [14:09] https://bugs.launchpad.net/mir/+bug/1293944 [14:09] Ubuntu bug 1293944 in Mir "Mir deb packages with versioned names cannot be installed simultaneously any more" [Medium,Triaged] [14:09] but the discussion started with the mp :) [14:09] and adding packages to the control [14:10] kgunn, so you need to add a new libmirshared1 package? [14:11] mterry: yeah, effectively....and instead of just reading [14:11] https://www.debian.org/doc/debian-policy/ch-controlfields.html [14:11] and hoping to get it right, alan_g was wanting someone with some experience to possibly guide [14:13] kgunn, k [14:13] mterry: there's more problems there than just adding a new library. We've existing libraries that ought (AFAICS) to be versioned. [14:13] alan_g, hello! [14:13] mterry: hi [14:13] alan_g, like libmirplatform [14:14] yes [14:14] and mesa/libmirclientplatform.so [14:14] and android/libmirplatformgraphics.so [14:15] alan_g, so many ABIs! [14:15] I can (mostly) sort out the cmake side of things but the packaging files are "magic" [14:16] alan_g, well, since you'll be changing the name of the files it installs (they'll have ABI version numbers now), you at least get to avoid the pain of breaks/replaces [14:16] alan_g, do you have a branch that does the cmake side of things? [14:17] mterry: I started with the new library. Not touched the existing ones yet. (Was trying to understand things first.) [14:18] alan_g, ok, let me look at existing packaging and recommend a thing [14:18] mterry: yeah, thanks...i was thinking a brief review of where we are would be helpful [14:18] And, maybe mesa|android/libmirplatformgraphics.so doesn't matter as it implements an API defined in libmirplatform. [14:20] alan_g, so libmirshared would be in practice an internal library, right? no -dev? [14:22] alan_g, none of these libraries have .symbols files :( [14:22] mterry: I think it needs a -dev - as there are headers [14:22] alan_g, are they public headers though? Like, will third-party packages build against it? Doesn't hurt to have them be public, but just asking [14:24] there are public headers (I'm looking to see how these get published now) [14:27] Ah! they are installed by mircommon-dev.install [14:28] alan_g, ah... hm. So you'll have to add a breaks/replace on libmirshared-dev [14:29] Or rename to libmircommon ;) [14:29] Which would work [14:29] alan_g, fair [14:30] alan_g, you'd probably want to rename mircommon-dev to libmircommon-dev then [14:31] So that would need a replace anyway? [14:35] alan_g, yeah for its old name. You might want to do all of Provides/Conflicts/Replaces for the old name, so the old package is removed on upgrade [14:36] mterry: and I can just add "libmircommon1 (= ${binary:Version}), " to that and the libraries that use it? [14:37] alan_g, well not to the libraries that use it, they'll have automatic dependencies created by shlibs:Depends [14:37] alan_g, but for libmircommon-dev, yes [14:38] OK. I'll try that for now. Is there an easy way for me to test/validate my changes? [14:39] alan_g, try an upgrade from a PPA and make sure it installs new packages fine and removes the old package... [14:39] alan_g, or you could use dpkg -i locally and make sure it complains correctly, but it won't automatically remove package, it will just error out [14:40] mterry: I'm not at all familiar with packaging - how do I get it on a PPA? [14:41] alan_g, do you have a PPA associated with your LP user? [14:41] Not if I had to do something to get one [14:42] I see "Create a new PPA" [14:42] alan_g, yup, do that [14:42] alan_g, just make some random testing PPA for yourself [14:43] done [14:44] alan_g, OK, now when you're ready, you can build a source package locally with debuild -S [14:44] from the checkout root? [14:45] alan_g, yeah [14:45] alan_g, and then do... dput ppa:alan-griffiths/NAMEOFYOURPPA ../mir_VERSION.changes [14:45] ah, junk-ppa I see [14:45] :) === dandrader is now known as dandrader|afk === dandrader|afk is now known as dandrader === dandrader is now known as dandrader|lunch [16:54] mterry: Sorry to keep bugging you. "debuild -S" gets as far as debsign: gpg error occurred! Aborting... [16:55] alan_g, oh right... do you have a gpg key that LP knows about? [16:55] mterry: yes [16:56] alan_g, OK [16:57] alan_g, so do "dch -i" and put some dumb comment in there, make sure to change UNRELEASED to utopic [16:57] alan_g, then try debuild -S again [16:58] * alan_g looks guilty - he's still on trusty === dandrader|lunch is now known as dandrader [17:17] alan_g, trusty! tsk tsk :) [17:17] alan_g, you can still push to the PPA from trusty, but you won't be able to test the upgrade path on your local machine that way [17:32] dednick, is a MirPromptSessionEvent event tied to a surface? ie. does it happen in relation to a specific surface? [17:32] * dandrader clueless about prompt sessions [17:36] alan_g, you there? === alan_g is now known as alan_g|EOD [17:38] dandrader: not really [17:41] racarr__ ping [17:41] dandrader: not tied to the surface [17:41] dandrader: go directly to the client prompt session. [17:41] dednick, oh.... [17:42] dandrader: Hi :) welcome back [17:43] racarr__, hi [17:43] and thanks :) [17:44] racarr__, so in mir, MirEvents are always tied to a surface, right? [17:44] mterry: something still went wrong - but I'll deal with it tomorrow [17:44] racarr__, or at least that's what it seems to be from reading the client api [17:45] alan_g|EOD, k, poke me when I'm online [17:48] racarr__, ie, MirEvents are always and will always be directed to some MirSurface of some client. i.e., the client never gets a MirEvent directly from the MirConnection or something. always through the handler set by mir_surface_set_event_handler [17:50] dandrader: That sounds right to me [17:53] racarr__, ok. but then we have MirPromptSessionEvent in MirEvent... [17:53] racarr__, which, as dednick said, is not tied to a surface [17:56] hmm I missed that [17:56] *looks* [18:03] dandrader: I wonder if those are just consumed in the client library? i.e. the lambda in MirPromptSession::MirPromptSession in the client library. [18:05] racarr__, which then generates a mir_prompt_session_state_change_callback call to the api user? [18:05] dandrader: SEems right [18:06] racarr__, I guess so, but then it seems wrong to me to have that in a MirEvent which is supposed to contain events that reach the api user through mir_event_delegate_callback [18:07] racarr__, would be like it's exposing an implementation detail to the API user [18:08] racarr__, right? [18:11] dandrader: I would agree yes [18:11] I am not a MirPromptSession expert so to speak lol [18:11] racarr__, me neither. will talk to dednick tomorrow [18:11] but yeah...I think MirEvent is a strange chocie [18:11] ive always felt it was strange to put [18:11] anything besides [18:12] Input events in "MirEvent" [18:12] racarr__, even surface resizes? [18:12] I kind of think its better if you just have [18:13] resize events or surface events or whatever [18:13] but there is no need to group them all in the "all in one event" [18:13] struct [18:14] racarr__, you do if they are going all to be sent to the same callback [18:14] maybe they shouldnt be though because [18:14] the callback kind of has funny semantics right, i.e. called from different threads [18:14] depending on the event type. [18:16] racarr__, hmm, I think it's more like a design choice: "have 1 callback and put a type field in the event" or "have one callback per event type and remove the type field" [18:17] historically the first option is more popular [18:18] I think its kind of a misguided attempt to save typing [18:19] at the expense of a small amount of coupling (e.g. in ABI breaks) between [18:19] what could be unrelated structs [18:19] lol [18:19] but in this situation it probably doesnt matter much [19:28] === dandrader is now known as dandrader|afk [20:22] yay [20:22] finished one pass through the whole QPA [20:41] racarr__: thanks! [20:47] racarr__: do you have review comments? [21:05] greyback: Just mailed them [21:05] greyback: The most concerning thing for me is some of the signal usage [21:06] and things that look like Q_EMIT address-of-value-reference-passed-by-mir [21:06] racarr__: what line numbers are you using? [21:06] oh sorry I think all line numbers are in [21:06] the respective implementation file [21:07] for [21:07] the class in latest rev [21:07] racarr__: ah those are the class name [21:07] ok [21:07] yes sorry my review strategy [21:07] was I wrote down all the classes [21:07] shuffled them in to a random order [21:07] and then started reading [21:07] lol [21:08] oh also paste [21:08] is a little alarming [21:08] Q_EMIT of pointers to mir objects done with care, the objects those pointers point to, qtmir has a shared_ptr to, so the pointer should always be valid [21:08] racarr__: I /think/ paste is what we've had in unity-mir for some time :) [21:09] yes ive just never reviewed it before lol [21:09] it seems like it hsould at least have the focus check [21:09] focus check? [21:09] You shouldn't be able to paste unless you are focused at least right [21:10] otherwise why not just paste all the time and see what you get [21:10] lol [21:11] I saw some of the Q_EMITs had comments like in SessionAuthorizer...but for example SurfaceConfigurator::attribute_set [21:12] if someone connects to that with a queued signal it could crash right? [21:13] true. sadly there's no way to enforce a direct connection [21:13] but I think that class will go away, since now there's the SurfaceObserver [21:14] oh yeah that should go away [21:17] requestAuthorizationForSession "but it seems like this non blocking behavior requires code connecting to the signal to make the right choice." - it does indeed [21:17] one needs to use a direct connection to that signal to properly authorize sessions