[07:23] <Saviq> Good friday o/
[07:51] <Saviq> alan_g hey, so unless I'm missing something, it's not about fullscreen or not - in BasicWindowManager we only ever respect the window-requested `output_id`, not that from `modifications`
[07:53] <alan_g[m]> Where are you looking?
[07:57] <Saviq> Really, anywhere in `BasicWindowManager`; `::display_area_for` only takes the `WindowInfo`, not the `WindowSpecification`
[07:58] <Saviq> Only `::update_attached_and_fullscreen_sets` takes `modification.output_id` into account
[07:59] <Saviq> And `::place_and_size_for_state`, but only to decide whether to even look at anything else
[08:06] <Saviq> I could, of course, override `top_left` in `FrameWindowManager`, but it probably makes more sense to actually respect the modifications one?
[08:23] <alan_g[m]> Yeah, the only place I see using it is `place_new_surface()` - which is called to populate the default `requested_specification` for `WindowManagementPolicy::place_new_window()`. I think we need the same logic in `place_and_size_for_state()`
[08:35] <alan_g[m]> ...and, like you say, `place_and_size_for_state()` uses `display_area_for(window_info)`, without considering the `modifications.output_id()`.
[08:48] <alan_g[m]> Hmm, even more "interesting" `case mir_window_state_fullscreen:` calls `display_area_for` instead of using the existing `display_area`.
[08:55] -GitHub[m]:#mir-server- **[MirServer/mir]** AlanGriffiths opened [pull request #2700](https://github.com/MirServer/mir/pull/2700): [miral] Respect the output id supplied when modifying a window
[09:36] -GitHub[m]:#mir-server- **[MirServer/mir]** AlanGriffiths requested a review from Saviq for [pull request #2700](https://github.com/MirServer/mir/pull/2700): [miral] Respect the output id supplied when modifying a window
[10:24] <Saviq> While that looks better, it doesn't help in Frame, 'cause we're spoofing the state to be maximized… Should `::confirm_placement_on_display` take `modifications.output_id` into account as well, or does maximized with `output_id` not make sense?
[10:29] <Saviq> We're doing the spoofing to respect exclusive zones (i.e. OSK). Which is, in itself, a disuptable thing to do, but we have no other mechanism to inform app of occluded areas atm.
[10:36] <alan_g[m]> You're right about the spoofing. It will only work because `place_new_surface()` does respect the output_id and that is called for an initial placement
[10:38] <alan_g[m]> So, we could spoof with something like:... (full message at <https://libera.ems.host/_matrix/media/r0/download/libera.chat/f1302894bb148d8f9680b42bc51c8839f8bdf7f6>)
[10:39] <Saviq> Ah. funky. Will try
[10:40] <alan_g[m]> NB I'm not suggesting this is "good"
[10:41] <Saviq> I know, but not much worse than what we do currently. It's at least inconsistent that we respect `WindowInfo::output_id`, but not `WindowSpecification::output_id` when maximized
[10:42] <Saviq> Which nudges me we should make a call (or respect the protocol, if that's defined there), and stick to it.
[10:45] <alan_g[m]> I think the problem is that we trying to use the default placement logic to override the default placement logic
[10:45] <Saviq> That, too :)
[10:45] <Saviq> But even despite that, IMO we need to be consistent on respecting, or not, `::output_id` when *maximized
[10:45] <alan_g[m]> Just having Frame use its own place_and_size_for_state would be better
[10:46] <Saviq> That basically was https://github.com/MirServer/ubuntu-frame/pull/29
[10:46] <alan_g[m]> Well, which protocol do you want to respect? wl-shell and xdg-shell only specify an output for fullscreen requests.
[10:47] <alan_g[m]> Wheras mir-shell...
[10:47] <alan_g[m]> Our window management API can what we choose
[10:52] <alan_g[m]> E.g. if a modify request specifies an output, then we can choose to respect it regardless of the window state. But xdg-shell clients won't specifiy it unless it's a fullscreen request. But do we remember the output on new requests, or reset it?
[11:07] <Saviq> Yeah, I suppose maximized with output_id is not something we explicitly enabled, that it works is more of an accident, but that's something that I think we should fix one way or the other.
[11:16] <alan_g[m]> Well, it doesn't work - which is why we spoof it in Frame
[11:29] <alan_g[m]> So, the miral API isn't exposing the right primitives. What we have is "place_and_size_for_state", what we want is "place_and_size_with_options", and the options are derived from the state
[12:05] <Saviq> > Well, it doesn't work
[12:05] <Saviq> IIUC maximized with output_id _does_ work, that's what we (ab)use in Frame to have things fullscreened, but respect the exclusive zones of the OSK…
[12:56] <alan_g[m]> I think what currently happens is we fullscreen with output_id (which places it on the output) and then maximise to respect the application_zone of that output.
[12:59] <alan_g[m]> Oh, I misunderstood. You mean it uses the output_id already associated with the window_info
[13:03] <alan_g[m]> Yeah, it is all a bit inconsistent
[13:33] <Saviq> Looks like that did it, thanks!
[14:13] <alan_g[m]> One of these cycles miral needs reworking! This didn't break any of the `WindowPlacement*` tests.
[15:08] -GitHub[m]:#mir-server- **[MirServer/mir]** Saviq assigned  to [pull request #2700](https://github.com/MirServer/mir/pull/2700): [miral] Respect the output id supplied when modifying a window
[16:59] <alan_g[m]> Have a good weekend! 0/
[17:00] <Saviq> Same \o
[17:30] -GitHub[m]:#mir-server- **[MirServer/mir]** bors[bot] merged [pull request #2700](https://github.com/MirServer/mir/pull/2700): [miral] Respect the output id supplied when modifying a window