SCI32: Clean up CelObj

* Rewrap comments to 80 columns
* Clarify comments where possible
* Use smart pointers where appropriate
* Change view/pic flags detection to always use word-size
  (byte-size check for flag 0x80 was a compiler optimisation)

3 files changed, 181 insertions, 236 deletions

diff --git a/engines/sci/engine/segment.h b/engines/sci/engine/segment.h
index d6e2dd3c24..2a4fd9149b 100644
--- a/engines/sci/engine/segment.h
+++ b/engines/sci/engine/segment.h
@@ -1102,12 +1102,11 @@ public:
 
 	BITMAP_PROPERTY(32, DataOffset, 24);
 
-	// NOTE: This property is used as a "magic number" for
-	// validating that a block of memory is a valid bitmap,
-	// and so is always set to the size of the header.
+	// This property is used as a "magic number" for validating that a block of
+	// memory is a valid bitmap, and so is always set to the size of the header.
 	BITMAP_PROPERTY(32, UncompressedDataOffset, 28);
 
-	// NOTE: This property always seems to be zero
+	// This property always seems to be zero in SSCI
 	BITMAP_PROPERTY(32, ControlOffset, 32);
 
 	inline uint16 getXResolution() const {
diff --git a/engines/sci/graphics/celobj32.cpp b/engines/sci/graphics/celobj32.cpp
index a49690a703..c2ae29b24e 100644
--- a/engines/sci/graphics/celobj32.cpp
+++ b/engines/sci/graphics/celobj32.cpp
@@ -35,7 +35,7 @@
 namespace Sci {
 #pragma mark CelScaler
 
-CelScaler *CelObj::_scaler = nullptr;
+Common::ScopedPtr<CelScaler> CelObj::_scaler;
 
 void CelScaler::activateScaleTables(const Ratio &scaleX, const Ratio &scaleY) {
 	for (int i = 0; i < ARRAYSIZE(_scaleTables); ++i) {
@@ -74,9 +74,9 @@ void CelScaler::buildLookupTable(int *table, const Ratio &ratio, const int size)
 	}
 }
 
-const CelScalerTable *CelScaler::getScalerTable(const Ratio &scaleX, const Ratio &scaleY) {
+const CelScalerTable &CelScaler::getScalerTable(const Ratio &scaleX, const Ratio &scaleY) {
 	activateScaleTables(scaleX, scaleY);
-	return &_scaleTables[_activeIndex];
+	return _scaleTables[_activeIndex];
 }
 
 #pragma mark -
@@ -87,21 +87,13 @@ void CelObj::init() {
 	CelObj::deinit();
 	_drawBlackLines = false;
 	_nextCacheId = 1;
-	_scaler = new CelScaler();
-	_cache = new CelCache;
-	_cache->resize(100);
+	_scaler.reset(new CelScaler());
+	_cache.reset(new CelCache(100));
 }
 
 void CelObj::deinit() {
-	delete _scaler;
-	_scaler = nullptr;
-	if (_cache != nullptr) {
-		for (CelCache::iterator it = _cache->begin(); it != _cache->end(); ++it) {
-			delete it->celObj;
-		}
-	}
-	delete _cache;
-	_cache = nullptr;
+	_scaler.reset();
+	_cache.reset();
 }
 
 #pragma mark -
@@ -172,10 +164,9 @@ struct SCALER_Scale {
 	_minX(targetRect.left),
 	_maxX(targetRect.right - 1),
 #endif
-	// The maximum width of the scaled object may not be as
-	// wide as the source data it requires if downscaling,
-	// so just always make the reader decompress an entire
-	// line of source data when scaling
+	// The maximum width of the scaled object may not be as wide as the source
+	// data it requires if downscaling, so just always make the reader
+	// decompress an entire line of source data when scaling
 	_reader(celObj, celObj._width) {
 #ifndef NDEBUG
 		assert(_minX <= _maxX);
@@ -203,39 +194,39 @@ struct SCALER_Scale {
 		// games which use global scaling are the ones that use low-resolution
 		// script coordinates too.
 
-		const CelScalerTable *table = CelObj::_scaler->getScalerTable(scaleX, scaleY);
+		const CelScalerTable &table = CelObj::_scaler->getScalerTable(scaleX, scaleY);
 
-		if (g_sci->_gfxFrameout->getCurrentBuffer().scriptWidth == kLowResX) {
+		if (g_sci->_gfxFrameout->getScriptWidth() == kLowResX) {
 			const int16 unscaledX = (scaledPosition.x / scaleX).toInt();
 			if (FLIP) {
 				const int lastIndex = celObj._width - 1;
 				for (int16 x = targetRect.left; x < targetRect.right; ++x) {
-					_valuesX[x] = lastIndex - (table->valuesX[x] - unscaledX);
+					_valuesX[x] = lastIndex - (table.valuesX[x] - unscaledX);
 				}
 			} else {
 				for (int16 x = targetRect.left; x < targetRect.right; ++x) {
-					_valuesX[x] = table->valuesX[x] - unscaledX;
+					_valuesX[x] = table.valuesX[x] - unscaledX;
 				}
 			}
 
 			const int16 unscaledY = (scaledPosition.y / scaleY).toInt();
 			for (int16 y = targetRect.top; y < targetRect.bottom; ++y) {
-				_valuesY[y] = table->valuesY[y] - unscaledY;
+				_valuesY[y] = table.valuesY[y] - unscaledY;
 			}
 		} else {
 			if (FLIP) {
 				const int lastIndex = celObj._width - 1;
 				for (int16 x = targetRect.left; x < targetRect.right; ++x) {
-					_valuesX[x] = lastIndex - table->valuesX[x - scaledPosition.x];
+					_valuesX[x] = lastIndex - table.valuesX[x - scaledPosition.x];
 				}
 			} else {
 				for (int16 x = targetRect.left; x < targetRect.right; ++x) {
-					_valuesX[x] = table->valuesX[x - scaledPosition.x];
+					_valuesX[x] = table.valuesX[x - scaledPosition.x];
 				}
 			}
 
 			for (int16 y = targetRect.top; y < targetRect.bottom; ++y) {
-				_valuesY[y] = table->valuesY[y - scaledPosition.y];
+				_valuesY[y] = table.valuesY[y - scaledPosition.y];
 			}
 		}
 	}
@@ -412,8 +403,8 @@ struct MAPPER_NoMDNoSkip {
 struct MAPPER_Map {
 	inline void draw(byte *target, const byte pixel, const uint8 skipColor) const {
 		if (pixel != skipColor) {
-			// NOTE: For some reason, SSCI never checks if the source
-			// pixel is *above* the range of remaps.
+			// For some reason, SSCI never checks if the source pixel is *above*
+			// the range of remaps, so we do not either.
 			if (pixel < g_sci->_gfxRemap32->getStartColor()) {
 				*target = pixel;
 			} else if (g_sci->_gfxRemap32->remapEnabled(pixel)) {
@@ -429,8 +420,8 @@ struct MAPPER_Map {
  */
 struct MAPPER_NoMap {
 	inline void draw(byte *target, const byte pixel, const uint8 skipColor) const {
-		// NOTE: For some reason, SSCI never checks if the source
-		// pixel is *above* the range of remaps.
+		// For some reason, SSCI never checks if the source pixel is *above* the
+		// range of remaps, so we do not either.
 		if (pixel != skipColor && pixel < g_sci->_gfxRemap32->getStartColor()) {
 			*target = pixel;
 		}
@@ -444,9 +435,9 @@ void CelObj::draw(Buffer &target, const ScreenItem &screenItem, const Common::Re
 	_drawBlackLines = screenItem._drawBlackLines;
 
 	if (_remap) {
-		// NOTE: In the original code this check was `g_Remap_numActiveRemaps && _remap`,
-		// but since we are already in a `_remap` branch, there is no reason to check it
-		// again
+		// In SSCI, this check was `g_Remap_numActiveRemaps && _remap`, but
+		// since we are already in a `_remap` branch, there is no reason to
+		// check that again
 		if (g_sci->_gfxRemap32->getRemapCount()) {
 			if (scaleX.isOne() && scaleY.isOne()) {
 				if (_compressionType == kCelCompressionNone) {
@@ -612,7 +603,7 @@ void CelObj::submitPalette() const {
 #pragma mark CelObj - Caching
 
 int CelObj::_nextCacheId = 1;
-CelCache *CelObj::_cache = nullptr;
+Common::ScopedPtr<CelCache> CelObj::_cache;
 
 int CelObj::searchCache(const CelInfo32 &celInfo, int *const nextInsertIndex) const {
 	*nextInsertIndex = -1;
@@ -648,12 +639,7 @@ void CelObj::putCopyInCache(const int cacheIndex) const {
 	}
 
 	CelCacheEntry &entry = (*_cache)[cacheIndex];
-
-	if (entry.celObj != nullptr) {
-		delete entry.celObj;
-	}
-
-	entry.celObj = duplicate();
+	entry.celObj.reset(duplicate());
 	entry.id = ++_nextCacheId;
 }
 
@@ -807,8 +793,8 @@ void CelObj::drawUncompHzFlipNoMDNoSkip(Buffer &target, const Common::Rect &targ
 }
 
 void CelObj::scaleDrawNoMD(Buffer &target, const Ratio &scaleX, const Ratio &scaleY, const Common::Rect &targetRect, const Common::Point &scaledPosition) const {
-	// In SSCI the checks are > because their rects are BR-inclusive;
-	// our checks are >= because our rects are BR-exclusive
+	// In SSCI the checks are > because their rects are BR-inclusive; our checks
+	// are >= because our rects are BR-exclusive
 	if (g_sci->_features->hasEmptyScaleDrawHack() &&
 		(targetRect.left >= targetRect.right ||
 		 targetRect.top >= targetRect.bottom)) {
@@ -822,8 +808,8 @@ void CelObj::scaleDrawNoMD(Buffer &target, const Ratio &scaleX, const Ratio &sca
 }
 
 void CelObj::scaleDrawUncompNoMD(Buffer &target, const Ratio &scaleX, const Ratio &scaleY, const Common::Rect &targetRect, const Common::Point &scaledPosition) const {
-	// In SSCI the checks are > because their rects are BR-inclusive;
-	// our checks are >= because our rects are BR-exclusive
+	// In SSCI the checks are > because their rects are BR-inclusive; our checks
+	// are >= because our rects are BR-exclusive
 	if (g_sci->_features->hasEmptyScaleDrawHack() &&
 		(targetRect.left >= targetRect.right ||
 		 targetRect.top >= targetRect.bottom)) {
@@ -861,11 +847,11 @@ int16 CelObjView::getNumCels(const GuiResourceId viewId, int16 loopNo) {
 
 	const uint16 loopCount = data[2];
 
-	// Every version of SCI32 has a logic error in this function that causes
-	// random memory to be read if a script requests the cel count for one
-	// past the maximum loop index. For example, GK1 room 808 does this, and
-	// gets stuck in an infinite loop because the game script expects this
-	// method to return a non-zero value.
+	// Every version of SSCI has a logic error in this function that causes
+	// random memory to be read if a script requests the cel count for one past
+	// the maximum loop index. For example, GK1 room 808 does this, and gets
+	// stuck in an infinite loop because the game script expects this method to
+	// return a non-zero value.
 	// This bug is triggered in basically every SCI32 game and appears to be
 	// universally fixable simply by always using the next lowest loop instead.
 	if (loopNo == loopCount) {
@@ -904,7 +890,7 @@ CelObjView::CelObjView(const GuiResourceId viewId, const int16 loopNo, const int
 	const int cacheIndex = searchCache(_info, &cacheInsertIndex);
 	if (cacheIndex != -1) {
 		CelCacheEntry &entry = (*_cache)[cacheIndex];
-		const CelObjView *const cachedCelObj = dynamic_cast<CelObjView *>(entry.celObj);
+		const CelObjView *const cachedCelObj = dynamic_cast<CelObjView *>(entry.celObj.get());
 		if (cachedCelObj == nullptr) {
 			error("Expected a CelObjView in cache slot %d", cacheIndex);
 		}
@@ -915,7 +901,7 @@ CelObjView::CelObjView(const GuiResourceId viewId, const int16 loopNo, const int
 
 	const Resource *const resource = g_sci->getResMan()->findResource(ResourceId(kResourceTypeView, viewId), false);
 
-	// NOTE: SCI2.1/SQ6 just silently returns here.
+	// SSCI just silently returns here
 	if (!resource) {
 		error("View resource %d not found", viewId);
 	}
@@ -944,8 +930,6 @@ CelObjView::CelObjView(const GuiResourceId viewId, const int16 loopNo, const int
 		_info.loopNo = loopCount - 1;
 	}
 
-	// NOTE: This is the actual check, in the actual location,
-	// from SCI engine.
 	if (loopNo < 0) {
 		error("Loop is less than 0");
 	}
@@ -1011,10 +995,8 @@ CelObjView::CelObjView(const GuiResourceId viewId, const int16 loopNo, const int
 		error("Compression type not supported - V: %d  L: %d  C: %d", _info.resourceId, _info.loopNo, _info.celNo);
 	}
 
-	if (celHeader[10] & 128) {
-		// NOTE: This is correct according to SCI2.1/SQ6/DOS;
-		// the engine re-reads the byte value as a word value
-		const uint16 flags = celHeader.getUint16SEAt(10);
+	const uint16 flags = celHeader.getUint16SEAt(10);
+	if (flags & 0x80) {
 		_transparent = flags & 1 ? true : false;
 		_remap = flags & 2 ? true : false;
 	} else if (_compressionType == kCelCompressionNone) {
@@ -1096,7 +1078,7 @@ Common::Point CelObjView::getLinkPosition(const int16 linkId) const {
 				Common::Point point;
 				point.x = linkTable.getInt16SEAt(0);
 				if (_mirrorX) {
-					// NOTE: SSCI had an off-by-one error here (missing -1)
+					// SSCI had an off-by-one error here (missing -1)
 					point.x = _width - point.x - 1;
 				}
 				point.y = linkTable.getInt16SEAt(2);
@@ -1127,7 +1109,7 @@ CelObjPic::CelObjPic(const GuiResourceId picId, const int16 celNo) {
 	const int cacheIndex = searchCache(_info, &cacheInsertIndex);
 	if (cacheIndex != -1) {
 		CelCacheEntry &entry = (*_cache)[cacheIndex];
-		const CelObjPic *const cachedCelObj = dynamic_cast<CelObjPic *>(entry.celObj);
+		const CelObjPic *const cachedCelObj = dynamic_cast<CelObjPic *>(entry.celObj.get());
 		if (cachedCelObj == nullptr) {
 			error("Expected a CelObjPic in cache slot %d", cacheIndex);
 		}
@@ -1138,7 +1120,7 @@ CelObjPic::CelObjPic(const GuiResourceId picId, const int16 celNo) {
 
 	const Resource *const resource = g_sci->getResMan()->findResource(ResourceId(kResourceTypePic, picId), false);
 
-	// NOTE: SCI2.1/SQ6 just silently returns here.
+	// SSCI just silently returns here
 	if (!resource) {
 		error("Pic resource %d not found", picId);
 	}
@@ -1183,10 +1165,9 @@ CelObjPic::CelObjPic(const GuiResourceId picId, const int16 celNo) {
 		_yResolution = 400;
 	}
 
-	if (celHeader.getUint8At(10) & 128) {
-		// NOTE: This is correct according to SCI2.1/SQ6/DOS;
-		// the engine re-reads the byte value as a word value
-		const uint16 flags = celHeader.getUint16SEAt(10);
+
+	const uint16 flags = celHeader.getUint16SEAt(10);
+	if (flags & 0x80) {
 		_transparent = flags & 1 ? true : false;
 		_remap = flags & 2 ? true : false;
 	} else {
@@ -1251,7 +1232,8 @@ CelObjMem::CelObjMem(const reg_t bitmapObject) {
 
 	SciBitmap *bitmap = g_sci->getEngineState()->_segMan->lookupBitmap(bitmapObject);
 
-	// NOTE: SSCI did no error checking here at all.
+	// SSCI did no error checking here at all so would just end up reading
+	// garbage or crashing if this ever happened
 	if (!bitmap) {
 		error("Bitmap %04x:%04x not found", PRINT_REG(bitmapObject));
 	}
@@ -1293,8 +1275,8 @@ CelObjColor::CelObjColor(const uint8 color, const int16 width, const int16 heigh
 }
 
 void CelObjColor::draw(Buffer &target, const ScreenItem &screenItem, const Common::Rect &targetRect, const bool mirrorX) {
-	// TODO: The original engine sets this flag but why? One cannot
-	// draw a solid color mirrored.
+	// One cannot draw a solid color mirrored, but SSCI sets it anyway, so we do
+	// too
 	_drawMirrored = mirrorX;
 	draw(target, targetRect);
 }
diff --git a/engines/sci/graphics/celobj32.h b/engines/sci/graphics/celobj32.h
index 6e50f7232e..02b2859f5c 100644
--- a/engines/sci/graphics/celobj32.h
+++ b/engines/sci/graphics/celobj32.h
@@ -33,14 +33,14 @@ namespace Sci {
 typedef Common::Rational Ratio;
 
 // SCI32 has four different coordinate systems:
-// 1. low resolution, 2. game/script resolution,
-// 3. text/bitmap resolution, 4. screen resolution
+// 1. low resolution, 2. game/script resolution, 3. text/bitmap resolution,
+// 4. screen resolution
 //
-// In CelObj, these values are used when there is
-// no baked in resolution of cels.
+// In CelObj, these values are used when there is no baked in resolution of
+// cels.
 //
-// In ScreenItem, it is used when deciding which
-// path to take to calculate dimensions.
+// In ScreenItem, it is used when deciding which path to take to calculate
+// dimensions.
 enum {
 	kLowResX = 320,
 	kLowResY = 200
@@ -60,8 +60,7 @@ enum CelCompressionType {
 };
 
 /**
- * A CelInfo32 object describes the basic properties of a
- * cel object.
+ * A CelInfo32 object describes the basic properties of a cel object.
  */
 struct CelInfo32 {
 	/**
@@ -70,26 +69,24 @@ struct CelInfo32 {
 	CelType type;
 
 	/**
-	 * For cel objects that draw from resources, the ID of
-	 * the resource to load.
+	 * For cel objects that draw from resources, the ID of the resource to load.
 	 */
 	GuiResourceId resourceId;
 
 	/**
-	 * For CelObjView, the loop number to draw from the
-	 * view resource.
+	 * For CelObjView, the loop number to draw from the view resource.
 	 */
 	int16 loopNo;
 
 	/**
-	 * For CelObjView and CelObjPic, the cel number to draw
-	 * from the view or pic resource.
+	 * For CelObjView and CelObjPic, the cel number to draw from the view or pic
+	 * resource.
 	 */
 	int16 celNo;
 
 	/**
-	 * For CelObjMem, a segment register pointing to a heap
-	 * resource containing headered bitmap data.
+	 * For CelObjMem, a segment register pointing to a heap resource containing
+	 * headered bitmap data.
 	 */
 	reg_t bitmap;
 
@@ -98,18 +95,16 @@ struct CelInfo32 {
 	 */
 	uint8 color;
 
-	// NOTE: In at least SCI2.1/SQ6, color is left
-	// uninitialised.
 	CelInfo32() :
+		// In SSCI, color is left uninitialised
 		type(kCelTypeMem),
 		resourceId(0),
 		loopNo(0),
 		celNo(0),
 		bitmap(NULL_REG) {}
 
-	// NOTE: This is the equivalence criteria used by
-	// CelObj::searchCache in at least SCI2.1/SQ6. Notably,
-	// it does not check the color field.
+	// This is the equivalence criteria used by CelObj::searchCache in at least
+	// SSCI SQ6. Notably, it does not check the color field.
 	inline bool operator==(const CelInfo32 &other) {
 		return (
 			type == other.type &&
@@ -143,13 +138,12 @@ struct CelInfo32 {
 class CelObj;
 struct CelCacheEntry {
 	/**
-	 * A monotonically increasing cache ID used to identify
-	 * the least recently used item in the cache for
-	 * replacement.
+	 * A monotonically increasing cache ID used to identify the least recently
+	 * used item in the cache for replacement.
 	 */
 	int id;
-	CelObj *celObj;
-	CelCacheEntry() : id(0), celObj(nullptr) {}
+	Common::ScopedPtr<CelObj> celObj;
+	CelCacheEntry() : id(0) {}
 };
 
 typedef Common::Array<CelCacheEntry> CelCache;
@@ -166,9 +160,9 @@ enum {
 
 struct CelScalerTable {
 	/**
-	 * A lookup table of indexes that should be used to find
-	 * the correct column to read from the source bitmap
-	 * when drawing a scaled version of the source bitmap.
+	 * A lookup table of indexes that should be used to find the correct column
+	 * to read from the source bitmap when drawing a scaled version of the
+	 * source bitmap.
 	 */
 	int valuesX[kCelScalerTableSize];
 
@@ -178,9 +172,9 @@ struct CelScalerTable {
 	Ratio scaleX;
 
 	/**
-	 * A lookup table of indexes that should be used to find
-	 * the correct row to read from a source bitmap when
-	 * drawing a scaled version of the source bitmap.
+	 * A lookup table of indexes that should be used to find the correct row to
+	 * read from a source bitmap when drawing a scaled version of the source
+	 * bitmap.
 	 */
 	int valuesY[kCelScalerTableSize];
 
@@ -202,25 +196,23 @@ class CelScaler {
 	int _activeIndex;
 
 	/**
-	 * Activates a scale table for the given X and Y ratios.
-	 * If there is no table that matches the given ratios,
-	 * the least most recently used table will be replaced
-	 * and activated.
+	 * Activates a scale table for the given X and Y ratios. If there is no
+	 * table that matches the given ratios, the least most recently used table
+	 * will be replaced and activated.
 	 */
 	void activateScaleTables(const Ratio &scaleX, const Ratio &scaleY);
 
 	/**
-	 * Builds a pixel lookup table in `table` for the given
-	 * ratio. The table will be filled up to the specified
-	 * size, which should be large enough to draw across the
-	 * entire target buffer.
+	 * Builds a pixel lookup table in `table` for the given ratio. The table
+	 * will be filled up to the specified size, which should be large enough to
+	 * draw across the entire target buffer.
 	 */
 	void buildLookupTable(int *table, const Ratio &ratio, const int size);
 
 public:
 	CelScaler() :
-	_scaleTables(),
-	_activeIndex(0) {
+		_scaleTables(),
+		_activeIndex(0) {
 		CelScalerTable &table = _scaleTables[0];
 		table.scaleX = Ratio();
 		table.scaleY = Ratio();
@@ -236,7 +228,7 @@ public:
 	/**
 	 * Retrieves scaler tables for the given X and Y ratios.
 	 */
-	const CelScalerTable *getScalerTable(const Ratio &scaleX, const Ratio &scaleY);
+	const CelScalerTable &getScalerTable(const Ratio &scaleX, const Ratio &scaleY);
 };
 
 #pragma mark -
@@ -244,53 +236,47 @@ public:
 
 class ScreenItem;
 /**
- * A cel object is the lowest-level rendering primitive in
- * the SCI engine and draws itself directly to a target
- * pixel buffer.
+ * A cel object is the lowest-level rendering primitive in the SCI engine and
+ * draws itself directly to a target pixel buffer.
  */
 class CelObj {
 protected:
 	/**
-	 * When true, every second line of the cel will be
-	 * rendered as a black line.
+	 * When true, every second line of the cel will be rendered as a black line.
 	 *
 	 * @see ScreenItem::_drawBlackLines
-	 * @note Using a static member because otherwise this
-	 * would otherwise need to be copied down through
-	 * several calls. (SSCI did similar, using a global
-	 * variable.)
+	 * @note Using a static member because otherwise this would otherwise need
+	 * to be copied down through several calls. (SSCI did similar, using a
+	 * global variable.)
 	 */
 	static bool _drawBlackLines;
 
 	/**
-	 * When true, this cel will be horizontally mirrored
-	 * when it is drawn. This is an internal flag that is
-	 * set by draw methods based on the combination of the
-	 * cel's `_mirrorX` property and the owner screen item's
-	 * `_mirrorX` property.
+	 * When true, this cel will be horizontally mirrored when it is drawn. This
+	 * is an internal flag that is set by draw methods based on the combination
+	 * of the cel's `_mirrorX` property and the owner screen item's `_mirrorX`
+	 * property.
 	 */
 	bool _drawMirrored;
 
 public:
-	static CelScaler *_scaler;
+	static Common::ScopedPtr<CelScaler> _scaler;
 
 	/**
-	 * The basic identifying information for this cel. This
-	 * information effectively acts as a composite key for
-	 * a cel object, and any cel object can be recreated
-	 * from this data alone.
+	 * The basic identifying information for this cel. This information
+	 * effectively acts as a composite key for a cel object, and any cel object
+	 * can be recreated from this data alone.
 	 */
 	CelInfo32 _info;
 
 	/**
-	 * The offset to the cel header for this cel within the
-	 * raw resource data.
+	 * The offset to the cel header for this cel within the raw resource data.
 	 */
 	uint32 _celHeaderOffset;
 
 	/**
-	 * The offset to the embedded palette for this cel
-	 * within the raw resource data.
+	 * The offset to the embedded palette for this cel within the raw resource
+	 * data.
 	 */
 	uint32 _hunkPaletteOffset;
 
@@ -300,36 +286,32 @@ public:
 	uint16 _width, _height;
 
 	/**
-	 * TODO: Documentation
+	 * The origin of the cel, relative to the top-left corner, in cel
+	 * coordinates.
 	 */
 	Common::Point _origin;
 
 	/**
-	 * The dimensions of the original coordinate system for
-	 * the cel. Used to scale cels from their native size
-	 * to the correct size on screen.
+	 * The dimensions of the original coordinate system for the cel. Used to
+	 * scale cels from their native size to the correct size on screen.
 	 *
-	 * @note This is set to scriptWidth/Height for
-	 * CelObjColor. For other cel objects, the value comes
-	 * from the raw resource data. For text bitmaps, this is
-	 * the width/height of the coordinate system used to
-	 * generate the text, which also defaults to
-	 * scriptWidth/Height but seems to typically be changed
-	 * to more closely match the native screen resolution.
+	 * @note This is set to scriptWidth/Height for CelObjColor. For other cel
+	 * objects, the value comes from the raw resource data. For text bitmaps,
+	 * this is the width/height of the coordinate system used to generate the
+	 * text, which also defaults to scriptWidth/Height but seems to typically be
+	 * changed to more closely match the native screen resolution.
 	 */
 	uint16 _xResolution, _yResolution;
 
 	/**
-	 * The skip (transparent) color for the cel. When
-	 * compositing, any pixels matching this color will not
-	 * be copied to the buffer.
+	 * The skip (transparent) color for the cel. When compositing, any pixels
+	 * matching this color will not be copied to the buffer.
 	 */
 	uint8 _skipColor;
 
 	/**
-	 * Whether or not this cel has any transparent regions.
-	 * This is used for optimised drawing of non-transparent
-	 * cels.
+	 * Whether or not this cel has any transparent regions. This is used for
+	 * optimised drawing of non-transparent cels.
 	 */
 	bool _transparent;
 
@@ -339,15 +321,14 @@ public:
 	CelCompressionType _compressionType;
 
 	/**
-	 * Whether or not this cel should be palette-remapped?
+	 * Whether or not this cel contains remap pixels.
 	 */
 	bool _remap;
 
 	/**
-	 * If true, the cel contains pre-mirrored picture data.
-	 * This value comes directly from the resource data and
-	 * is XORed with the `_mirrorX` property of the owner
-	 * screen item when rendering.
+	 * If true, the cel contains pre-mirrored picture data. This value comes
+	 * directly from the resource data and is XORed with the `_mirrorX` property
+	 * of the owner screen item when rendering.
 	 */
 	bool _mirrorX;
 
@@ -364,71 +345,63 @@ public:
 	virtual ~CelObj() {};
 
 	/**
-	 * Draws the cel to the target buffer using the priority
-	 * and positioning information from the given screen
-	 * item. The mirroring of the cel will be unchanged from
-	 * any previous call to draw.
+	 * Draws the cel to the target buffer using the priority and positioning
+	 * information from the given screen item. The mirroring of the cel will be
+	 * unchanged from any previous call to draw.
 	 */
 	void draw(Buffer &target, const ScreenItem &screenItem, const Common::Rect &targetRect) const;
 
 	/**
-	 * Draws the cel to the target buffer using the priority
-	 * and positioning information from the given screen
-	 * item and the given mirror flag.
+	 * Draws the cel to the target buffer using the priority and positioning
+	 * information from the given screen item and the given mirror flag.
 	 *
-	 * @note In SCI engine, this function was a virtual
-	 * function, but CelObjView, CelObjPic, and CelObjMem
-	 * all used the same function and the compiler
-	 * deduplicated the copies; we deduplicate the source by
-	 * putting the implementation on CelObj instead of
-	 * copying it to 3/4 of the subclasses.
+	 * @note In SSCI, this function was a virtual function, but CelObjView,
+	 * CelObjPic, and CelObjMem all used the same function and the compiler
+	 * deduplicated the copies; we deduplicate the source by putting the
+	 * implementation on CelObj instead of copying it to 3/4 of the subclasses.
 	 */
 	virtual void draw(Buffer &target, const ScreenItem &screenItem, const Common::Rect &targetRect, const bool mirrorX);
 
 	/**
-	 * Draws the cel to the target buffer using the
-	 * positioning and mirroring information from the
-	 * provided arguments.
+	 * Draws the cel to the target buffer using the positioning and mirroring
+	 * information from the provided arguments.
 	 *
-	 * @note In SCI engine, this function was a virtual
-	 * function, but CelObjView, CelObjPic, and CelObjMem
-	 * all used the same function and the compiler
-	 * deduplicated the copies; we deduplicate the source by
-	 * putting the implementation on CelObj instead of
-	 * copying it to 3/4 of the subclasses.
+	 * @note In SSCI, this function was a virtual function, but CelObjView,
+	 * CelObjPic, and CelObjMem all used the same function and the compiler
+	 * deduplicated the copies; we deduplicate the source by putting the
+	 * implementation on CelObj instead of copying it to 3/4 of the subclasses.
 	 */
 	virtual void draw(Buffer &target, const Common::Rect &targetRect, const Common::Point &scaledPosition, const bool mirrorX);
 
 	/**
-	 * Draws the cel to the target buffer using the given
-	 * position and scaling parameters. The mirroring of the
-	 * cel will be unchanged from any previous call to draw.
+	 * Draws the cel to the target buffer using the given position and scaling
+	 * parameters. The mirroring of the cel will be unchanged from any previous
+	 * call to draw.
 	 */
 	void drawTo(Buffer &target, const Common::Rect &targetRect, const Common::Point &scaledPosition, const Ratio &scaleX, const Ratio &scaleY) const;
 
 	/**
-	 * Creates a copy of this cel on the free store and
-	 * returns a pointer to the new object. The new cel will
-	 * point to a shared copy of bitmap/resource data.
+	 * Creates a copy of this cel on the free store and returns a pointer to the
+	 * new object. The new cel will point to a shared copy of bitmap/resource
+	 * data.
 	 */
 	virtual CelObj *duplicate() const = 0;
 
 	/**
-	 * Retrieves a pointer to the raw resource data for this
-	 * cel. This method cannot be used with a CelObjColor.
+	 * Retrieves a pointer to the raw resource data for this cel. This method
+	 * cannot be used with a CelObjColor.
 	 */
 	virtual const SciSpan<const byte> getResPointer() const = 0;
 
 	/**
-	 * Reads the pixel at the given coordinates. This method
-	 * is valid only for CelObjView and CelObjPic.
+	 * Reads the pixel at the given coordinates. This method is valid only for
+	 * CelObjView and CelObjPic.
 	 */
 	virtual uint8 readPixel(const uint16 x, const uint16 y, const bool mirrorX) const;
 
 	/**
-	 * Submits the palette from this cel to the palette
-	 * manager for integration into the master screen
-	 * palette.
+	 * Submits the palette from this cel to the palette manager for integration
+	 * into the master screen palette.
 	 */
 	void submitPalette() const;
 
@@ -454,7 +427,8 @@ private:
 	void drawUncompHzFlipMap(Buffer &target, const Common::Rect &targetRect, const Common::Point &scaledPosition) const;
 	void scaleDrawMap(Buffer &target, const Ratio &scaleX, const Ratio &scaleY, const Common::Rect &targetRect, const Common::Point &scaledPosition) const;
 	void scaleDrawUncompMap(Buffer &target, const Ratio &scaleX, const Ratio &scaleY, const Common::Rect &targetRect, const Common::Point &scaledPosition) const;
-	// NOTE: The original includes versions of the above functions with priority parameters, which were not actually used in SCI32
+	// SSCI includes versions of the above functions with priority parameters
+	// which are not actually used in SCI32
 
 	void drawHzFlipNoMD(Buffer &target, const Common::Rect &targetRect, const Common::Point &scaledPosition) const;
 	void drawNoFlipNoMD(Buffer &target, const Common::Rect &targetRect, const Common::Point &scaledPosition) const;
@@ -464,37 +438,34 @@ private:
 	void drawUncompHzFlipNoMDNoSkip(Buffer &target, const Common::Rect &targetRect, const Common::Point &scaledPosition) const;
 	void scaleDrawNoMD(Buffer &target, const Ratio &scaleX, const Ratio &scaleY, const Common::Rect &targetRect, const Common::Point &scaledPosition) const;
 	void scaleDrawUncompNoMD(Buffer &target, const Ratio &scaleX, const Ratio &scaleY, const Common::Rect &targetRect, const Common::Point &scaledPosition) const;
-	// NOTE: The original includes versions of the above functions with priority parameters, which were not actually used in SCI32
+	// SSCI includes versions of the above functions with priority parameters
+	// which are not actually used in SCI32
 
 #pragma mark -
 #pragma mark CelObj - Caching
 protected:
 	/**
-	 * A monotonically increasing cache ID used to identify
-	 * the least recently used item in the cache for
-	 * replacement.
+	 * A monotonically increasing cache ID used to identify the least recently
+	 * used item in the cache for replacement.
 	 */
 	static int _nextCacheId;
 
 	/**
-	 * A cache of cel objects used to avoid reinitialisation
-	 * overhead for cels with the same CelInfo32.
+	 * A cache of cel objects used to avoid reinitialisation overhead for cels
+	 * with the same CelInfo32.
 	 */
-	// NOTE: At least SQ6 uses a fixed cache size of 100.
-	static CelCache *_cache;
+	static Common::ScopedPtr<CelCache> _cache;
 
 	/**
-	 * Searches the cel cache for a CelObj matching the
-	 * provided CelInfo32. If not found, -1 is returned.
-	 * nextInsertIndex will receive the index of the oldest
-	 * item in the cache, which can be used to replace
-	 * the oldest item with a newer item.
+	 * Searches the cel cache for a CelObj matching the provided CelInfo32. If
+	 * not found, -1 is returned. `nextInsertIndex` will receive the index of
+	 * the oldest item in the cache, which can be used to replace the oldest
+	 * item with a newer item.
 	 */
 	int searchCache(const CelInfo32 &celInfo, int *nextInsertIndex) const;
 
 	/**
-	 * Puts a copy of this CelObj into the cache at the
-	 * given cache index.
+	 * Puts a copy of this CelObj into the cache at the given cache index.
 	 */
 	void putCopyInCache(int index) const;
 };
@@ -503,22 +474,20 @@ protected:
 #pragma mark CelObjView
 
 /**
- * A CelObjView is the drawing primitive for a View type
- * resource. Each CelObjView corresponds to a single cel
- * within a single loop of a view.
+ * A CelObjView is the drawing primitive for a View type resource. Each
+ * CelObjView corresponds to a single cel within a single loop of a view.
  */
 class CelObjView : public CelObj {
 private:
 	/**
-	 * Analyses resources without baked-in remap flags
-	 * to determine whether or not they should be remapped.
+	 * Analyses resources without baked-in remap flags to determine whether or
+	 * not they should be remapped.
 	 */
 	bool analyzeUncompressedForRemap() const;
 
 	/**
-	 * Analyses compressed resources without baked-in remap
-	 * flags to determine whether or not they should be
-	 * remapped.
+	 * Analyses compressed resources without baked-in remap flags to determine
+	 * whether or not they should be remapped.
 	 */
 	bool analyzeForRemap() const;
 
@@ -532,9 +501,8 @@ public:
 	static int16 getNumCels(const GuiResourceId viewId, const int16 loopNo);
 
 	/**
-	 * Draws the cel to the target buffer using the
-	 * positioning, mirroring, and scaling information from
-	 * the provided arguments.
+	 * Draws the cel to the target buffer using the positioning, mirroring, and
+	 * scaling information from the provided arguments.
 	 */
 	void draw(Buffer &target, const Common::Rect &targetRect, const Common::Point &scaledPosition, bool mirrorX, const Ratio &scaleX, const Ratio &scaleY);
 
@@ -548,16 +516,14 @@ public:
 #pragma mark CelObjPic
 
 /**
- * A CelObjPic is the drawing primitive for a Picture type
- * resource. Each CelObjPic corresponds to a single cel
- * within a picture.
+ * A CelObjPic is the drawing primitive for a Picture type resource. Each
+ * CelObjPic corresponds to a single cel within a picture.
  */
 class CelObjPic : public CelObj {
 private:
 	/**
-	 * Analyses uncompressed resources without baked-in skip
-	 * flags to determine whether or not they can use fast
-	 * blitting.
+	 * Analyses uncompressed resources without baked-in skip flags to determine
+	 * whether or not they can use fast blitting.
 	 */
 	bool analyzeUncompressedForSkip() const;
 
@@ -568,14 +534,13 @@ public:
 	uint8 _celCount;
 
 	/**
-	 * The position of this cel relative to the top-left
-	 * corner of the picture.
+	 * The position of this cel relative to the top-left corner of the picture.
 	 */
 	Common::Point _relativePosition;
 
 	/**
-	 * The z-buffer priority for this cel. Higher prorities
-	 * are drawn on top of lower priorities.
+	 * The z-buffer priority for this cel. Higher prorities are drawn on top of
+	 * lower priorities.
 	 */
 	int16 _priority;
 
@@ -593,10 +558,9 @@ public:
 #pragma mark CelObjMem
 
 /**
- * A CelObjMem is the drawing primitive for arbitrary
- * bitmaps generated in memory. Generated bitmaps in SCI32
- * include text & vector drawings and per-pixel screen
- * transitions like dissolves.
+ * A CelObjMem is the drawing primitive for arbitrary bitmaps generated in
+ * memory. Generated bitmaps in SCI32 include text & vector drawings and
+ * per-pixel screen transitions like dissolves.
  */
 class CelObjMem : public CelObj {
 public: