Changeset View
Standalone View
shell/screenpool.cpp
Show First 20 Lines • Show All 98 Lines • ▼ Show 20 Line(s) | |||||
99 | { | 99 | { | ||
100 | return m_primaryConnector; | 100 | return m_primaryConnector; | ||
101 | } | 101 | } | ||
102 | 102 | | |||
103 | void ScreenPool::setPrimaryConnector(const QString &primary) | 103 | void ScreenPool::setPrimaryConnector(const QString &primary) | ||
104 | { | 104 | { | ||
105 | if (m_primaryConnector == primary) { | 105 | if (m_primaryConnector == primary) { | ||
106 | return; | 106 | return; | ||
107 | } | 107 | } | ||
108 | Q_ASSERT(m_idForConnector.contains(primary)); | | |||
109 | 108 | | |||
davidedmundson: Either this is a valid case to be in, and this assert doesn't make sense.
Or we should never… | |||||
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: 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. hoffmannrobert: This is a valid case, and the assert doesn't make sense, it needs to be removed.
In the case… | |||||
110 | int oldIdForPrimary = m_idForConnector.value(primary); | 109 | int oldIdForPrimary = -1; | ||
110 | if (m_idForConnector.contains(primary)) { | ||||
111 | oldIdForPrimary = m_idForConnector.value(primary); | ||||
112 | } | ||||
113 | | ||||
114 | if (oldIdForPrimary == -1) { | ||||
115 | // move old primary to new free id | ||||
116 | oldIdForPrimary = firstAvailableId(); | ||||
117 | insertScreenMapping(oldIdForPrimary, m_primaryConnector); | ||||
118 | } | ||||
111 | 119 | | |||
112 | m_idForConnector[primary] = 0; | 120 | m_idForConnector[primary] = 0; | ||
113 | m_connectorForId[0] = primary; | 121 | m_connectorForId[0] = primary; | ||
114 | m_idForConnector[m_primaryConnector] = oldIdForPrimary; | 122 | m_idForConnector[m_primaryConnector] = oldIdForPrimary; | ||
115 | m_connectorForId[oldIdForPrimary] = m_primaryConnector; | 123 | m_connectorForId[oldIdForPrimary] = m_primaryConnector; | ||
116 | m_primaryConnector = primary; | 124 | m_primaryConnector = primary; | ||
117 | save(); | 125 | save(); | ||
118 | } | 126 | } | ||
▲ Show 20 Lines • Show All 95 Lines • Show Last 20 Lines |
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.