fix: auto-detect zone coordinate format instead of trusting Units field
The zone loader now ignores the Units DB field and detects the coordinate format by checking for decimal points: decimal values are percentages, integer-only values are legacy pixels. This fixes motion detection being broken when zones had Units=Pixels but percentage coordinates (or vice versa), which resulted in a ~99x99 pixel zone on a 2560x1440 monitor. The PHP zone view now always forces Units=Percent when saving, since it always works in percentage space. convertPixelPointsToPercent() now returns bool to indicate whether conversion occurred. Tests added for: truncation bug via atoi, correct percentage-to-pixel conversion, auto-detect heuristic, and resolution independence. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>pull/4693/head
parent
2a899efbcd
commit
7d78b722d0
|
|
@ -984,54 +984,20 @@ std::vector<Zone> Zone::Load(const std::shared_ptr<Monitor> &monitor) {
|
|||
/* HTML colour code is actually BGR in memory, we want RGB */
|
||||
AlarmRGB = rgb_convert(AlarmRGB, ZM_SUBPIX_ORDER_BGR);
|
||||
|
||||
// Auto-detect coordinate format: decimal points mean percentages,
|
||||
// integer-only means legacy pixel values. Units field is not trusted.
|
||||
Debug(5, "Parsing polygon %s (Units=%s)", Coords, Units);
|
||||
Polygon polygon;
|
||||
if (!strcmp(Units, "Pixels")) {
|
||||
// Legacy pixel-based coordinates: parse as integer pixel values
|
||||
if (!ParsePolygonString(Coords, polygon)) {
|
||||
if (strchr(Coords, '.')) {
|
||||
// Decimal values present — treat as percentages regardless of Units field
|
||||
if (!ParsePercentagePolygon(Coords, monitor->Width(), monitor->Height(), polygon)) {
|
||||
Error("Unable to parse polygon string '%s' for zone %d/%s for monitor %s, ignoring",
|
||||
Coords, Id, Name, monitor->Name());
|
||||
continue;
|
||||
}
|
||||
} else {
|
||||
// Percentage-based coordinates (default): convert to pixels using monitor dimensions.
|
||||
// However, if any coordinate value exceeds 100, these are actually pixel values
|
||||
// stored with incorrect Units — fall back to pixel parsing with a warning.
|
||||
bool has_pixel_values = false;
|
||||
{
|
||||
const char *s = Coords;
|
||||
while (*s != '\0') {
|
||||
double val = strtod(s, nullptr);
|
||||
if (val > 100.0) {
|
||||
has_pixel_values = true;
|
||||
break;
|
||||
}
|
||||
// Skip to next number: find comma then space (x,y pairs separated by spaces)
|
||||
const char *comma = strchr(s, ',');
|
||||
if (!comma) break;
|
||||
val = strtod(comma + 1, nullptr);
|
||||
if (val > 100.0) {
|
||||
has_pixel_values = true;
|
||||
break;
|
||||
}
|
||||
const char *space = strchr(comma + 1, ' ');
|
||||
if (space) {
|
||||
s = space + 1;
|
||||
} else {
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (has_pixel_values) {
|
||||
Debug(1, "Zone %d/%s has Units=Percent but Coords contain pixel values (>100), "
|
||||
"parsing as pixels instead", Id, Name);
|
||||
if (!ParsePolygonString(Coords, polygon)) {
|
||||
Error("Unable to parse polygon string '%s' for zone %d/%s for monitor %s, ignoring",
|
||||
Coords, Id, Name, monitor->Name());
|
||||
continue;
|
||||
}
|
||||
} else if (!ParsePercentagePolygon(Coords, monitor->Width(), monitor->Height(), polygon)) {
|
||||
// Integer-only coordinates — treat as pixel values
|
||||
if (!ParsePolygonString(Coords, polygon)) {
|
||||
Error("Unable to parse polygon string '%s' for zone %d/%s for monitor %s, ignoring",
|
||||
Coords, Id, Name, monitor->Name());
|
||||
continue;
|
||||
|
|
|
|||
|
|
@ -220,3 +220,106 @@ TEST_CASE("Zone: pixel values through ParsePercentagePolygon produce wrong resul
|
|||
REQUIRE(verts[1] == Vector2(static_cast<int>(width), 0));
|
||||
REQUIRE(verts[2] == Vector2(static_cast<int>(width), static_cast<int>(height)));
|
||||
}
|
||||
|
||||
// --- Auto-detect format tests ---
|
||||
// The zone loader uses strchr(Coords, '.') to decide format:
|
||||
// decimal point present -> ParsePercentagePolygon
|
||||
// no decimal point -> ParsePolygonString (legacy pixels)
|
||||
// These tests verify both parsers handle the inputs they'll receive
|
||||
// under auto-detection, and document the broken case that auto-detection prevents.
|
||||
|
||||
TEST_CASE("Zone: ParsePolygonString truncates decimal coords via atoi", "[Zone]") {
|
||||
// This is the bug that broke motion detection: percentage coords like
|
||||
// "0.00,0.00 99.96,0.00 99.96,99.93 0.00,99.93" parsed by
|
||||
// ParsePolygonString (which uses atoi) get truncated to a 99x99 pixel zone
|
||||
Polygon polygon;
|
||||
bool ok = Zone::ParsePolygonString("0.00,0.00 99.96,0.00 99.96,99.93 0.00,99.93", polygon);
|
||||
REQUIRE(ok);
|
||||
|
||||
auto const &verts = polygon.GetVertices();
|
||||
REQUIRE(verts.size() == 4);
|
||||
// atoi("99.96") = 99, atoi("99.93") = 99
|
||||
// On a 2560x1440 monitor this would be a 99x99 pixel zone — essentially no coverage
|
||||
REQUIRE(verts[1] == Vector2(99, 0));
|
||||
REQUIRE(verts[2] == Vector2(99, 99));
|
||||
}
|
||||
|
||||
TEST_CASE("Zone: decimal coords through ParsePercentagePolygon give correct pixels", "[Zone]") {
|
||||
// Same coords as above, but correctly routed to ParsePercentagePolygon
|
||||
// by the auto-detect logic (decimal point present)
|
||||
Polygon polygon;
|
||||
unsigned int width = 2560;
|
||||
unsigned int height = 1440;
|
||||
|
||||
bool ok = Zone::ParsePercentagePolygon(
|
||||
"0.00,0.00 99.96,0.00 99.96,99.93 0.00,99.93", width, height, polygon);
|
||||
REQUIRE(ok);
|
||||
|
||||
auto const &verts = polygon.GetVertices();
|
||||
REQUIRE(verts.size() == 4);
|
||||
// 99.96% of 2560 = 2558.976 -> 2559
|
||||
REQUIRE(verts[1].x_ == 2559);
|
||||
REQUIRE(verts[1].y_ == 0);
|
||||
// 99.93% of 1440 = 1438.992 -> 1439
|
||||
REQUIRE(verts[2].x_ == 2559);
|
||||
REQUIRE(verts[2].y_ == 1439);
|
||||
}
|
||||
|
||||
TEST_CASE("Zone: integer pixel coords stay as pixels", "[Zone]") {
|
||||
// Legacy integer-only coords should be parsed as raw pixel values
|
||||
// Auto-detect: no decimal point -> ParsePolygonString
|
||||
Polygon polygon;
|
||||
bool ok = Zone::ParsePolygonString("0,0 2559,0 2559,1439 0,1439", polygon);
|
||||
REQUIRE(ok);
|
||||
|
||||
auto const &verts = polygon.GetVertices();
|
||||
REQUIRE(verts.size() == 4);
|
||||
REQUIRE(verts[0] == Vector2(0, 0));
|
||||
REQUIRE(verts[1] == Vector2(2559, 0));
|
||||
REQUIRE(verts[2] == Vector2(2559, 1439));
|
||||
REQUIRE(verts[3] == Vector2(0, 1439));
|
||||
}
|
||||
|
||||
TEST_CASE("Zone: auto-detect heuristic — strchr for decimal point", "[Zone]") {
|
||||
// Verify the heuristic used by the zone loader:
|
||||
// strchr(coords, '.') distinguishes percentage from pixel coords
|
||||
|
||||
// Percentage coords always have decimal points from round(..., 2)
|
||||
const char *pct_coords = "0.00,0.00 99.96,0.00 99.96,99.93 0.00,99.93";
|
||||
REQUIRE(strchr(pct_coords, '.') != nullptr);
|
||||
|
||||
// Legacy pixel coords are always integers
|
||||
const char *px_coords = "0,0 2559,0 2559,1439 0,1439";
|
||||
REQUIRE(strchr(px_coords, '.') == nullptr);
|
||||
|
||||
// Edge case: small pixel zone that looks like it could be percentages
|
||||
// but has no decimal points — correctly detected as pixels
|
||||
const char *small_px = "0,0 50,0 50,50 0,50";
|
||||
REQUIRE(strchr(small_px, '.') == nullptr);
|
||||
}
|
||||
|
||||
TEST_CASE("Zone: percentage coords at various resolutions", "[Zone]") {
|
||||
Polygon polygon;
|
||||
|
||||
// The same percentage zone should produce proportional pixel coords
|
||||
// regardless of monitor resolution
|
||||
const char *coords = "10.00,20.00 90.00,20.00 90.00,80.00 10.00,80.00";
|
||||
|
||||
SECTION("640x480") {
|
||||
bool ok = Zone::ParsePercentagePolygon(coords, 640, 480, polygon);
|
||||
REQUIRE(ok);
|
||||
auto const &v = polygon.GetVertices();
|
||||
REQUIRE(v[0] == Vector2(64, 96)); // 10% of 640, 20% of 480
|
||||
REQUIRE(v[1] == Vector2(576, 96)); // 90% of 640
|
||||
REQUIRE(v[2] == Vector2(576, 384)); // 80% of 480
|
||||
}
|
||||
|
||||
SECTION("3840x2160 (4K)") {
|
||||
bool ok = Zone::ParsePercentagePolygon(coords, 3840, 2160, polygon);
|
||||
REQUIRE(ok);
|
||||
auto const &v = polygon.GetVertices();
|
||||
REQUIRE(v[0] == Vector2(384, 432)); // 10% of 3840, 20% of 2160
|
||||
REQUIRE(v[1] == Vector2(3456, 432)); // 90% of 3840
|
||||
REQUIRE(v[2] == Vector2(3456, 1728)); // 80% of 2160
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1467,7 +1467,7 @@ function limitPoints(&$points, $min_x, $min_y, $max_x, $max_y) {
|
|||
} // end function limitPoints( $points, $min_x, $min_y, $max_x, $max_y )
|
||||
|
||||
function convertPixelPointsToPercent(&$points, $width, $height) {
|
||||
if (!$width || !$height) return;
|
||||
if (!$width || !$height) return false;
|
||||
$isPixel = false;
|
||||
foreach ($points as $point) {
|
||||
if ($point['x'] > 100 || $point['y'] > 100) {
|
||||
|
|
@ -1482,6 +1482,7 @@ function convertPixelPointsToPercent(&$points, $width, $height) {
|
|||
}
|
||||
unset($point);
|
||||
}
|
||||
return $isPixel;
|
||||
}
|
||||
|
||||
function scalePoints(&$points, $scale) {
|
||||
|
|
|
|||
|
|
@ -96,6 +96,8 @@ if ( !isset($zone) ) {
|
|||
|
||||
# Auto-detect pixel coordinates and convert to percentages
|
||||
convertPixelPointsToPercent($zone['Points'], $monitor->ViewWidth(), $monitor->ViewHeight());
|
||||
# Always store as percentages
|
||||
$zone['Units'] = 'Percent';
|
||||
|
||||
# Ensure Zone fits within the limits of the Monitor
|
||||
limitPoints($zone['Points'], $minX, $minY, $maxX, $maxY);
|
||||
|
|
|
|||
Loading…
Reference in New Issue