D13853: Fix setting primary connector if primary output changed

[Robert Hoffmann]

hoffmannrobert marked an inline comment as done.
hoffmannrobert added inline comments.

INLINE COMMENTS

> davidedmundson wrote in screenpool.cpp:108
> Either this is a valid case to be in, and this assert doesn't make sense.
> 
> Or we should never be in this case, and the patch doesn't make sense.
> 
> I don't fully understand the background of how we end up in this situation to say which it is.

This is a valid case, and the assert doesn't make sense, it needs to be removed.

In the case described in the test plan, the old primary display ist HDMI-3, the new primary is HDMI-2. If HDMI-2 is plugged in and "extend to right" is selected in the OSD, the QGuiApplication::screenAdded signal starts ShellCorona::addOutput().

This checks, if the new screen is redundant (ShellCorona::isOutputRedundant). At this time, both have the same geometry (two equal screens, QRect(0,0 1920x1080)), so yes, the new screen is considered redundant and not added to the screen pool.

There is already some remedy against this wrong assumption:
On the signal QGuiApplication::primaryScreenChanged ShellCorona::primaryOutputChanged is called. Here, with m_reconsiderOutputsTimer.start() ShellCorona::reconsiderOutputs() is run at a later time, which will correct this.

But that's too late for the primary change: before reconsiderOutputs()  is run, ScreenPool::setPrimaryConnector( newPrimary->name() ) is called. Here, m_idForConnector does not contain this new primary, it only knows "HDMI-3", which is still mapped to 0.

If the Q_ASSERT is disabled, m_idForConnector.value(primary (The new primary HDMI-2!) ) returns 0, so both HDMI-2 and HDMI-3 are mapped to 0 in m_idForConnector, and m_connectorForId[0] is set to "HDMI-2", so the mapping to HDMI-3 is lost.

The other case, booting with HDMI-2 and hotplugging HDMI-3 works, because in ShellCorona::isOutputRedundant, geometries are different: HDMI-2 has: QRect(0,0 1920x1080) and HDMI-3: QRect(1920,0 1920x1080), so one doesn't contain the other.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D13853

To: hoffmannrobert, #plasma, mart, davidedmundson
Cc: davidedmundson, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20180704/7c183d61/attachment-0001.html>