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

List:       kde-panel-devel
Subject:    D3498: Add scaling to DRM backend
From:       Roman Gilg <noreply () phabricator ! kde ! org>
Date:       2017-03-31 12:43:44
Message-ID: 20170331124343.67810.19863.0171F750 () phabricator ! kde ! org
[Download RAW message or body]

subdiff added inline comments.

INLINE COMMENTS

> davidedmundson wrote in drm_output.cpp:104
> It's good design to not have member variables that can ever be in a corrupt state, \
> it reduces chance for bugs being introduced later, which is why I'm against \
> changing it. 
> Dividing two integers is not going to make any performance difference whatsoever \
> and an int divided by an int will result in the same every single time.

Ok, fine with me. Can you now expand the testing parameter from the other Diff, so \
it's possible to use it in DRM mode? I tried to set a hard coded value in \
DrmOutput::setScale in order to test different scale factors, but the results were \
garbage, so I think (hope) that this hard coding doesn't set the scale factors \
elsewhere correctly and the testing parameter is necessary.

REPOSITORY
  R108 KWin

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

To: davidedmundson, graesslin, subdiff, #plasma
Cc: subdiff, #kwin, plasma-devel, kwin, progwolff, lesliezhai, ali-mohamed, \
hardening, jensreuterberg, abetts, sebas, apol


[Attachment #3 (text/html)]

<table><tr><td style="">subdiff added inline comments.
</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/D3498" rel="noreferrer">View \
Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div \
style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: \
3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; \
border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; \
background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; \
text-decoration: none;" href="https://phabricator.kde.org/D3498#inline-21684" \
rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: \
bold;">davidedmundson</span> wrote in <span style="color: #4b4d51; font-weight: \
bold;">drm_output.cpp:104</span></div> <div style="margin: 8px 0; padding: 0 12px; \
color: #74777D;"><p style="padding: 0; margin: 8px;">It&#039;s good design to not \
have member variables that can ever be in a corrupt state, it reduces chance for bugs \
being introduced later, which is why I&#039;m against changing it.</p>

<p style="padding: 0; margin: 8px;">Dividing two integers is not going to make any \
performance difference whatsoever and an int divided by an int will result in the \
same every single time.</p></div></div> <div style="margin: 8px 0; padding: 0 \
12px;"><p style="padding: 0; margin: 8px;">Ok, fine with me. Can you now expand the \
testing parameter from the other Diff, so it&#039;s possible to use it in DRM mode? I \
tried to set a hard coded value in DrmOutput::setScale in order to test different \
scale factors, but the results were garbage, so I think (hope) that this hard coding \
doesn&#039;t set the scale factors elsewhere correctly and the testing parameter is \
necessary.</p></div></div></div></div></div><br \
/><div><strong>REPOSITORY</strong><div><div>R108 KWin</div></div></div><br \
/><div><strong>REVISION DETAIL</strong><div><a \
href="https://phabricator.kde.org/D3498" \
rel="noreferrer">https://phabricator.kde.org/D3498</a></div></div><br \
/><div><strong>To: </strong>davidedmundson, graesslin, subdiff, Plasma<br \
/><strong>Cc: </strong>subdiff, KWin, plasma-devel, kwin, progwolff, lesliezhai, \
ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol<br /></div>



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

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