[Refactoring] change qt math function to cmath analog #89

pperehozhih · 2018-01-18T22:40:10Z

Issue #36
change

qMax -> std::max
qMin -> std::min
qCeil -> std::ceil
qFloor -> std::floor

crackedmind · 2018-01-18T22:53:53Z

✅ Build kepka 1.0.59-saswpqds completed (commit dd608d56cb by @pperehozhih)

berkus

Implement missing conversions to std::clamp

berkus · 2018-01-23T18:08:48Z

Telegram/SourceFiles/boxes/share_box.cpp

@@ -370,7 +370,7 @@ void ShareBox::Inner::repaintChatAtIndex(int index) {

 	auto row = index / _columnCount;
 	auto column = index % _columnCount;
-	update(rtlrect(_rowsLeft + qFloor(column * _rowWidthReal), row * _rowHeight, _rowWidth, _rowHeight, width()));
+   update(rtlrect(_rowsLeft + std::floor(column * _rowWidthReal), row * _rowHeight, _rowWidth, _rowHeight, width()));


Tab - replace with spaces.

Did you mean the opposite? Replace spaces with tab? Currently, Kepka codebase mainly uses tabs.

berkus · 2018-01-23T19:39:20Z

Telegram/SourceFiles/history/history_inner_widget.cpp

 		}
 		qint32 descAtX = (descMaxWidth - _botAbout->width) / 2 - st::msgPadding.left();
-		qint32 descAtY = qMin(_historyPaddingTop - descH, qMax(0, (_scroll->height() - descH) / 2)) + st::msgMargin.top();
+		qint32 descAtY = std::min(_historyPaddingTop - descH, std::max(0, (_scroll->height() - descH) / 2)) + st::msgMargin.top();


Use clamp here?

Clamp cant be used here. There may be situations it will lead to undefined behavior, this constuction doesnt guarantee that second argument of std:max greater 0 and first argument of std::min greater 0, or one of these greater than other

From specs of std::clamp
"The behavior is undefined if the value of lo is greater than hi"

berkus · 2018-01-23T19:40:12Z

Telegram/SourceFiles/history/history_inner_widget.cpp

 		}
 		qint32 descAtX = (descMaxWidth - _botAbout->width) / 2 - st::msgPadding.left();
-		qint32 descAtY = qMin(newHistoryPaddingTop - descH, qMax(0, (_scroll->height() - descH) / 2)) + st::msgMargin.top();
+		qint32 descAtY = std::min(newHistoryPaddingTop - descH, std::max(0, (_scroll->height() - descH) / 2)) + st::msgMargin.top();


Clamp cant be used here. There may be situations it will lead to undefined behavior, this constuction doesnt guarantee that second argument of std:max greater 0 and first argument of std::min greater 0, or one of these greater than other

From specs of std::clamp
"The behavior is undefined if the value of lo is greater than hi"

berkus · 2018-01-23T19:41:12Z

Telegram/SourceFiles/history/history_message.cpp

@@ -463,7 +463,7 @@ void HistoryMessageReply::updateName() const {
 			w += st::msgServiceFont->spacew + _replyToVia->_maxWidth;
 		}

-		_maxReplyWidth = previewSkip + qMax(w, qMin(replyToText.maxWidth(), qint32(st::maxSignatureSize)));
+		_maxReplyWidth = previewSkip + std::max(w, std::min(replyToText.maxWidth(), qint32(st::maxSignatureSize)));


Clamp cant be used here. There may be situations it will lead to undefined behavior, this constuction doesnt guarantee that second argument of std:max greater 0 and first argument of std::min greater 0, or one of these greater than other

From specs of std::clamp
"The behavior is undefined if the value of lo is greater than hi"

berkus · 2018-01-23T19:41:25Z

Telegram/SourceFiles/history/history_message.cpp

 	if (_media && _media->currentWidth() < maxwidth) {
-		maxwidth = qMax(_media->currentWidth(), qMin(maxwidth, plainMaxWidth()));
+		maxwidth = std::max(_media->currentWidth(), std::min(maxwidth, plainMaxWidth()));


Clamp cant be used here. There may be situations it will lead to undefined behavior, this constuction doesnt guarantee that second argument of std:max greater 0 and first argument of std::min greater 0, or one of these greater than other

From specs of std::clamp
"The behavior is undefined if the value of lo is greater than hi"

berkus · 2018-01-23T19:47:01Z

Telegram/SourceFiles/media/media_audio_capture.cpp

 		double coef = 1. / fadeSamples, fadedFrom = d->fullSamples - skipSamples;
-		short *ptr = srcSamplesDataChannel, *zeroEnd = ptr + qMin(samplesCnt, qMax(0, skipSamples - d->fullSamples)), *end = ptr + fadedCnt;
+		short *ptr = srcSamplesDataChannel, *zeroEnd = ptr + std::min(samplesCnt, std::max(0, skipSamples - d->fullSamples)), *end = ptr + fadedCnt;


Clamp cant be used here. There may be situations it will lead to undefined behavior, this constuction doesnt guarantee that second argument of std:max greater 0 and first argument of std::min greater 0, or one of these greater than other

From specs of std::clamp
"The behavior is undefined if the value of lo is greater than hi"

berkus · 2018-01-23T20:00:31Z

Telegram/SourceFiles/mediaview.cpp

-		qint32 maxw = qMin(qMax(width() - 2 * skipw - st::mediaviewCaptionPadding.left() - st::mediaviewCaptionPadding.right() - 2 * st::mediaviewCaptionMargin.width(), int(st::msgMinWidth)), _caption.maxWidth());
-		qint32 maxh = qMin(_caption.countHeight(maxw), int(height() / 4 - st::mediaviewCaptionPadding.top() - st::mediaviewCaptionPadding.bottom() - 2 * st::mediaviewCaptionMargin.height()));
+		qint32 skipw = std::max(_dateNav.left() + _dateNav.width(), _headerNav.left() + _headerNav.width());
+		qint32 maxw = std::min(std::max(width() - 2 * skipw - st::mediaviewCaptionPadding.left() - st::mediaviewCaptionPadding.right() - 2 * st::mediaviewCaptionMargin.width(), int(st::msgMinWidth)), _caption.maxWidth());


Clamp cant be used here. There may be situations it will lead to undefined behavior, this constuction doesnt guarantee that second argument of std:max greater 0 and first argument of std::min greater 0, or one of these greater than other

From specs of std::clamp
"The behavior is undefined if the value of lo is greater than hi"

berkus · 2018-01-23T20:00:52Z

Telegram/SourceFiles/mediaview.cpp

-			auto skipw = qMax(_dateNav.left() + _dateNav.width(), _headerNav.left() + _headerNav.width());
-			auto maxw = qMin(qMax(width() - 2 * skipw - st::mediaviewCaptionPadding.left() - st::mediaviewCaptionPadding.right() - 2 * st::mediaviewCaptionMargin.width(), int(st::msgMinWidth)), _caption.maxWidth());
+			auto skipw = std::max(_dateNav.left() + _dateNav.width(), _headerNav.left() + _headerNav.width());
+			auto maxw = std::min(std::max(width() - 2 * skipw - st::mediaviewCaptionPadding.left() - st::mediaviewCaptionPadding.right() - 2 * st::mediaviewCaptionMargin.width(), int(st::msgMinWidth)), _caption.maxWidth());


Clamp cant be used here. There may be situations it will lead to undefined behavior, this constuction doesnt guarantee that second argument of std:max greater 0 and first argument of std::min greater 0, or one of these greater than other

From specs of std::clamp
"The behavior is undefined if the value of lo is greater than hi"

crackedmind · 2018-01-27T22:44:14Z

❌ Build kepka 1.0.65-kpremwsc failed (commit 52a41b1c90 by @pperehozhih)

…td::clamp (if it's possible)

crackedmind · 2018-01-27T23:39:54Z

✅ Build kepka 1.0.65-eyljxsgy completed (commit 2fed5995dd by @pperehozhih)

leha-bot · 2018-02-28T19:56:56Z

I consider to merge it w/o clamp changes. We will make it in next PR.

[Refactoring] change qt math function to cmath analog

e20f1c3

berkus added the pr:WIP DO NOT MERGE This PR is in progress, do not merge yet label Jan 19, 2018

berkus requested changes Jan 23, 2018

View reviewed changes

[Refactoring] set current standart to c++17 and change any logic to s…

b4e40ac

…td::clamp (if it's possible)

pperehozhih force-pushed the pp_change_math_function branch from 374d4a9 to b4e40ac Compare January 27, 2018 22:53

leha-bot added this to the Refactoring milestone Feb 13, 2018

berkus mentioned this pull request Feb 28, 2018

Replace weird qMin/qMax combinations with std::min, max and clamp #36

Open

3 tasks

berkus approved these changes Feb 28, 2018

View reviewed changes

berkus merged commit 1cb6dce into procxx:dev Feb 28, 2018

berkus removed the pr:WIP DO NOT MERGE This PR is in progress, do not merge yet label Feb 28, 2018

leha-bot added category:Enhancement This is a proposed improvement category:Infrastructure Related to build tools, automation category:Refactoring Related to code refactoring labels Feb 28, 2018

leha-bot mentioned this pull request Apr 10, 2018

PCH Task-list #134

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Refactoring] change qt math function to cmath analog #89

[Refactoring] change qt math function to cmath analog #89

pperehozhih commented Jan 18, 2018

crackedmind commented Jan 18, 2018

berkus left a comment

berkus Jan 23, 2018

ForNeVeR Jan 24, 2018

berkus Jan 23, 2018

pperehozhih Jan 27, 2018

berkus Jan 23, 2018

pperehozhih Jan 27, 2018

berkus Jan 23, 2018

pperehozhih Jan 27, 2018

berkus Jan 23, 2018

pperehozhih Jan 27, 2018

berkus Jan 23, 2018

pperehozhih Jan 27, 2018

berkus Jan 23, 2018

pperehozhih Jan 27, 2018

berkus Jan 23, 2018

pperehozhih Jan 27, 2018

crackedmind commented Jan 27, 2018

crackedmind commented Jan 27, 2018

leha-bot commented Feb 28, 2018

[Refactoring] change qt math function to cmath analog #89

[Refactoring] change qt math function to cmath analog #89

Conversation

pperehozhih commented Jan 18, 2018

crackedmind commented Jan 18, 2018

berkus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crackedmind commented Jan 27, 2018

crackedmind commented Jan 27, 2018

leha-bot commented Feb 28, 2018