SCI32: Clean up GfxRemap32

* Rewrap comments to 80 columns
* Clarify comments where possible

3 files changed, 125 insertions, 166 deletions

diff --git a/engines/sci/engine/kgraphics32.cpp b/engines/sci/engine/kgraphics32.cpp
index 1734221ec2..02b2cea9a9 100644
--- a/engines/sci/engine/kgraphics32.cpp
+++ b/engines/sci/engine/kgraphics32.cpp
@@ -1057,9 +1057,9 @@ reg_t kRemapColorsByRange(EngineState *s, int argc, reg_t *argv) {
 	const int16 from = argv[1].toSint16();
 	const int16 to = argv[2].toSint16();
 	const int16 base = argv[3].toSint16();
-	// NOTE: There is an optional last parameter after `base`
-	// which was only used by the priority map debugger, which
-	// does not exist in release versions of SSCI
+	// There is an optional last parameter after `base` which was only used by
+	// the priority map debugger which does not exist in release versions of
+	// SSCI
 	g_sci->_gfxRemap32->remapByRange(color, from, to, base);
 	return s->r_acc;
 }
@@ -1067,9 +1067,9 @@ reg_t kRemapColorsByRange(EngineState *s, int argc, reg_t *argv) {
 reg_t kRemapColorsByPercent(EngineState *s, int argc, reg_t *argv) {
 	const uint8 color = argv[0].toUint16();
 	const int16 percent = argv[1].toSint16();
-	// NOTE: There is an optional last parameter after `percent`
-	// which was only used by the priority map debugger, which
-	// does not exist in release versions of SSCI
+	// There is an optional last parameter after `percent` which was only used
+	// by the priority map debugger, which does not exist in release versions of
+	// SSCI
 	g_sci->_gfxRemap32->remapByPercent(color, percent);
 	return s->r_acc;
 }
@@ -1077,9 +1077,9 @@ reg_t kRemapColorsByPercent(EngineState *s, int argc, reg_t *argv) {
 reg_t kRemapColorsToGray(EngineState *s, int argc, reg_t *argv) {
 	const uint8 color = argv[0].toUint16();
 	const int16 gray = argv[1].toSint16();
-	// NOTE: There is an optional last parameter after `gray`
-	// which was only used by the priority map debugger, which
-	// does not exist in release versions of SSCI
+	// There is an optional last parameter after `gray` which was only used by
+	// the priority map debugger, which does not exist in release versions of
+	// SSCI
 	g_sci->_gfxRemap32->remapToGray(color, gray);
 	return s->r_acc;
 }
@@ -1088,9 +1088,9 @@ reg_t kRemapColorsToPercentGray(EngineState *s, int argc, reg_t *argv) {
 	const uint8 color = argv[0].toUint16();
 	const int16 gray = argv[1].toSint16();
 	const int16 percent = argv[2].toSint16();
-	// NOTE: There is an optional last parameter after `percent`
-	// which was only used by the priority map debugger, which
-	// does not exist in release versions of SSCI
+	// There is an optional last parameter after `percent` which was only used
+	// by the priority map debugger, which does not exist in release versions of
+	// SSCI
 	g_sci->_gfxRemap32->remapToPercentGray(color, gray, percent);
 	return s->r_acc;
 }
diff --git a/engines/sci/graphics/remap32.cpp b/engines/sci/graphics/remap32.cpp
index 9b5ffcd2f7..6fe0a53f71 100644
--- a/engines/sci/graphics/remap32.cpp
+++ b/engines/sci/graphics/remap32.cpp
@@ -100,9 +100,9 @@ bool SingleRemap::updateBrightness() {
 		}
 
 		if (_percent != _lastPercent || _originalColorsChanged[i]) {
-			// NOTE: SSCI checked if percent was over 100 and only
-			// then clipped values, but we always unconditionally
-			// ensure the result is in the correct range
+			// SSCI checked if percent was over 100 and only then clipped
+			// values, but we always unconditionally ensure the result is in the
+			// correct range for simplicity's sake
 			color.r = MIN(255, (uint16)color.r * _percent / 100);
 			color.g = MIN(255, (uint16)color.g * _percent / 100);
 			color.b = MIN(255, (uint16)color.b * _percent / 100);
@@ -188,8 +188,7 @@ bool SingleRemap::apply() {
 	const GfxRemap32 *const gfxRemap32 = g_sci->_gfxRemap32;
 	const uint8 remapStartColor = gfxRemap32->getStartColor();
 
-	// Blocked colors are not allowed to be used as target
-	// colors for the remap
+	// Blocked colors are not allowed to be used as target colors for the remap
 	bool blockedColors[236];
 	Common::fill(blockedColors, blockedColors + remapStartColor, false);
 
@@ -207,9 +206,8 @@ bool SingleRemap::apply() {
 		}
 	}
 
-	// NOTE: SSCI did a loop over colors here to create a
-	// new array of updated, unblocked colors, but then
-	// never used it
+	// SSCI did a loop over colors here to create a new array of updated,
+	// unblocked colors, but then never used it
 
 	bool updated = false;
 	for (uint i = 1; i < remapStartColor; ++i) {
@@ -280,8 +278,8 @@ int16 SingleRemap::matchColor(const Color &color, const int minimumDistance, int
 		bestIndex = i;
 	}
 
-	// This value is only valid if the last index to
-	// perform a distance calculation was the best index
+	// This value is only valid if the last index to perform a distance
+	// calculation was the best index
 	outDistance = distance;
 	return bestIndex;
 }
@@ -295,10 +293,9 @@ GfxRemap32::GfxRemap32() :
 	_blockedRangeCount(0),
 	_remapStartColor(236),
 	_numActiveRemaps(0) {
-	// The `_remapStartColor` seems to always be 236 in SSCI,
-	// but if it is ever changed then the various C-style
-	// member arrays hard-coded to 236 need to be changed to
-	// match the highest possible value of `_remapStartColor`
+	// The `_remapStartColor` seems to always be 236 in SSCI, but if it is ever
+	// changed then the various C-style member arrays hard-coded to 236 need to
+	// be changed to match the highest possible value of `_remapStartColor`
 	assert(_remapStartColor == 236);
 
 	if (g_sci->_features->hasMidPaletteCode()) {
@@ -316,9 +313,8 @@ void GfxRemap32::remapOff(const uint8 color) {
 		return;
 	}
 
-	// NOTE: SSCI simply ignored invalid input values, but
-	// we at least give a warning so games can be investigated
-	// for script bugs
+	// SSCI simply ignored invalid input values, but we at least give a warning
+	// so games can be investigated for script bugs
 	if (color < _remapStartColor || color > _remapEndColor) {
 		warning("GfxRemap32::remapOff: %d out of remap range", color);
 		return;
@@ -341,9 +337,8 @@ void GfxRemap32::remapAllOff() {
 }
 
 void GfxRemap32::remapByRange(const uint8 color, const int16 from, const int16 to, const int16 delta) {
-	// NOTE: SSCI simply ignored invalid input values, but
-	// we at least give a warning so games can be investigated
-	// for script bugs
+	// SSCI simply ignored invalid input values, but we at least give a warning
+	// so games can be investigated for script bugs
 	if (color < _remapStartColor || color > _remapEndColor) {
 		warning("GfxRemap32::remapByRange: %d out of remap range", color);
 		return;
@@ -375,9 +370,8 @@ void GfxRemap32::remapByRange(const uint8 color, const int16 from, const int16 t
 }
 
 void GfxRemap32::remapByPercent(const uint8 color, const int16 percent) {
-	// NOTE: SSCI simply ignored invalid input values, but
-	// we at least give a warning so games can be investigated
-	// for script bugs
+	// SSCI simply ignored invalid input values, but we at least give a warning
+	// so games can be investigated for script bugs
 	if (color < _remapStartColor || color > _remapEndColor) {
 		warning("GfxRemap32::remapByPercent: %d out of remap range", color);
 		return;
@@ -397,9 +391,8 @@ void GfxRemap32::remapByPercent(const uint8 color, const int16 percent) {
 }
 
 void GfxRemap32::remapToGray(const uint8 color, const int8 gray) {
-	// NOTE: SSCI simply ignored invalid input values, but
-	// we at least give a warning so games can be investigated
-	// for script bugs
+	// SSCI simply ignored invalid input values, but we at least give a warning
+	// so games can be investigated for script bugs
 	if (color < _remapStartColor || color > _remapEndColor) {
 		warning("GfxRemap32::remapToGray: %d out of remap range", color);
 		return;
@@ -423,9 +416,8 @@ void GfxRemap32::remapToGray(const uint8 color, const int8 gray) {
 }
 
 void GfxRemap32::remapToPercentGray(const uint8 color, const int16 gray, const int16 percent) {
-	// NOTE: SSCI simply ignored invalid input values, but
-	// we at least give a warning so games can be investigated
-	// for script bugs
+	// SSCI simply ignored invalid input values, but we at least give a warning
+	// so games can be investigated for script bugs
 	if (color < _remapStartColor || color > _remapEndColor) {
 		warning("GfxRemap32::remapToPercentGray: %d out of remap range", color);
 		return;
diff --git a/engines/sci/graphics/remap32.h b/engines/sci/graphics/remap32.h
index 407e879e7c..9fc7ef637d 100644
--- a/engines/sci/graphics/remap32.h
+++ b/engines/sci/graphics/remap32.h
@@ -55,8 +55,7 @@ public:
 	RemapType _type;
 
 	/**
-	 * The first color that should be shifted by a range
-	 * remap.
+	 * The first color that should be shifted by a range remap.
 	 */
 	uint8 _from;
 
@@ -66,158 +65,140 @@ public:
 	uint8 _to;
 
 	/**
-	 * The direction and amount that the colors should be
-	 * shifted in a range remap.
+	 * The direction and amount that the colors should be shifted in a range
+	 * remap.
 	 */
 	int16 _delta;
 
 	/**
-	 * The difference in brightness that should be
-	 * applied by a brightness (percent) remap.
+	 * The difference in brightness that should be applied by a brightness
+	 * (percent) remap.
 	 *
-	 * This value may be be greater than 100, in
-	 * which case the color will be oversaturated.
+	 * This value may be be greater than 100, in which case the color will be
+	 * oversaturated.
 	 */
 	int16 _percent;
 
 	/**
-	 * The amount of desaturation that should be
-	 * applied by a saturation (gray) remap, where
-	 * 0 is full saturation and 100 is full
-	 * desaturation.
+	 * The amount of desaturation that should be applied by a saturation (gray)
+	 * remap, where 0 is full saturation and 100 is full desaturation.
 	 */
 	uint8 _gray;
 
 	/**
-	 * The final array used by CelObj renderers to composite
-	 * remapped pixels to the screen buffer.
+	 * The final array used by CelObj renderers to composite remapped pixels to
+	 * the screen buffer.
 	 *
 	 * Here is how it works:
 	 *
-	 * The source bitmap being rendered will have pixels
-	 * within the remap range (236-245 or 236-254), and the
-	 * target buffer will have colors in the non-remapped
-	 * range (0-235).
+	 * The source bitmap being rendered will have pixels within the remap range
+	 * (236-245 or 236-254), and the target buffer will have colors in the
+	 * non-remapped range (0-235).
 	 *
-	 * To arrive at the correct color, first the source
-	 * pixel is used to look up the correct SingleRemap for
-	 * that pixel. Then, the final composited color is
-	 * looked up in this array using the target's pixel
-	 * color. In other words,
+	 * To arrive at the correct color, first the source pixel is used to look up
+	 * the correct SingleRemap for that pixel. Then, the final composited color
+	 * is looked up in this array using the target's pixel color. In other
+	 * words,
 	 * `target = _remaps[remapEndColor - source].remapColors[target]`.
 	 */
 	uint8 _remapColors[236];
 
 	/**
-	 * Resets this SingleRemap's color information to
-	 * default values.
+	 * Resets this SingleRemap's color information to default values.
 	 */
 	void reset();
 
 	/**
-	 * Recalculates and reapplies remap colors to the
-	 * `_remapColors` array.
+	 * Recalculates and reapplies remap colors to the `_remapColors` array.
 	 */
 	bool update();
 
 private:
 	/**
-	 * The previous brightness value. Used to
-	 * determine whether or not targetColors needs
-	 * to be updated.
+	 * The previous brightness value. Used to determine whether or not
+	 * `_idealColors` needs to be updated.
 	 */
 	int16 _lastPercent;
 
 	/**
-	 * The previous saturation value. Used to
-	 * determine whether or not targetColors needs
-	 * to be updated.
+	 * The previous saturation value. Used to determine whether or not
+	 * `_idealColors` needs to be updated.
 	 */
 	uint8 _lastGray;
 
 	/**
-	 * The colors from the current GfxPalette32 palette
-	 * before this SingleRemap is applied.
+	 * The colors from the current GfxPalette32 palette before this SingleRemap
+	 * is applied.
 	 */
 	Color _originalColors[236];
 
 	/**
-	 * Map of colors that changed in `_originalColors`
-	 * when this SingleRemap was updated. This map is
-	 * transient and gets reset to `false` after the
+	 * Map of colors that changed in `_originalColors` when this SingleRemap was
+	 * updated. This map is transient and gets reset to `false` after the
 	 * SingleRemap finishes updating.
 	 */
 	bool _originalColorsChanged[236];
 
 	/**
-	 * The ideal target RGB color values for each generated
-	 * remap color.
+	 * The ideal target RGB color values for each generated remap color.
 	 */
 	Color _idealColors[236];
 
 	/**
-	 * Map of colors that changed in `_idealColors` when
-	 * this SingleRemap was updated. This map is transient
-	 * and gets reset to `false` after the SingleRemap
-	 * finishes applying.
+	 * Map of colors that changed in `_idealColors` when this SingleRemap was
+	 * updated. This map is transient and gets reset to `false` after the
+	 * SingleRemap finishes applying.
 	 */
 	bool _idealColorsChanged[236];
 
 	/**
-	 * When applying a SingleRemap, finding an appropriate
-	 * color in the palette is the responsibility of a
-	 * distance function. Once a match is found, the
-	 * distance of that match is stored here so that the
-	 * next time the SingleRemap is applied, it can check
-	 * the distance from the previous application and avoid
-	 * triggering an expensive redraw of the entire screen
+	 * When applying a SingleRemap, finding an appropriate color in the palette
+	 * is the responsibility of a distance function. Once a match is found, the
+	 * distance of that match is stored here so that the next time the
+	 * SingleRemap is applied, it can check the distance from the previous
+	 * application and avoid triggering an expensive redraw of the entire screen
 	 * if the new palette value only changed slightly.
 	 */
 	int _matchDistances[236];
 
 	/**
-	 * Computes the final target values for a range remap
-	 * and applies them directly to the `_remaps` map.
+	 * Computes the final target values for a range remap and applies them
+	 * directly to the `_remaps` map.
 	 *
 	 * @note Was ByRange in SSCI.
 	 */
 	bool updateRange();
 
 	/**
-	 * Computes the intermediate target values for a
-	 * brightness remap and applies them indirectly via
-	 * the `apply` method.
+	 * Computes the intermediate target values for a brightness remap and
+	 * applies them indirectly via the `apply` method.
 	 *
 	 * @note Was ByPercent in SSCI.
 	 */
 	bool updateBrightness();
 
 	/**
-	 * Computes the intermediate target values for a
-	 * saturation remap and applies them indirectly via
-	 * the `apply` method.
+	 * Computes the intermediate target values for a saturation remap and
+	 * applies them indirectly via the `apply` method.
 	 *
 	 * @note Was ToGray in SSCI.
 	 */
 	bool updateSaturation();
 
 	/**
-	 * Computes the intermediate target values for a
-	 * saturation + brightness bitmap and applies them
-	 * indirectly via the `apply` method.
+	 * Computes the intermediate target values for a saturation + brightness
+	 * bitmap and applies them indirectly via the `apply` method.
 	 *
 	 * @note Was ToPercentGray in SSCI.
 	 */
 	bool updateSaturationAndBrightness();
 
 	/**
-	 * Computes and applies the final values to the
-	 * `_remaps` map.
+	 * Computes and applies the final values to the `_remaps` map.
 	 *
-	 * @note In SSCI, a boolean array of changed values
-	 * was passed into this method, but this was done by
-	 * creating arrays on the stack in the caller. Instead
-	 * of doing this, we simply add another member property
+	 * @note In SSCI, a boolean array of changed values was passed into this
+	 * method, but this was done by creating arrays on the stack in the caller.
+	 * Instead of doing this, we simply add another member property
 	 * `_idealColorsChanged` and use that instead.
 	 */
 	bool apply();
@@ -225,19 +206,18 @@ private:
 	/**
 	 * Calculates the square distance of two colors.
 	 *
-	 * @note In SSCI this method is Rgb24::Dist, but it is
-	 * only used by SingleRemap.
+	 * @note In SSCI this method is Rgb24::Dist, but it is only used by
+	 * SingleRemap.
 	 */
 	int colorDistance(const Color &a, const Color &b) const;
 
 	/**
-	 * Finds the closest index in the next palette matching
-	 * the given RGB color. Returns -1 if no match can be
-	 * found that is closer than `minimumDistance`.
+	 * Finds the closest index in the next palette matching the given RGB color.
+	 * Returns -1 if no match can be found that is closer than
+	 * `minimumDistance`.
 	 *
-	 * @note In SSCI, this method is SOLPalette::Match, but
-	 * this particular signature is only used by
-	 * SingleRemap.
+	 * @note In SSCI, this method is SOLPalette::Match, but this particular
+	 * signature is only used by SingleRemap.
 	 */
 	int16 matchColor(const Color &color, const int minimumDistance, int &outDistance, const bool *const blockedIndexes) const;
 };
@@ -246,8 +226,7 @@ private:
 #pragma mark GfxRemap32
 
 /**
- * This class provides color remapping support for SCI32
- * games.
+ * This class provides color remapping support for SCI32 games.
  */
 class GfxRemap32 : public Common::Serializable {
 public:
@@ -262,8 +241,8 @@ public:
 	inline int16 getBlockedRangeCount() const { return _blockedRangeCount; }
 
 	/**
-	 * Turns off remapping of the given color. If `color` is
-	 * 0, all remaps are turned off.
+	 * Turns off remapping of the given color. If `color` is 0, all remaps are
+	 * turned off.
 	 */
 	void remapOff(const uint8 color);
 
@@ -273,55 +252,48 @@ public:
 	void remapAllOff();
 
 	/**
-	 * Configures a SingleRemap for the remap color `color`.
-	 * The SingleRemap will shift palette colors between
-	 * `from` and `to` (inclusive) by `delta` palette
-	 * entries when the remap is applied.
+	 * Configures a SingleRemap for the remap color `color`. The SingleRemap
+	 * will shift palette colors between `from` and `to` (inclusive) by `delta`
+	 * palette entries when the remap is applied.
 	 */
 	void remapByRange(const uint8 color, const int16 from, const int16 to, const int16 delta);
 
 	/**
-	 * Configures a SingleRemap for the remap color `color`
-	 * to modify the brightness of remapped colors by
-	 * `percent`.
+	 * Configures a SingleRemap for the remap color `color` to modify the
+	 * brightness of remapped colors by `percent`.
 	 */
 	void remapByPercent(const uint8 color, const int16 percent);
 
 	/**
-	 * Configures a SingleRemap for the remap color `color`
-	 * to modify the saturation of remapped colors by
-	 * `gray`.
+	 * Configures a SingleRemap for the remap color `color` to modify the
+	 * saturation of remapped colors by `gray`.
 	 */
 	void remapToGray(const uint8 color, const int8 gray);
 
 	/**
-	 * Configures a SingleRemap for the remap color `color`
-	 * to modify the brightness of remapped colors by
-	 * `percent`, and saturation of remapped colors by
-	 * `gray`.
+	 * Configures a SingleRemap for the remap color `color` to modify the
+	 * brightness of remapped colors by `percent`, and saturation of remapped
+	 * colors by `gray`.
 	 */
 	void remapToPercentGray(const uint8 color, const int16 gray, const int16 percent);
 
 	/**
-	 * Prevents GfxRemap32 from using the given range of
-	 * palette entries as potential remap targets.
+	 * Prevents GfxRemap32 from using the given range of palette entries as
+	 * potential remap targets.
 	 *
 	 * @NOTE Was DontMapToRange in SSCI.
 	 */
 	void blockRange(const uint8 from, const int16 count);
 
 	/**
-	 * Determines whether or not the given color has an
-	 * active remapper. If it does not, it is treated as a
-	 * skip color and the pixel is not drawn.
+	 * Determines whether or not the given color has an active remapper. If it
+	 * does not, it is treated as a skip color and the pixel is not drawn.
 	 *
-	 * @note SSCI uses a boolean array to decide whether a
-	 * a pixel is remapped, but it is possible to get the
-	 * same information from `_remaps`, as this function
-	 * does.
-	 * Presumably, the separate array was created for
-	 * performance reasons, since this is called a lot in
-	 * the most critical section of the renderer.
+	 * @note SSCI uses a boolean array to decide whether a pixel is remapped,
+	 * but it is possible to get the same information from `_remaps`, as this
+	 * function does. Presumably, the separate array was created for performance
+	 * reasons, since this is called a lot in the most critical section of the
+	 * renderer.
 	 */
 	inline bool remapEnabled(uint8 color) const {
 		const uint8 index = _remapEndColor - color;
@@ -335,10 +307,9 @@ public:
 	}
 
 	/**
-	 * Calculates the correct color for a target by looking
-	 * up the target color in the SingleRemap that controls
-	 * the given sourceColor. If there is no remap for the
-	 * given color, it will be treated as a skip color.
+	 * Calculates the correct color for a target by looking up the target color
+	 * in the SingleRemap that controls the given sourceColor. If there is no
+	 * remap for the given color, it will be treated as a skip color.
 	 */
 	inline uint8 remapColor(const uint8 sourceColor, const uint8 targetColor) const {
 		const uint8 index = _remapEndColor - sourceColor;
@@ -358,12 +329,11 @@ public:
 	}
 
 	/**
-	 * Updates all active remaps in response to a palette
-	 * change or a remap settings change.
+	 * Updates all active remaps in response to a palette change or a remap
+	 * settings change.
 	 *
-	 * `paletteChanged` is true if the next palette in
-	 * GfxPalette32 has been previously modified by other
-	 * palette operations.
+	 * `paletteChanged` is true if the next palette in GfxPalette32 has been
+	 * previously modified by other palette operations.
 	 */
 	bool remapAllTables(const bool paletteUpdated);
 
@@ -371,14 +341,12 @@ private:
 	typedef Common::Array<SingleRemap> SingleRemapsList;
 
 	/**
-	 * The first index of the remap area in the system
-	 * palette.
+	 * The first index of the remap area in the system palette.
 	 */
 	const uint8 _remapStartColor;
 
 	/**
-	 * The last index of the remap area in the system
-	 * palette.
+	 * The last index of the remap area in the system palette.
 	 */
 	uint8 _remapEndColor;
 
@@ -393,20 +361,19 @@ private:
 	SingleRemapsList _remaps;
 
 	/**
-	 * If true, indicates that one or more SingleRemaps were
-	 * reconfigured and all remaps need to be recalculated.
+	 * If true, indicates that one or more SingleRemaps were reconfigured and
+	 * all remaps need to be recalculated.
 	 */
 	bool _needsUpdate;
 
 	/**
-	 * The first color that is blocked from being used as a
-	 * remap target color.
+	 * The first color that is blocked from being used as a remap target color.
 	 */
 	uint8 _blockedRangeStart;
 
 	/**
-	 * The size of the range of blocked colors. If zero,
-	 * all colors are potential targets for remapping.
+	 * The size of the range of blocked colors. If zero, all colors are
+	 * potential targets for remapping.
 	 */
 	int16 _blockedRangeCount;
 };