Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions src/devices/video/poly.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,9 @@

#pragma once

#include <climits>
#include <atomic>
#include <climits>
#include <limits>


#define KEEP_POLY_STATISTICS 0
Expand Down Expand Up @@ -365,12 +366,19 @@ class poly_manager
using unit_array = poly_array<work_unit, 0>;

// round in a cross-platform consistent manner
inline int32_t round_coordinate(BaseType value)
static int32_t round_coordinate(BaseType value)
{
int32_t result = int32_t(std::floor(value));
if (value > 0 && result < 0)
return INT_MAX - 1;
return result + (value - BaseType(result) > BaseType(0.5));
// saturate (avoid overflow/underflow)
if (value >= BaseType(std::numeric_limits<int32_t>::max()))
return std::numeric_limits<int32_t>::max();
Comment on lines +372 to +373
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You know that if it doesn’t hit this test, int32_t(ipart) will never give std::numeric_limits<int32_t>::max(), because std::floor always produces a lesser or equal value, and converting to integer rounds towards zero.

In cases where BaseType can't represent std::numeric_limits<int32_t>::max() precisely, BaseType(std::numeric_limits<int32_t>::max()) gets rounded away from zero so the test should still be safe.

const BaseType ipart = std::floor(value);
if (ipart < BaseType(std::numeric_limits<int32_t>::min()))
return std::numeric_limits<int32_t>::min();
Comment on lines +374 to +376
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be < not <= or it will fail to round when BaseType can precisely represent a 32-bit integer and ipart == BaseType(std::numeric_limits<int32_t>::min()) and fpart > BaseType(0.5).


// round
// TODO: this rounds the midpoint towards negative infinity - should it use more standard behaviour?
const BaseType fpart = value - ipart;
return int32_t(ipart) + ((fpart > BaseType(0.5)) ? 1 : 0);
}

// internal helpers
Expand Down