Reduce deep nesting in gfx/pal_packing.cpp

src/gfx/pal_packing.cpp

@@ -307,42 +307,38 @@ static void decant(std::vector<AssignedSets> &assignments, std::vector<ColorSet>
 		// We do this by adding the first available color set, and then looking for palettes with
 		// common colors. (As an optimization, we know we can skip palettes already scanned.)
 		std::vector<bool> processed(from.nbColorSets(), false);
-		std::unordered_set<uint16_t> colors;
+		for (std::vector<bool>::iterator iter;
-		std::vector<size_t> members;
+		     (iter = std::find(RANGE(processed), true)) != processed.end();) {
-		while (true) {
-			auto iter = std::find(RANGE(processed), true);
-			if (iter == processed.end()) { // Processed everything!
-				break;
-			}
 			auto attrs = from.begin();
 			std::advance(attrs, iter - processed.begin());
 
+			std::unordered_set<uint16_t> colors(RANGE(colorSets[attrs->colorSetIndex]));
+			std::vector<size_t> members = {static_cast<size_t>(iter - processed.begin())};
+			*iter = true; // Mark the first color set as processed
+
 			// Build up the "component"...
-			colors.clear();
+			for (; ++iter != processed.end(); ++attrs) {
-			members.clear();
+				// If at least one color matches, add it
-			assume(members.empty()); // Compiler optimization hint
+				if (ColorSet const &colorSet = colorSets[attrs->colorSetIndex];
-			do {
+				    std::find_first_of(RANGE(colors), RANGE(colorSet)) != colors.end()) {
-				ColorSet const &colorSet = colorSets[attrs->colorSetIndex];
-				// If this is the first color set, or if at least one color matches, add it
-				if (members.empty()
-				    || std::find_first_of(RANGE(colors), RANGE(colorSet)) != colors.end()) {
 					colors.insert(RANGE(colorSet));
 					members.push_back(iter - processed.begin());
 					*iter = true; // Mark that color set as processed
 				}
-				++attrs;
+			}
-			} while (++iter != processed.end());
 
-			if (to.combinedVolume(RANGE(colors)) <= options.maxOpaqueColors()) {
+			if (to.combinedVolume(RANGE(colors)) > options.maxOpaqueColors()) {
-				// Iterate through the component's color sets, and transfer them
+				continue;
-				auto member = from.begin();
+			}
-				size_t curIndex = 0;
+
-				for (size_t index : members) {
+			// Iterate through the component's color sets, and transfer them
-					std::advance(member, index - curIndex);
+			auto member = from.begin();
-					curIndex = index;
+			size_t curIndex = 0;
-					to.assign(std::move(*member));
+			for (size_t index : members) {
-					from.remove(member); // Removing does not shift elements, so it's cheap
+				std::advance(member, index - curIndex);
-				}
+				curIndex = index;
+				to.assign(std::move(*member));
+				from.remove(member); // Removing does not shift elements, so it's cheap
 			}
 		}
 	});
@@ -383,12 +379,13 @@ std::tuple<std::vector<size_t>, size_t> overloadAndRemove(std::vector<ColorSet>
 		);
 	}
 
-	// Begin with all color sets queued up for insertion
-	std::queue<ColorSetAttrs> queue(std::deque<ColorSetAttrs>(RANGE(sortedColorSetIDs)));
 	// Begin with no pages
-	std::vector<AssignedSets> assignments{};
+	std::vector<AssignedSets> assignments;
 
-	for (; !queue.empty(); queue.pop()) {
+	// Begin with all color sets queued up for insertion
+	for (std::queue<ColorSetAttrs> queue(std::deque<ColorSetAttrs>(RANGE(sortedColorSetIDs)));
+	     !queue.empty();
+	     queue.pop()) {
 		ColorSetAttrs const &attrs = queue.front(); // Valid until the `queue.pop()`
 		options.verbosePrint(Options::VERB_TRACE, "Handling color set %zu\n", attrs.colorSetIndex);
 
@@ -428,91 +425,78 @@ std::tuple<std::vector<size_t>, size_t> overloadAndRemove(std::vector<ColorSet>
 			    bestPalIndex
 			);
 			assignments.emplace_back(colorSets, std::move(attrs));
-		} else {
+			continue;
+		}
+
+		options.verbosePrint(
+		    Options::VERB_TRACE,
+		    "Assigning color set %zu to palette %zu\n",
+		    attrs.colorSetIndex,
+		    bestPalIndex
+		);
+		AssignedSets &bestPal = assignments[bestPalIndex];
+		// Add the color to that palette
+		bestPal.assign(std::move(attrs));
+
+		auto compareEfficiency = [&](ColorSetAttrs const &attrs1, ColorSetAttrs const &attrs2) {
+			ColorSet const &colorSet1 = colorSets[attrs1.colorSetIndex];
+			ColorSet const &colorSet2 = colorSets[attrs2.colorSetIndex];
+			size_t size1 = colorSet1.size();
+			size_t size2 = colorSet2.size();
+			uint32_t relSize1 = bestPal.relSizeOf(colorSet1);
+			uint32_t relSize2 = bestPal.relSizeOf(colorSet2);
+
 			options.verbosePrint(
 			    Options::VERB_TRACE,
-			    "Assigning color set %zu to palette %zu\n",
+			    "  Color sets %zu <=> %zu: Efficiency: %zu / %" PRIu32 " <=> %zu / "
-			    attrs.colorSetIndex,
+			    "%" PRIu32 "\n",
-			    bestPalIndex
+			    attrs1.colorSetIndex,
+			    attrs2.colorSetIndex,
+			    size1,
+			    relSize1,
+			    size2,
+			    relSize2
 			);
-			AssignedSets &bestPal = assignments[bestPalIndex];
-			// Add the color to that palette
-			bestPal.assign(std::move(attrs));
 
-			// If this overloads the palette, get it back to normal (if possible)
+			// This comparison is algebraically equivalent to
-			while (bestPal.volume() > options.maxOpaqueColors()) {
+			// `size1 / relSize1 <=> size2 / relSize2`,
-				options.verbosePrint(
+			// but without potential precision loss from floating-point division.
-				    Options::VERB_TRACE,
+			size_t efficiency1 = size1 * relSize2;
-				    "Palette %zu is overloaded! (%zu > %" PRIu8 ")\n",
+			size_t efficiency2 = size2 * relSize1;
-				    bestPalIndex,
+			return (efficiency1 > efficiency2) - (efficiency1 < efficiency2);
-				    bestPal.volume(),
+		};
-				    options.maxOpaqueColors()
-				);
 
-				// Look for a color set minimizing "efficiency" (size / rel_size)
+		// If this overloads the palette, get it back to normal (if possible)
-				auto [minEfficiencyIter, maxEfficiencyIter] = std::minmax_element(
+		while (bestPal.volume() > options.maxOpaqueColors()) {
-				    RANGE(bestPal),
+			options.verbosePrint(
-				    [&bestPal, &colorSets](ColorSetAttrs const &lhs, ColorSetAttrs const &rhs) {
+			    Options::VERB_TRACE,
-					    ColorSet const &lhsColorSet = colorSets[lhs.colorSetIndex];
+			    "Palette %zu is overloaded! (%zu > %" PRIu8 ")\n",
-					    ColorSet const &rhsColorSet = colorSets[rhs.colorSetIndex];
+			    bestPalIndex,
-					    size_t lhsSize = lhsColorSet.size();
+			    bestPal.volume(),
-					    size_t rhsSize = rhsColorSet.size();
+			    options.maxOpaqueColors()
-					    uint32_t lhsRelSize = bestPal.relSizeOf(lhsColorSet);
+			);
-					    uint32_t rhsRelSize = bestPal.relSizeOf(rhsColorSet);
 
-					    options.verbosePrint(
+			// Look for a color set minimizing "efficiency" (size / relSize)
-					        Options::VERB_TRACE,
+			auto [minEfficiencyIter, maxEfficiencyIter] = std::minmax_element(
-					        "  Color sets %zu <=> %zu: Efficiency: %zu / %" PRIu32 " <=> %zu / "
+			    RANGE(bestPal),
-					        "%" PRIu32 "\n",
+			    [&compareEfficiency](ColorSetAttrs const &lhs, ColorSetAttrs const &rhs) {
-					        lhs.colorSetIndex,
+				    return compareEfficiency(lhs, rhs) < 0;
-					        rhs.colorSetIndex,
+			    }
-					        lhsSize,
+			);
-					        lhsRelSize,
-					        rhsSize,
-					        rhsRelSize
-					    );
-					    // 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
+			// All efficiencies are identical iff min equals max
-				ColorSet const &minColorSet = colorSets[minEfficiencyIter->colorSetIndex];
+			if (compareEfficiency(*minEfficiencyIter, *maxEfficiencyIter) == 0) {
-				ColorSet const &maxColorSet = colorSets[maxEfficiencyIter->colorSetIndex];
+				options.verbosePrint(Options::VERB_TRACE, "  All efficiencies are identical\n");
-				size_t minSize = minColorSet.size();
+				break;
-				size_t maxSize = maxColorSet.size();
-				uint32_t minRelSize = bestPal.relSizeOf(minColorSet);
-				uint32_t maxRelSize = bestPal.relSizeOf(maxColorSet);
-				options.verbosePrint(
-				    Options::VERB_TRACE,
-				    "  Color sets %zu <= %zu: Efficiency: %zu / %" PRIu32 " <= %zu / %" PRIu32 "\n",
-				    minEfficiencyIter->colorSetIndex,
-				    maxEfficiencyIter->colorSetIndex,
-				    minSize,
-				    minRelSize,
-				    maxSize,
-				    maxRelSize
-				);
-				// This comparison is algebraically equivalent to
-				// `maxSize / maxRelSize == minSize / minRelSize`,
-				// but without potential precision loss from floating-point division.
-				if (maxSize * minRelSize == minSize * maxRelSize) {
-					options.verbosePrint(Options::VERB_TRACE, "  All efficiencies are identical\n");
-					break;
-				}
-
-				// Remove the color set with minimal efficiency
-				options.verbosePrint(
-				    Options::VERB_TRACE,
-				    "  Removing color set %zu\n",
-				    minEfficiencyIter->colorSetIndex
-				);
-				queue.emplace(std::move(*minEfficiencyIter));
-				queue.back().banFrom(bestPalIndex); // Ban it from this palette
-				bestPal.remove(minEfficiencyIter);
 			}
+
+			// Remove the color set with minimal efficiency
+			options.verbosePrint(
+			    Options::VERB_TRACE, "  Removing color set %zu\n", minEfficiencyIter->colorSetIndex
+			);
+			queue.emplace(std::move(*minEfficiencyIter));
+			queue.back().banFrom(bestPalIndex); // Ban it from this palette
+			bestPal.remove(minEfficiencyIter);
 		}
 	}