[msvc] Fix warnings, mainly casting to smaller types #270
Conversation
|
@ntadej — I see the checks passed. Out of curiosity, do the GitHub Actions show a summary of warnings count before and after? And does that number go down with this commit? For iOS builds I checked a sample file that was changed, |
|
Those warnings are actually silenced at the moment as I found them too spammy (MSVC is also quite verbose). 😊 Also as we have Also to add I runtime tested this on Windows, macOS and iOS (Qt version) and I did not observe any visibly noticeable regressions (I tried really hard not to introduce typos that would affect functionality). |
|
@roblabs, @atierian, @birkskyum, is there anything else I should do from my side to proceed? |
…aplibre#270)"" This reverts commit 8958d3b.
| @@ -122,13 +122,13 @@ IntersectStatus CollisionIndex::intersectsTileEdges(const CollisionBox& box, | |||
| const float tileY2 = tileEdges[3]; | |||
|
|
|||
| // Check left border | |||
| int minSectionLength = std::min(tileX1 - x1, x2 - tileX1); | |||
| float minSectionLength = std::min(tileX1 - x1, x2 - tileX1); | |||
There was a problem hiding this comment.
Seems to break some render tests...
There was a problem hiding this comment.
Why this happens is obscure to me. If you look closer there is a cast to int when this variable is copied over to the result.
There was a problem hiding this comment.
Also it would be useful to actually to paste what tests this breaks to see the effect.
| (((p[0] / p[3] + 1) / 2) * size.width) + viewportPadding, | ||
| (((-p[1] / p[3] + 1) / 2) * size.height) + viewportPadding | ||
| static_cast<float>(((p[0] / p[3] + 1) / 2) * size.width) + viewportPadding, | ||
| static_cast<float>(((-p[1] / p[3] + 1) / 2) * size.height) + viewportPadding |
| @@ -145,7 +145,7 @@ void align(Shaping& shaping, | |||
| if (maxLineHeight != lineHeight) { | |||
| shiftY = -blockHeight * verticalAlign - Shaping::yOffset; | |||
| } else { | |||
| shiftY = (-verticalAlign * lineCount + 0.5) * lineHeight; | |||
| shiftY = (-verticalAlign * static_cast<float>(lineCount + 0.5)) * lineHeight; | |||
| return util::max(0.0, util::min(xCellCount - 1.0, std::floor(x * xScale))); | ||
| return util::max(size_t(0), util::min(xCellCount - 1, static_cast<size_t>(std::floor(x * xScale)))); | ||
| } | ||
|
|
||
| template <class T> | ||
| std::size_t GridIndex<T>::convertToYCellCoord(const float y) const { | ||
| return util::max(0.0, util::min(yCellCount - 1.0, std::floor(y * yScale))); | ||
| return util::max(size_t(0), util::min(yCellCount - 1, static_cast<size_t>(std::floor(y * yScale)))); |
If the checks by GitHub Actions checks are met, then it seems good to go. Thanks for supporting FOSS4G!
MSVC is quite eager with warnings (which is good in my opinion). This MR cleans most of the core codebase of MSVC warnings:
doubletofloat)Note that this is mostly silencing warnings (e.g. using
static_cast) but I also tried to fix some trivial cases and add some template specialisations. This should be purely technical, but let's see what also CI says.