Position OSD a bit farther down
ClosedPublic

Authored by ngraham on Apr 28 2020, 6:36 PM.

Details

Summary

A common user complains is that our OSDs--particularly the volume OSD--are too intrusive
and get in the way of the screen content. For example when adjusting the volume while
watching a full-screen video, the volume change OSD will typically appear right in the
middle of an actor's face.

D20569 was an attempt to use a horizontal OSD to alleviate this issue. It mostly worked,
but IMO it was still positioned too high up.

This patch moved the placement down a little bit to make the OSD appear even farther from
the center of the screen to make it less likely to

I tried not to move it down too far or else it would interfere with subtitles in videos.

Test Plan

With D20569:



Diff Detail

Repository
R108 KWin
Branch
position-osd-a-bit-farther-down (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 27056
Build 27074: arc lint + arc unit
ngraham created this revision.Apr 28 2020, 6:36 PM
Restricted Application added a project: KWin. · View Herald TranscriptApr 28 2020, 6:36 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
ngraham requested review of this revision.Apr 28 2020, 6:36 PM

An alternative I could imagine would be to suppress OSDs entirely when viewing full-screen window content, much like how non-critical notifications are suppressed in this case.

ngraham edited the test plan for this revision. (Show Details)Apr 28 2020, 6:39 PM
kori added a subscriber: kori.Apr 28 2020, 6:48 PM
zzag added a subscriber: zzag.Apr 28 2020, 7:39 PM

I tried not to move it down too far or else it would interfere with subtitles in videos.

The problem with this patch is that the proposed placement area is based on personal preferences of one person. Have you considered placing OSDs somewhere else so they interfere with screen contents less? e.g. in screen corners, etc?

ngraham added a subscriber: filipf.Apr 28 2020, 9:56 PM
In D29263#659354, @zzag wrote:

I tried not to move it down too far or else it would interfere with subtitles in videos.

The problem with this patch is that the proposed placement area is based on personal preferences of one person. Have you considered placing OSDs somewhere else so they interfere with screen contents less? e.g. in screen corners, etc?

I believe @filipf is working on that. One complication is that both the current and proposed OSD designs only looks good horizontally centered, but the placement is done in KWin which has no knowledge of the visual style. @filipf told me that an ugly hack was needed to override the positioning from the plasma theme, so maybe that's an area for improvement, so that Plasma themes can determine for themselves where they'd like the OSD positioned based on its visual design.

Which OSDs do you intend to change and which ones do you intend to remain as is?

Which OSDs do you intend to change and which ones do you intend to remain as is?

This patch is a proposal to make D20569 work better. So the answer would be "all of them" if D20569 lands.

I think the OSD QML should be able to govern its relative screen position. This way one could make a look and feel package where the OSD is a vertical bar in a corner, for instance, like some other popular OS does it. Right now everyone's forced into the same spot.

If we want to change "all of them " we need patches in kscreen (2) and a patch in kwin's osd.

It sucks that it's so fragmented.

I don't think we should care about the KScreen OSD positioning in this discussion. It's a dialog you have to interact with, whereas the volume OSD is just feedback for something you've just pressed and like to be somethign you do while watching a video and so on

I don't think we should care about the KScreen OSD positioning in this discussion. It's a dialog you have to interact with, whereas the volume OSD is just feedback for something you've just pressed and like to be somethign you do while watching a video and so on

Yeah.

If we can control positioning from QML in the plasma theme, I'll happily abandon this. Conceptually that seems more appropriate anyway since that design of the OSD itself largely determines which area of the screen is appropriate for it to appear in. A vertical OSD will be more at home in a corner, a horizontal one will be more at home near the top or bottom, etc.

With the smaller appearance, I'm feeling a bit like it just floats there... for no reason... it gives me the impression than there's no visual connection with anything. It's a bit like it's in a random position, maybe because it's not vertically centered. Could be fine anyway, but I was wondering if there's another place where it would better fit. Maybe top left? Can it avoid panels?

Ping. I think this is a mild improvement

With the smaller appearance, I'm feeling a bit like it just floats there... for no reason... it gives me the impression than there's no visual connection with anything.

Well, that's the truth! :)

I was wondering if there's another place where it would better fit. Maybe top left? Can it avoid panels?

Various folks suggested making it a notification, and then it would show up in the same position as the rest of the notifications in the stack. That might work, though IIRC the suggestion didn't prove very popular.

If we put it in the corner, I feel like a vertical OSD would be better; in fact IMO the optimal positioning inherently depends on the shape of the OSD. However right now we don't have customizable OSDs so all we can really do is make the KWin-determined positioning match the size and shape of the default one. In the long term, I would support making the OSDs changeable via GHNS, as David suggested in D20569--and even better, the GHNS OSD theme would be able to specify a preferred location which KWin would honor. That would be really nice.

However that's a very significant undertaking and not doing this because we want that will just kill this forever while we wait (as it did last time @broulik tried).

ndavis added a subscriber: ndavis.May 19 2020, 6:27 PM

I'm feeling pretty lukewarm about this idea. It's maybe slightly better in some situations, but with the smaller size, I think it'll also be easier to miss if it's not in the center. Since the OSD is smaller, being exactly in the center isn't such a huge problem either. Although, I suppose it would still get in the way of crosshairs if the user was playing a shooter game.

I'm feeling pretty lukewarm about this idea. It's maybe slightly better in some situations, but with the smaller size, I think it'll also be easier to miss if it's not in the center. Since the OSD is smaller, being exactly in the center isn't such a huge problem either. Although, I suppose it would still get in the way of crosshairs if the user was playing a shooter game.

That's the problem that's being at least (supposedly, hopefully) being improved on here. There are other things also typically right in the center of the screen: actors' mouths when watching a movie or a video, for example.

ndavis accepted this revision.May 19 2020, 6:55 PM
This revision is now accepted and ready to land.May 19 2020, 6:55 PM
ndavis added inline comments.May 19 2020, 6:57 PM
placement.cpp
506

why 0.37?

ngraham marked an inline comment as done.May 19 2020, 8:30 PM
ngraham added inline comments.
placement.cpp
506

Existing magic number used for positioning is replaced with a different one to adjust the position.

ngraham marked an inline comment as done.May 19 2020, 8:33 PM

In the absence of a way for the Global Theme or (proposed-but-not-yet-existing OSD themes) to set a custom OSD position, are KWin people okay with this patch to improve the positioning of the new OSD?

In the absence of a way for the Global Theme or (proposed-but-not-yet-existing OSD themes) to set a custom OSD position, are KWin people okay with this patch to improve the positioning of the new OSD?

Non-Breeze OSDs will be affected? If so, that seems bad.

How do you think about putting it horizontally centered at the top of the screen? Maybe even avoiding panels, if possible?
I'd be fine both with "floating a bit under the panel" and "merged with screen/panel top" like krunner.

I don't think it's a problem to change this even for non-breeze themes.

It might be best not to change anything about the position until we can come up with a significantly better solution.

ngraham added a comment.EditedMay 19 2020, 9:00 PM

How do you think about putting it horizontally centered at the top of the screen? Maybe even avoiding panels, if possible?
I'd be fine both with "floating a bit under the panel" and "merged with screen/panel top" like krunner.

I don't think it's a problem to change this even for non-breeze themes.

That seems like it could be a sensible solution, yeah.

However @ndavis might be right that we should come up with a way for themes to override the positioning first--especially if we do that. A square OSD will look kind of weird jammed up at the top of the screen.

I don't think this patch in its current form will cause problems for 3rd-party themes, though. The new position is not much farther down than the current one. It's a pretty small change.

ndavis requested changes to this revision.May 19 2020, 9:06 PM
This revision now requires changes to proceed.May 19 2020, 9:06 PM

I'm a bit confused. Aren't we changing the size for third party global themes as well? Do all global themes set their size?

I'm a bit confused. Aren't we changing the size for third party global themes as well? Do all global themes set their size?

That depends on whether or not the global theme has its own OSD qml files. You will find that some do, while others don't. So yes, there will still be square OSDs out there.

meven added a subscriber: meven.May 20 2020, 9:24 AM

In the absence of a way for the Global Theme or (proposed-but-not-yet-existing OSD themes) to set a custom OSD position, are KWin people okay with this patch to improve the positioning of the new OSD?

Personally I would be fine with it.
It changes the default for the best.

It will change all themes yes, but all had the same issue.
Having a setting for themes to choose an OSD placement is an orthogonal matter and a preexistant issue IMO.

And users will really enjoy this with D20569 landed already.

As a side note, a missing feature of this API would be to be able to select the screen to display the Osd on, so that KScreen could share the same code path and theming.
And maybe another to prevent osd to a specified screen, i.e presentation windows like libreoffice impress.

@davidedmundson ?

ndavis accepted this revision.May 20 2020, 2:43 PM
This revision is now accepted and ready to land.May 20 2020, 2:43 PM
zzag added a comment.May 20 2020, 3:04 PM

In the absence of a way for the Global Theme or (proposed-but-not-yet-existing OSD themes) to set a custom OSD position, are KWin people okay with this patch to improve the positioning of the new OSD?

Speaking for myself, I'm not. I don't like 0.37 because it's not based on anything. Why not 0.39 or 0.4242?

In D29263#673249, @zzag wrote:

In the absence of a way for the Global Theme or (proposed-but-not-yet-existing OSD themes) to set a custom OSD position, are KWin people okay with this patch to improve the positioning of the new OSD?

Speaking for myself, I'm not. I don't like 0.37 because it's not based on anything. Why not 0.39 or 0.4242?

The existing magic number is also totally random, and does not even match what it says it will do (it results in an OSD position about 45% up, not in the bottom third as it claims.

Do you have a suggestion?

zzag added a comment.May 21 2020, 6:50 AM

The existing magic number is also totally random, and does not even match

I wouldn't say that fractions such as 1/2, 1/4, 1/3, 2/3 and so on are that random.

what it says it will do (it results in an OSD position about 45% up, not in the bottom third as it claims.

The placement area and the screen area are different.

Do you have a suggestion?

area.top() + 2 * area.height() / 3 - c->height() / 2?

ngraham updated this revision to Diff 83113.May 22 2020, 2:36 PM
ngraham marked an inline comment as done.

Address review feedback

ngraham updated this revision to Diff 83114.May 22 2020, 2:37 PM

don't need to include cmath anymore

zzag added inline comments.May 22 2020, 2:37 PM
placement.cpp
37

Unused include, remove it?

ngraham marked an inline comment as done.May 22 2020, 2:37 PM
zzag accepted this revision.May 22 2020, 2:37 PM
ngraham closed this revision.May 22 2020, 2:38 PM