aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorColin Snover2017-10-05 20:53:27 -0500
committerColin Snover2017-10-06 22:10:50 -0500
commit5cffa3891f48d64154f00571c8127ef2c601d6d5 (patch)
tree56b8b94e67819ee70c8aa1a4054cc993ebff83c0
parentf037d4df1680aecefac2ffb240547385a74971d0 (diff)
downloadscummvm-rg350-5cffa3891f48d64154f00571c8127ef2c601d6d5.tar.gz
scummvm-rg350-5cffa3891f48d64154f00571c8127ef2c601d6d5.tar.bz2
scummvm-rg350-5cffa3891f48d64154f00571c8127ef2c601d6d5.zip
SCI32: Clean up GfxPalette32
* Replace raw pointers with smart pointers * Use references instead of const pointers where appropriate * Tweak initialisation * Tweak palette copies to the stack
-rw-r--r--engines/sci/engine/savegame.cpp40
-rw-r--r--engines/sci/graphics/palette32.cpp121
-rw-r--r--engines/sci/graphics/palette32.h14
3 files changed, 78 insertions, 97 deletions
diff --git a/engines/sci/engine/savegame.cpp b/engines/sci/engine/savegame.cpp
index b0fa31a1bb..06a26ec58e 100644
--- a/engines/sci/engine/savegame.cpp
+++ b/engines/sci/engine/savegame.cpp
@@ -866,25 +866,25 @@ void GfxPalette::saveLoadWithSerializer(Common::Serializer &s) {
}
#ifdef ENABLE_SCI32
-static void saveLoadPalette32(Common::Serializer &s, Palette *const palette) {
- s.syncAsUint32LE(palette->timestamp);
- for (int i = 0; i < ARRAYSIZE(palette->colors); ++i) {
- s.syncAsByte(palette->colors[i].used);
- s.syncAsByte(palette->colors[i].r);
- s.syncAsByte(palette->colors[i].g);
- s.syncAsByte(palette->colors[i].b);
+static void saveLoadPalette32(Common::Serializer &s, Palette &palette) {
+ s.syncAsUint32LE(palette.timestamp);
+ for (int i = 0; i < ARRAYSIZE(palette.colors); ++i) {
+ s.syncAsByte(palette.colors[i].used);
+ s.syncAsByte(palette.colors[i].r);
+ s.syncAsByte(palette.colors[i].g);
+ s.syncAsByte(palette.colors[i].b);
}
}
-static void saveLoadOptionalPalette32(Common::Serializer &s, Palette **const palette) {
+static void saveLoadOptionalPalette32(Common::Serializer &s, Common::ScopedPtr<Palette> &palette) {
bool hasPalette = false;
if (s.isSaving()) {
- hasPalette = (*palette != nullptr);
+ hasPalette = palette;
}
s.syncAsByte(hasPalette);
if (hasPalette) {
if (s.isLoading()) {
- *palette = new Palette;
+ palette.reset(new Palette);
}
saveLoadPalette32(s, *palette);
}
@@ -899,14 +899,11 @@ void GfxPalette32::saveLoadWithSerializer(Common::Serializer &s) {
++_version;
for (int i = 0; i < kNumCyclers; ++i) {
- delete _cyclers[i];
- _cyclers[i] = nullptr;
+ _cyclers[i].reset();
}
- delete _varyTargetPalette;
- _varyTargetPalette = nullptr;
- delete _varyStartPalette;
- _varyStartPalette = nullptr;
+ _varyTargetPalette.reset();
+ _varyStartPalette.reset();
}
s.syncAsSint16LE(_varyDirection);
@@ -928,14 +925,14 @@ void GfxPalette32::saveLoadWithSerializer(Common::Serializer &s) {
if (g_sci->_features->hasLatePaletteCode() && s.getVersion() >= 41) {
s.syncAsSint16LE(_gammaLevel);
- saveLoadPalette32(s, &_sourcePalette);
+ saveLoadPalette32(s, _sourcePalette);
++_version;
_needsUpdate = true;
_gammaChanged = true;
}
- saveLoadOptionalPalette32(s, &_varyTargetPalette);
- saveLoadOptionalPalette32(s, &_varyStartPalette);
+ saveLoadOptionalPalette32(s, _varyTargetPalette);
+ saveLoadOptionalPalette32(s, _varyStartPalette);
// _nextPalette is not saved by SSCI
@@ -944,14 +941,15 @@ void GfxPalette32::saveLoadWithSerializer(Common::Serializer &s) {
bool hasCycler = false;
if (s.isSaving()) {
- cycler = _cyclers[i];
+ cycler = _cyclers[i].get();
hasCycler = (cycler != nullptr);
}
s.syncAsByte(hasCycler);
if (hasCycler) {
if (s.isLoading()) {
- _cyclers[i] = cycler = new PalCycler;
+ cycler = new PalCycler;
+ _cyclers[i].reset(cycler);
}
s.syncAsByte(cycler->fromColor);
diff --git a/engines/sci/graphics/palette32.cpp b/engines/sci/graphics/palette32.cpp
index d8a2ead66d..7a3b396da8 100644
--- a/engines/sci/graphics/palette32.cpp
+++ b/engines/sci/graphics/palette32.cpp
@@ -39,11 +39,10 @@ namespace Sci {
HunkPalette::HunkPalette(const SciSpan<const byte> &rawPalette) :
_version(0),
- // NOTE: The header size in palettes is garbage. In at least KQ7 2.00b and
- // Phant1, the 999.pal sets this value to 0. In most other palettes it is
- // set to 14, but the *actual* size of the header structure used in SSCI is
- // 13, which is reflected by `kHunkPaletteHeaderSize`.
- // _headerSize(rawPalette[0]),
+ // The header size in palettes is garbage. In at least KQ7 2.00b and Phant1,
+ // the 999.pal sets this value to 0. In most other palettes it is set to 14,
+ // but the *actual* size of the header structure used in SSCI is 13, which
+ // is reflected by `kHunkPaletteHeaderSize`.
_numPalettes(rawPalette.getUint8At(kNumPaletteEntriesOffset)),
_data() {
assert(_numPalettes == 0 || _numPalettes == 1);
@@ -371,18 +370,17 @@ static const uint8 gammaTables[GfxPalette32::numGammaTables][256] = {
_varyLastTick(0),
_varyTime(0),
_varyDirection(0),
+ _varyPercent(0),
_varyTargetPercent(0),
_varyNumTimesPaused(0),
// Palette cycling
- _cyclers(),
_cycleMap(),
// Gamma correction
_gammaLevel(-1),
_gammaChanged(false) {
- _varyPercent = _varyTargetPercent;
for (int i = 0, len = ARRAYSIZE(_fadeTable); i < len; ++i) {
_fadeTable[i] = 100;
}
@@ -390,11 +388,6 @@ static const uint8 gammaTables[GfxPalette32::numGammaTables][256] = {
loadPalette(999);
}
-GfxPalette32::~GfxPalette32() {
- varyOff();
- cycleAllOff();
-}
-
bool GfxPalette32::loadPalette(const GuiResourceId resourceId) {
Resource *palResource = _resMan->findResource(ResourceId(kResourceTypePalette, resourceId), false);
@@ -604,16 +597,16 @@ void GfxPalette32::setVary(const Palette &target, const int16 percent, const int
}
void GfxPalette32::setVaryPercent(const int16 percent, const int32 ticks) {
- if (_varyTargetPalette != nullptr) {
+ if (_varyTargetPalette) {
setVaryTime(percent, ticks);
}
- // NOTE: SSCI had two additional parameters for this function to change the
+ // SSCI had two additional parameters for this function to change the
// `_varyFromColor`, but they were always hardcoded to be ignored
}
void GfxPalette32::setVaryTime(const int32 time) {
- if (_varyTargetPalette != nullptr) {
+ if (_varyTargetPalette) {
setVaryTime(_varyTargetPercent, time);
}
}
@@ -645,16 +638,8 @@ void GfxPalette32::varyOff() {
_varyFromColor = 0;
_varyToColor = 255;
_varyDirection = 0;
-
- if (_varyTargetPalette != nullptr) {
- delete _varyTargetPalette;
- _varyTargetPalette = nullptr;
- }
-
- if (_varyStartPalette != nullptr) {
- delete _varyStartPalette;
- _varyStartPalette = nullptr;
- }
+ _varyTargetPalette.reset();
+ _varyStartPalette.reset();
}
void GfxPalette32::varyPause() {
@@ -667,7 +652,7 @@ void GfxPalette32::varyOn() {
--_varyNumTimesPaused;
}
- if (_varyTargetPalette != nullptr && _varyNumTimesPaused == 0) {
+ if (_varyTargetPalette && _varyNumTimesPaused == 0) {
if (_varyPercent != _varyTargetPercent && _varyTime != 0) {
_varyDirection = (_varyTargetPercent - _varyPercent > 0) ? 1 : -1;
} else {
@@ -677,28 +662,26 @@ void GfxPalette32::varyOn() {
}
void GfxPalette32::setTarget(const Palette &palette) {
- delete _varyTargetPalette;
- _varyTargetPalette = new Palette(palette);
+ _varyTargetPalette.reset(new Palette(palette));
}
void GfxPalette32::setStart(const Palette &palette) {
- delete _varyStartPalette;
- _varyStartPalette = new Palette(palette);
+ _varyStartPalette.reset(new Palette(palette));
}
void GfxPalette32::mergeStart(const Palette &palette) {
- if (_varyStartPalette != nullptr) {
+ if (_varyStartPalette) {
mergePalette(*_varyStartPalette, palette);
} else {
- _varyStartPalette = new Palette(palette);
+ _varyStartPalette.reset(new Palette(palette));
}
}
void GfxPalette32::mergeTarget(const Palette &palette) {
- if (_varyTargetPalette != nullptr) {
+ if (_varyTargetPalette) {
mergePalette(*_varyTargetPalette, palette);
} else {
- _varyTargetPalette = new Palette(palette);
+ _varyTargetPalette.reset(new Palette(palette));
}
}
@@ -714,9 +697,9 @@ void GfxPalette32::applyVary() {
_varyPercent += _varyDirection;
}
- if (_varyPercent == 0 || _varyTargetPalette == nullptr) {
+ if (_varyPercent == 0 || !_varyTargetPalette) {
for (int i = 0; i < ARRAYSIZE(_nextPalette.colors); ++i) {
- if (_varyStartPalette != nullptr && i >= _varyFromColor && i <= _varyToColor) {
+ if (_varyStartPalette && i >= _varyFromColor && i <= _varyToColor) {
_nextPalette.colors[i] = _varyStartPalette->colors[i];
} else {
_nextPalette.colors[i] = _sourcePalette.colors[i];
@@ -728,7 +711,7 @@ void GfxPalette32::applyVary() {
Color targetColor = _varyTargetPalette->colors[i];
Color sourceColor;
- if (_varyStartPalette != nullptr) {
+ if (_varyStartPalette) {
sourceColor = _varyStartPalette->colors[i];
} else {
sourceColor = _sourcePalette.colors[i];
@@ -816,27 +799,28 @@ void GfxPalette32::setCycle(const uint8 fromColor, const uint8 toColor, const in
clearCycleMap(fromColor, cycler->numColorsToCycle);
} else {
for (int i = 0; i < kNumCyclers; ++i) {
- if (_cyclers[i] == nullptr) {
- _cyclers[i] = cycler = new PalCycler;
+ if (!_cyclers[i]) {
+ cycler = new PalCycler;
+ _cyclers[i].reset(cycler);
break;
}
}
}
- // If there are no free cycler slots, SCI engine overrides the first oldest
- // cycler that it finds, where "oldest" is determined by the difference
- // between the tick and now
+ // If there are no free cycler slots, SSCI overrides the first oldest cycler
+ // that it finds, where "oldest" is determined by the difference between the
+ // tick and now
if (cycler == nullptr) {
const uint32 now = g_sci->getTickCount();
uint32 minUpdateDelta = 0xFFFFFFFF;
for (int i = 0; i < kNumCyclers; ++i) {
- PalCycler *const candidate = _cyclers[i];
+ PalCyclerOwner &candidate = _cyclers[i];
const uint32 updateDelta = now - candidate->lastUpdateTick;
if (updateDelta < minUpdateDelta) {
minUpdateDelta = updateDelta;
- cycler = candidate;
+ cycler = candidate.get();
}
}
@@ -882,19 +866,18 @@ void GfxPalette32::cyclePause(const uint8 fromColor) {
void GfxPalette32::cycleAllOn() {
for (int i = 0; i < kNumCyclers; ++i) {
- PalCycler *const cycler = _cyclers[i];
- if (cycler != nullptr && cycler->numTimesPaused > 0) {
+ PalCyclerOwner &cycler = _cyclers[i];
+ if (cycler && cycler->numTimesPaused > 0) {
--cycler->numTimesPaused;
}
}
}
void GfxPalette32::cycleAllPause() {
- // NOTE: The original engine did not check for null pointers in the
- // palette cyclers pointer array.
+ // SSCI did not check for null pointers in the palette cyclers pointer array
for (int i = 0; i < kNumCyclers; ++i) {
- PalCycler *const cycler = _cyclers[i];
- if (cycler != nullptr) {
+ PalCyclerOwner &cycler = _cyclers[i];
+ if (cycler) {
// This seems odd, because currentCycle is 0..numColorsPerCycle,
// but fromColor is 0..255. When applyAllCycles runs, the values
// end up back in range
@@ -905,8 +888,8 @@ void GfxPalette32::cycleAllPause() {
applyAllCycles();
for (int i = 0; i < kNumCyclers; ++i) {
- PalCycler *const cycler = _cyclers[i];
- if (cycler != nullptr) {
+ PalCyclerOwner &cycler = _cyclers[i];
+ if (cycler) {
++cycler->numTimesPaused;
}
}
@@ -914,11 +897,10 @@ void GfxPalette32::cycleAllPause() {
void GfxPalette32::cycleOff(const uint8 fromColor) {
for (int i = 0; i < kNumCyclers; ++i) {
- PalCycler *const cycler = _cyclers[i];
- if (cycler != nullptr && cycler->fromColor == fromColor) {
+ PalCyclerOwner &cycler = _cyclers[i];
+ if (cycler && cycler->fromColor == fromColor) {
clearCycleMap(fromColor, cycler->numColorsToCycle);
- delete cycler;
- _cyclers[i] = nullptr;
+ _cyclers[i].reset();
break;
}
}
@@ -926,11 +908,10 @@ void GfxPalette32::cycleOff(const uint8 fromColor) {
void GfxPalette32::cycleAllOff() {
for (int i = 0; i < kNumCyclers; ++i) {
- PalCycler *const cycler = _cyclers[i];
- if (cycler != nullptr) {
+ PalCyclerOwner &cycler = _cyclers[i];
+ if (cycler) {
clearCycleMap(cycler->fromColor, cycler->numColorsToCycle);
- delete cycler;
- _cyclers[i] = nullptr;
+ _cyclers[i].reset();
}
}
}
@@ -969,9 +950,9 @@ void GfxPalette32::setCycleMap(const uint16 fromColor, const uint16 numColorsToS
PalCycler *GfxPalette32::getCycler(const uint16 fromColor) {
for (int cyclerIndex = 0; cyclerIndex < kNumCyclers; ++cyclerIndex) {
- PalCycler *cycler = _cyclers[cyclerIndex];
- if (cycler != nullptr && cycler->fromColor == fromColor) {
- return cycler;
+ PalCyclerOwner &cycler = _cyclers[cyclerIndex];
+ if (cycler && cycler->fromColor == fromColor) {
+ return cycler.get();
}
}
@@ -980,12 +961,12 @@ PalCycler *GfxPalette32::getCycler(const uint16 fromColor) {
void GfxPalette32::applyAllCycles() {
Color paletteCopy[256];
- memcpy(paletteCopy, _nextPalette.colors, sizeof(Color) * 256);
+ memcpy(paletteCopy, _nextPalette.colors, sizeof(paletteCopy));
for (int i = 0; i < kNumCyclers; ++i) {
- PalCycler *const cycler = _cyclers[i];
- if (cycler != nullptr) {
- cycler->currentCycle = (((int) cycler->currentCycle) + 1) % cycler->numColorsToCycle;
+ PalCyclerOwner &cycler = _cyclers[i];
+ if (cycler) {
+ cycler->currentCycle = (cycler->currentCycle + 1) % cycler->numColorsToCycle;
for (int j = 0; j < cycler->numColorsToCycle; j++) {
_nextPalette.colors[cycler->fromColor + j] = paletteCopy[cycler->fromColor + (cycler->currentCycle + j) % cycler->numColorsToCycle];
}
@@ -995,12 +976,12 @@ void GfxPalette32::applyAllCycles() {
void GfxPalette32::applyCycles() {
Color paletteCopy[256];
- memcpy(paletteCopy, _nextPalette.colors, sizeof(Color) * 256);
+ memcpy(paletteCopy, _nextPalette.colors, sizeof(paletteCopy));
const uint32 now = g_sci->getTickCount();
for (int i = 0; i < kNumCyclers; ++i) {
- PalCycler *const cycler = _cyclers[i];
- if (cycler == nullptr) {
+ PalCyclerOwner &cycler = _cyclers[i];
+ if (!cycler) {
continue;
}
diff --git a/engines/sci/graphics/palette32.h b/engines/sci/graphics/palette32.h
index b8ba85eb4a..2d00b86501 100644
--- a/engines/sci/graphics/palette32.h
+++ b/engines/sci/graphics/palette32.h
@@ -23,6 +23,8 @@
#ifndef SCI_GRAPHICS_PALETTE32_H
#define SCI_GRAPHICS_PALETTE32_H
+#include "common/ptr.h"
+
namespace Sci {
#pragma mark HunkPalette
@@ -233,7 +235,6 @@ struct PalCycler {
class GfxPalette32 {
public:
GfxPalette32(ResourceManager *resMan);
- ~GfxPalette32();
void saveLoadWithSerializer(Common::Serializer &s);
@@ -440,13 +441,13 @@ private:
* operation. If this palette is not specified, `_sourcePalette` is used
* instead.
*/
- Palette *_varyStartPalette;
+ Common::ScopedPtr<Palette> _varyStartPalette;
/**
* An optional palette used to provide target colors for a palette vary
* operation.
*/
- Palette *_varyTargetPalette;
+ Common::ScopedPtr<Palette> _varyTargetPalette;
/**
* The minimum palette index that has been varied from the source palette.
@@ -553,7 +554,8 @@ private:
kNumCyclers = 10
};
- PalCycler *_cyclers[kNumCyclers];
+ typedef Common::ScopedPtr<PalCycler> PalCyclerOwner;
+ PalCyclerOwner _cyclers[kNumCyclers];
/**
* Updates the `currentCycle` of the given `cycler` by `speed` entries.
@@ -564,8 +566,8 @@ private:
* The cycle map is used to detect overlapping cyclers, and to avoid
* remapping to palette entries that are being cycled.
*
- * According to SCI engine code, when two cyclers overlap, a fatal error has
- * occurred and the engine will display an error and then exit.
+ * According to SSCI, when two cyclers overlap, a fatal error has occurred
+ * and the engine will display an error and then exit.
*
* The color remapping system avoids attempts to remap to palette entries
* that are cycling because they won't be the expected color once the cycler