diff options
author | Colin Snover | 2017-10-06 15:14:49 -0500 |
---|---|---|
committer | Colin Snover | 2017-10-06 22:11:03 -0500 |
commit | 1b4214695566ccadbe27806f646bed3c4c944809 (patch) | |
tree | 9dc35e32aec4482f22d6d973229fcc6e0098ef51 | |
parent | a2c8674252743c451600f53c77df3c7fac6e7786 (diff) | |
download | scummvm-rg350-1b4214695566ccadbe27806f646bed3c4c944809.tar.gz scummvm-rg350-1b4214695566ccadbe27806f646bed3c4c944809.tar.bz2 scummvm-rg350-1b4214695566ccadbe27806f646bed3c4c944809.zip |
SCI32: Clean up GfxRemap32
* Rewrap comments to 80 columns
* Clarify comments where possible
-rw-r--r-- | engines/sci/engine/kgraphics32.cpp | 24 | ||||
-rw-r--r-- | engines/sci/graphics/remap32.cpp | 50 | ||||
-rw-r--r-- | engines/sci/graphics/remap32.h | 217 |
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; }; |