[Scummvm-git-logs] scummvm master -> 750e2060bf35b44b344d9cabc20d5f23cfa8b87e

[phcoder]

This automated email contains information about 1 new commit which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
750e2060bf NEVERHOOD: Fix crash on expiring animated sprite backref


Commit: 750e2060bf35b44b344d9cabc20d5f23cfa8b87e
    https://github.com/scummvm/scummvm/commit/750e2060bf35b44b344d9cabc20d5f23cfa8b87e
Author: Vladimir Serbinenko (phcoder at gmail.com)
Date: 2023-01-29T07:19:36+01:00

Commit Message:
NEVERHOOD: Fix crash on expiring animated sprite backref

Thanks a lot to -=CHE at TER=- for discovering this and excellent debugging
work.

Changed paths:
    engines/neverhood/sprite.cpp
    engines/neverhood/sprite.h


diff --git a/engines/neverhood/sprite.cpp b/engines/neverhood/sprite.cpp
index 8cc058c47be..c79e2a87a83 100644
--- a/engines/neverhood/sprite.cpp
+++ b/engines/neverhood/sprite.cpp
@@ -183,13 +183,13 @@ void StaticSprite::updatePosition() {
 // AnimatedSprite
 
 AnimatedSprite::AnimatedSprite(NeverhoodEngine *vm, int objectPriority)
-	: Sprite(vm, objectPriority), _animResource(vm), _subtitleSurface(new AnimatedSpriteSubtitles(vm, this)) {
+	: Sprite(vm, objectPriority), _animResource(vm), _subtitleSurface(new AnimatedSpriteSubtitles(vm)) {
 
 	init();
 }
 
 AnimatedSprite::AnimatedSprite(NeverhoodEngine *vm, uint32 fileHash, int surfacePriority, int16 x, int16 y)
-	: Sprite(vm, 1100), _animResource(vm), _subtitleSurface(new AnimatedSpriteSubtitles(vm, this)) {
+	: Sprite(vm, 1100), _animResource(vm), _subtitleSurface(new AnimatedSpriteSubtitles(vm)) {
 
 	init();
 	SetUpdateHandler(&AnimatedSprite::update);
@@ -224,6 +224,8 @@ void AnimatedSprite::init() {
 	_plFirstFrameHash = 0;
 	_plLastFrameHash = 0;
 	_animStatus = 0;
+	if (_subtitleSurface)
+		_subtitleSurface->setFrameIndex(-1, 0);
 }
 
 void AnimatedSprite::update() {
@@ -309,7 +311,8 @@ void AnimatedSprite::updateAnim() {
 			if (_animStatus == 1) {
 				if (_animResource.load(_newAnimFileHash)) {
 					_currAnimFileHash = _newAnimFileHash;
-					_subtitles.reset(new SubtitlePlayer(_vm, _newAnimFileHash, kSubtitleWidth));
+					if (_subtitleSurface)
+						_subtitleSurface->setHash(_newAnimFileHash);
 				} else {
 					_animResource.load(calcHash("sqDefault"));
 					_currAnimFileHash = 0;
@@ -323,7 +326,8 @@ void AnimatedSprite::updateAnim() {
 			} else {
 				if (_animResource.load(_newAnimFileHash)) {
 					_currAnimFileHash = _newAnimFileHash;
-					_subtitles.reset(new SubtitlePlayer(_vm, _newAnimFileHash, kSubtitleWidth));
+					if (_subtitleSurface)
+						_subtitleSurface->setHash(_newAnimFileHash);
 				} else {
 					_animResource.load(calcHash("sqDefault"));
 					_currAnimFileHash = 0;
@@ -348,6 +352,8 @@ void AnimatedSprite::updateAnim() {
 
 	}
 
+	if (_subtitleSurface)
+		_subtitleSurface->setFrameIndex(_currFrameIndex, _lastFrameIndex);
 }
 
 void AnimatedSprite::updatePosition() {
@@ -367,11 +373,7 @@ void AnimatedSprite::updatePosition() {
 		_surface->getDrawRect().y = filterY(_y + _drawOffset.y);
 	}
 
-	int subCenterX = _surface->getDrawRect().x + _surface->getDrawRect().width / 2;
-	_subtitleSurface->getDrawRect().x = MAX(subCenterX - kSubtitleWidth / 2, 0);
-	_subtitleSurface->getDrawRect().width = kSubtitleWidth;
-	_subtitleSurface->getDrawRect().y = MIN(_surface->getDrawRect().y + _surface->getDrawRect().height + 1, 480 - (SubtitlePlayer::kSubtitleCharHeight - 1));
-	_subtitleSurface->getDrawRect().height = SubtitlePlayer::kSubtitleCharHeight;
+	_subtitleSurface->updatePosition(_surface->getDrawRect());
 
 	if (_needRefresh) {
 		_surface->drawAnimResource(_animResource, _currFrameIndex, _doDeltaX, _doDeltaY, _drawOffset.width, _drawOffset.height);
@@ -400,23 +402,42 @@ void AnimatedSprite::updateFrameIndex() {
 				_currFrameIndex = _lastFrameIndex;
 		}
 	}
+
+	if (_subtitleSurface)
+		_subtitleSurface->setFrameIndex(_currFrameIndex, _lastFrameIndex);
+}
+
+AnimatedSprite::AnimatedSpriteSubtitles::AnimatedSpriteSubtitles(NeverhoodEngine *vm) :
+	BaseSurface(vm, 0xffff, kSubtitleWidth, SubtitlePlayer::kSubtitleCharHeight, "animated sprite subtitles"), _currFrameIndex(-1), _subCenterX(320) {
+}
+
+void AnimatedSprite::AnimatedSpriteSubtitles::setHash(uint32 fileHash) {
+	_subtitles.reset(new SubtitlePlayer(_vm, fileHash, kSubtitleWidth));
+}
+
+void AnimatedSprite::AnimatedSpriteSubtitles::setFrameIndex(int currFrameIndex, int lastFrameIndex) {
+	if (currFrameIndex > 0 && currFrameIndex < lastFrameIndex)
+		_currFrameIndex = currFrameIndex;
+	else
+		_currFrameIndex = -1;
 }
 
-AnimatedSprite::AnimatedSpriteSubtitles::AnimatedSpriteSubtitles(NeverhoodEngine *vm, AnimatedSprite *backref) :
-	_backref(backref), BaseSurface(vm, 0xffff, kSubtitleWidth, SubtitlePlayer::kSubtitleCharHeight, "animated sprite subtitles") {
+void AnimatedSprite::AnimatedSpriteSubtitles::updatePosition(const NDrawRect &parentDrawRect) {
+	_subCenterX = parentDrawRect.x + parentDrawRect.width / 2;
+	getDrawRect().x = MAX(_subCenterX - kSubtitleWidth / 2, 0);
+	getDrawRect().width = kSubtitleWidth;
+	getDrawRect().y = MIN(parentDrawRect.y + parentDrawRect.height + 1, 480 - (SubtitlePlayer::kSubtitleCharHeight - 1));
+	getDrawRect().height = SubtitlePlayer::kSubtitleCharHeight;
 }
 
 void AnimatedSprite::AnimatedSpriteSubtitles::draw() {
-	if (_backref->_subtitles && _backref->_subtitles->isValid() &&
-	    _backref->_currFrameIndex != 0 &&
-	    _backref->_currFrameIndex < _backref->_lastFrameIndex) {
-		int subCenterX = _backref->_surface->getDrawRect().x + _backref->_surface->getDrawRect().width / 2;
-		_backref->_subtitles->renderFrame(_backref->_currFrameIndex, subCenterX - getDrawRect().x);
-		const Graphics::Surface *bottom = _backref->_subtitles->getBottomSubs();
+	if (_subtitles && _subtitles->isValid() && _currFrameIndex > 0) {
+		_subtitles->renderFrame(_currFrameIndex, _subCenterX - getDrawRect().x);
+		const Graphics::Surface *bottom = _subtitles->getBottomSubs();
 		if (bottom) {
-			_vm->_screen->drawSurface2(bottom, _drawRect, _clipRect, true, ++_version, nullptr, _backref->_subtitles->getSubtitleAlpha());
+			_vm->_screen->drawSurface2(bottom, _drawRect, _clipRect, true, ++_version, nullptr, _subtitles->getSubtitleAlpha());
 		}
-		if (_backref->_subtitles->getTopSubs())
+		if (_subtitles->getTopSubs())
 			warning("Top subs are unsupported");
 	}
 }
diff --git a/engines/neverhood/sprite.h b/engines/neverhood/sprite.h
index b19a7a7c3b7..9265382d531 100644
--- a/engines/neverhood/sprite.h
+++ b/engines/neverhood/sprite.h
@@ -155,15 +155,19 @@ protected:
 	class AnimatedSpriteSubtitles : public BaseSurface {
 	public:
 		void draw() override;
-		AnimatedSpriteSubtitles(NeverhoodEngine *vm, AnimatedSprite *backRef);
+		AnimatedSpriteSubtitles(NeverhoodEngine *vm);
+		void setFrameIndex(int currFrameIndex, int lastFrameIndex);
+		void setHash(uint32 fileHash);
+		void updatePosition(const NDrawRect &parentDrawRect);
 	private:
-		AnimatedSprite *_backref;
+		Common::ScopedPtr<SubtitlePlayer> _subtitles;
+		int _currFrameIndex;
+		int _subCenterX;
 	};
 	
 	static const int kSubtitleWidth = 320;
 	Common::SharedPtr<AnimatedSpriteSubtitles> _subtitleSurface;
 	typedef void (AnimatedSprite::*AnimationCb)();
-	Common::ScopedPtr<SubtitlePlayer> _subtitles;
 	AnimResource _animResource;
 	uint32 _currAnimFileHash, _newAnimFileHash, _nextAnimFileHash;
 	int16 _currFrameIndex, _lastFrameIndex;