Skip to content

Commit

Permalink
Avoid precision loss from floating-point division in calculating effi…
Browse files Browse the repository at this point in the history
…ciency

This was breaking test results on 32-bit MinGW
  • Loading branch information
Rangi42 authored and ISSOtm committed Nov 27, 2024
1 parent 81c3521 commit c33acb9
Showing 1 changed file with 22 additions and 10 deletions.
32 changes: 22 additions & 10 deletions src/gfx/pal_packing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -423,24 +423,36 @@ std::tuple<DefaultInitVec<size_t>, size_t>
);

// Look for a proto-pal minimizing "efficiency" (size / rel_size)
auto efficiency = [&bestPal](ProtoPalette const &pal) {
return pal.size() / bestPal.relSizeOf(pal);
};
auto [minEfficiencyIter, maxEfficiencyIter] = std::minmax_element(
RANGE(bestPal),
[&efficiency,
&protoPalettes](ProtoPalAttrs const &lhs, ProtoPalAttrs const &rhs) {
return efficiency(protoPalettes[lhs.protoPalIndex])
< efficiency(protoPalettes[rhs.protoPalIndex]);
[&bestPal, &protoPalettes](ProtoPalAttrs const &lhs, ProtoPalAttrs const &rhs) {
ProtoPalette const &lhsProtoPal = protoPalettes[lhs.protoPalIndex];
ProtoPalette const &rhsProtoPal = protoPalettes[rhs.protoPalIndex];
size_t lhsSize = lhsProtoPal.size();
size_t rhsSize = rhsProtoPal.size();
double lhsRelSize = bestPal.relSizeOf(lhsProtoPal);
double rhsRelSize = bestPal.relSizeOf(rhsProtoPal);

// This comparison is algebraically equivalent to
// `lhsSize / lhsRelSize < rhsSize / rhsRelSize`,
// but without potential precision loss from floating-point division.
return lhsSize * rhsRelSize < rhsSize * lhsRelSize;
}
);

// All efficiencies are identical iff min equals max
// TODO: maybe not ideal to re-compute these two?
ProtoPalette const &minProtoPal = protoPalettes[minEfficiencyIter->protoPalIndex];
ProtoPalette const &maxProtoPal = protoPalettes[maxEfficiencyIter->protoPalIndex];
size_t minSize = minProtoPal.size();
size_t maxSize = maxProtoPal.size();
double minRelSize = bestPal.relSizeOf(minProtoPal);
double maxRelSize = bestPal.relSizeOf(maxProtoPal);
// This comparison is algebraically equivalent to
// `maxSize / maxRelSize - minSize / minRelSize < .001`,
// but without potential precision loss from floating-point division.
// TODO: yikes for float comparison! I *think* this threshold is OK?
if (efficiency(protoPalettes[maxEfficiencyIter->protoPalIndex])
- efficiency(protoPalettes[minEfficiencyIter->protoPalIndex])
< .001) {
if (maxSize * minRelSize - minSize * maxRelSize < minRelSize * maxRelSize * .001) {
break;
}

Expand Down

0 comments on commit c33acb9

Please sign in to comment.