[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-panel-devel
Subject:    D6011: let's continue in debug code instead of returning from XRandRConfig::applyKScreenConfig
From:       Christoph Lutz <noreply () phabricator ! kde ! org>
Date:       2017-05-29 14:33:38
Message-ID: differential-rev-PHID-DREV-ept47mzdjcfrwowh6lio-req () phabricator ! kde ! org
[Download RAW message or body]

clutz created this revision.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.

REVISION SUMMARY
  There is code for some yet not fixed bug in XRandRConfig::applyKScreenConfig that \
should print debug information in case of currentMode is NULL. This situation should \
not happen but was seen in the past. After printing the debug info, the method \
directly returned from applyKScreenConfig, omitting all the other potentially useful \
initializations code that comes afterwards.  
        
  
  From my POV this is wrong and we had situations in which desktop initialization \
works better if we don't return here. That's why I changed "return" into "continue".

TEST PLAN
  The concrete situation in which I regularily got this currentMode == NULL situation \
leads to the following bug:  
        
    - kernel 3.13.0
    - physical pc with two displays, one 4:3 and the other 16:9 format
    - forced the system to always start in clone-mode if there is not
      yet a user profile --> we patched kscreen therefore
    - doing the following workflow:
        - log in --> sessions comes up in clone mode (as wished)
        - autorandr-gui --> "extended desktop - left"
        - press OK and DON't store the profile (so no change is stored)
        - log off
        - log in again
    - The following things happen:
        - Now we run into this "currentMode == NULL" situation (message in log file)
        - the screens come up in clone mode (as the previous settings were not saved)
        - BUG: the resolutions are wrong (parts from one of the two screens are cut).
               applyKScreenConfig returned before it was finished.
        
  
  With this chage, this bug no longer occures and the resolution is as expected set \
to the best commonly available resolution.

REPOSITORY
  R110 KScreen Library

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

AFFECTED FILES
  backends/xrandr/xrandrconfig.cpp

To: clutz
Cc: plasma-devel, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, \
jensreuterberg, abetts, sebas, apol, mart, lukas


[Attachment #3 (unknown)]

<table><tr><td style="">clutz created this revision.<br />Restricted Application \
added a project: Plasma.<br />Restricted Application added a subscriber: \
plasma-devel. </td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px \
8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; \
background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); \
display: inline-block; border: 1px solid rgba(71,87,120,.2);" \
href="https://phabricator.kde.org/D6011" rel="noreferrer">View \
Revision</a></tr></table><br /><div><strong>REVISION SUMMARY</strong><div><p>There is \
code for some yet not fixed bug in XRandRConfig::applyKScreenConfig that should print \
debug information in case of currentMode is NULL. This situation should not happen \
but was seen in the past. After printing the debug info, the method directly returned \
from applyKScreenConfig, omitting all the other potentially useful initializations \
code that comes afterwards.</p>

<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" \
data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px \
&quot;Menlo&quot;, &quot;Consolas&quot;, &quot;Monaco&quot;, monospace; padding: \
12px; margin: 0; background: rgba(71, 87, 120, 0.08);"></pre></div>

<p>From my POV this is wrong and we had situations in which desktop initialization \
works better if we don&#039;t return here. That&#039;s why I changed \
&quot;return&quot; into &quot;continue&quot;.</p></div></div><br /><div><strong>TEST \
PLAN</strong><div><p>The concrete situation in which I regularily got this \
currentMode == NULL situation leads to the following bug:</p>

<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" \
data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px \
&quot;Menlo&quot;, &quot;Consolas&quot;, &quot;Monaco&quot;, monospace; padding: \
                12px; margin: 0; background: rgba(71, 87, 120, 0.08);">    
- kernel 3.13.0
- physical pc with two displays, one 4:3 and the other 16:9 format
- forced the system to always start in clone-mode if there is not
  yet a user profile --&gt; we patched kscreen therefore
- doing the following workflow:
    - log in --&gt; sessions comes up in clone mode (as wished)
    - autorandr-gui --&gt; &quot;extended desktop - left&quot;
    - press OK and DON&#039;t store the profile (so no change is stored)
    - log off
    - log in again
- The following things happen:
    - Now we run into this &quot;currentMode == NULL&quot; situation (message in log \
                file)
    - the screens come up in clone mode (as the previous settings were not saved)
    - BUG: the resolutions are wrong (parts from one of the two screens are cut).
           applyKScreenConfig returned before it was finished.</pre></div>

<p>With this chage, this bug no longer occures and the resolution is as expected set \
to the best commonly available resolution.</p></div></div><br \
/><div><strong>REPOSITORY</strong><div><div>R110 KScreen Library</div></div></div><br \
/><div><strong>REVISION DETAIL</strong><div><a \
href="https://phabricator.kde.org/D6011" \
rel="noreferrer">https://phabricator.kde.org/D6011</a></div></div><br \
/><div><strong>AFFECTED \
FILES</strong><div><div>backends/xrandr/xrandrconfig.cpp</div></div></div><br \
/><div><strong>To: </strong>clutz<br /><strong>Cc: </strong>plasma-devel, ZrenBot, \
spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, \
mart, lukas<br /></div>



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic