[Scummvm-git-logs] scummvm master -> 1c34e1c614570f8b8870527d7af701a2190ddcec

fracturehill noreply at scummvm.org
Sat Oct 7 20:32:48 UTC 2023


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

Summary:
7f84df864d AGS: Fix incorrect format check in generic blitting code
d542838664 Revert "AGS: Fix clipping issue in blitting code"
8fc73a36a6 Revert "AGS: Fix blitting-related crash"
1c34e1c614 AGS: Fix for surface blitting-related crashes


Commit: 7f84df864d011f607046aa94b5b9e1f4cbeed797
    https://github.com/scummvm/scummvm/commit/7f84df864d011f607046aa94b5b9e1f4cbeed797
Author: Kaloyan Chehlarski (strahy at outlook.com)
Date: 2023-10-07T23:32:10+03:00

Commit Message:
AGS: Fix incorrect format check in generic blitting code

This fixes a bunch of errors in 5 Days a Stranger (and all other
games with 16-bit surfaces), including leaked memory and
incorrectly displayed text.

Changed paths:
    engines/ags/lib/allegro/surface_generic.cpp


diff --git a/engines/ags/lib/allegro/surface_generic.cpp b/engines/ags/lib/allegro/surface_generic.cpp
index 1dc753cfed9..40989b09fa8 100644
--- a/engines/ags/lib/allegro/surface_generic.cpp
+++ b/engines/ags/lib/allegro/surface_generic.cpp
@@ -88,7 +88,7 @@ void BITMAP::drawInnerGeneric(DrawInnerArgs &args) {
 				*destVal = srcCol;
 				continue;
 			} else if ((DestBytesPerPixel == SrcBytesPerPixel) && args.srcAlpha == -1) {
-				if (DestBytesPerPixel)
+				if (DestBytesPerPixel == 4)
 					*(uint32 *)destVal = srcCol;
 				else
 					*(uint16 *)destVal = srcCol;


Commit: d5428386640ca46e77e614b71682b51badfec534
    https://github.com/scummvm/scummvm/commit/d5428386640ca46e77e614b71682b51badfec534
Author: Kaloyan Chehlarski (strahy at outlook.com)
Date: 2023-10-07T23:32:10+03:00

Commit Message:
Revert "AGS: Fix clipping issue in blitting code"

This reverts commit 90beb8887ec9f593e1a9ef6a67461d95d4b9ae4b.
The changes were based on a wrong assumption on
where the issue lies, and also introduced even buggier
behavior in scrolling scenes.

Changed paths:
    engines/ags/lib/allegro/surface_avx2.cpp
    engines/ags/lib/allegro/surface_generic.cpp
    engines/ags/lib/allegro/surface_neon.cpp
    engines/ags/lib/allegro/surface_sse2.cpp


diff --git a/engines/ags/lib/allegro/surface_avx2.cpp b/engines/ags/lib/allegro/surface_avx2.cpp
index e0d031bda0c..e0032a40d16 100644
--- a/engines/ags/lib/allegro/surface_avx2.cpp
+++ b/engines/ags/lib/allegro/surface_avx2.cpp
@@ -479,14 +479,14 @@ static void drawInner4BppWithConv(BITMAP::DrawInnerArgs &args) {
 	// Clip the bounds ahead of time (so we don't waste time checking if we are in bounds when
 	// we are in the inner loop)
 	int xCtrStart = 0, xCtrBppStart = 0, xCtrWidth = args.dstRect.width();
+	if (args.xStart + xCtrWidth > args.destArea.w) {
+		xCtrWidth = args.destArea.w - args.xStart;
+	}
 	if (args.xStart < 0) {
 		xCtrStart = -args.xStart;
 		xCtrBppStart = xCtrStart * SrcBytesPerPixel;
 		args.xStart = 0;
 	}
-	if (args.xStart + xCtrWidth > args.destArea.w) {
-		xCtrWidth = args.destArea.w - args.xStart;
-	}
 	int destY = args.yStart, srcYCtr = 0, yCtr = 0, scaleYCtr = 0, yCtrHeight = (xCtrWidth % 4 == 0) ? args.dstRect.height() : (args.dstRect.height() - 1);
 	if (Scale) yCtrHeight = args.dstRect.height();
 	if (args.yStart < 0) {
diff --git a/engines/ags/lib/allegro/surface_generic.cpp b/engines/ags/lib/allegro/surface_generic.cpp
index 40989b09fa8..5da1536a66b 100644
--- a/engines/ags/lib/allegro/surface_generic.cpp
+++ b/engines/ags/lib/allegro/surface_generic.cpp
@@ -34,14 +34,14 @@ void BITMAP::drawInnerGeneric(DrawInnerArgs &args) {
 	// Instead of skipping pixels outside our boundary here, we just clip
 	// our area instead.
 	int xCtrStart = 0, xCtrBppStart = 0, xCtrWidth = args.dstRect.width();
+	if (args.xStart + xCtrWidth > args.destArea.w) { // Clip the right
+		xCtrWidth = args.destArea.w - args.xStart;
+	}
 	if (args.xStart < 0) { // Clip the left
 		xCtrStart = -args.xStart;
 		xCtrBppStart = xCtrStart * SrcBytesPerPixel;
 		args.xStart = 0;
 	}
-	if (args.xStart + xCtrWidth > args.destArea.w) { // Clip the right
-		xCtrWidth = args.destArea.w - args.xStart;
-	}
 	int destY = args.yStart, yCtr = 0, srcYCtr = 0, scaleYCtr = 0, yCtrHeight = args.dstRect.height();
 	if (args.yStart < 0) { // Clip the top
 		yCtr = -args.yStart;
diff --git a/engines/ags/lib/allegro/surface_neon.cpp b/engines/ags/lib/allegro/surface_neon.cpp
index 21e4712d7a4..ea414e287e3 100644
--- a/engines/ags/lib/allegro/surface_neon.cpp
+++ b/engines/ags/lib/allegro/surface_neon.cpp
@@ -476,14 +476,14 @@ static void drawInner4BppWithConv(BITMAP::DrawInnerArgs &args) {
 	// Clip the bounds ahead of time (so we don't waste time checking if we are in bounds when
 	// we are in the inner loop)
 	int xCtrStart = 0, xCtrBppStart = 0, xCtrWidth = args.dstRect.width();
+	if (args.xStart + xCtrWidth > args.destArea.w) {
+		xCtrWidth = args.destArea.w - args.xStart;
+	}
 	if (args.xStart < 0) {
 		xCtrStart = -args.xStart;
 		xCtrBppStart = xCtrStart * SrcBytesPerPixel;
 		args.xStart = 0;
 	}
-	if (args.xStart + xCtrWidth > args.destArea.w) {
-		xCtrWidth = args.destArea.w - args.xStart;
-	}
 	int destY = args.yStart, srcYCtr = 0, yCtr = 0, scaleYCtr = 0, yCtrHeight = (xCtrWidth % 4 == 0) ? args.dstRect.height() : (args.dstRect.height() - 1);
 	if (Scale) yCtrHeight = args.dstRect.height();
 	if (args.yStart < 0) {
diff --git a/engines/ags/lib/allegro/surface_sse2.cpp b/engines/ags/lib/allegro/surface_sse2.cpp
index a2b9ceafb1b..651050ea4a8 100644
--- a/engines/ags/lib/allegro/surface_sse2.cpp
+++ b/engines/ags/lib/allegro/surface_sse2.cpp
@@ -496,14 +496,14 @@ static void drawInner4BppWithConv(BITMAP::DrawInnerArgs &args) {
 	// Clip the bounds ahead of time (so we don't waste time checking if we are in bounds when
 	// we are in the inner loop)
 	int xCtrStart = 0, xCtrBppStart = 0, xCtrWidth = args.dstRect.width();
+	if (args.xStart + xCtrWidth > args.destArea.w) {
+		xCtrWidth = args.destArea.w - args.xStart;
+	}
 	if (args.xStart < 0) {
 		xCtrStart = -args.xStart;
 		xCtrBppStart = xCtrStart * SrcBytesPerPixel;
 		args.xStart = 0;
 	}
-	if (args.xStart + xCtrWidth > args.destArea.w) {
-		xCtrWidth = args.destArea.w - args.xStart;
-	}
 	int destY = args.yStart, srcYCtr = 0, yCtr = 0, scaleYCtr = 0, yCtrHeight = (xCtrWidth % 4 == 0) ? args.dstRect.height() : (args.dstRect.height() - 1);
 	if (Scale) yCtrHeight = args.dstRect.height();
 	if (args.yStart < 0) {


Commit: 8fc73a36a63f061889f0cffd407a00551136c1bb
    https://github.com/scummvm/scummvm/commit/8fc73a36a63f061889f0cffd407a00551136c1bb
Author: Kaloyan Chehlarski (strahy at outlook.com)
Date: 2023-10-07T23:32:11+03:00

Commit Message:
Revert "AGS: Fix blitting-related crash"

This reverts commit e2720d39f327bd152a472cf41b1272fb0e866d16.
The changes were based on a wrong assumption on where
the issue actually lies, and introduced a new problem:
some surfaces' bottom rows were getting entirely skipped.

Changed paths:
    engines/ags/lib/allegro/surface_avx2.cpp
    engines/ags/lib/allegro/surface_neon.cpp
    engines/ags/lib/allegro/surface_sse2.cpp


diff --git a/engines/ags/lib/allegro/surface_avx2.cpp b/engines/ags/lib/allegro/surface_avx2.cpp
index e0032a40d16..a3b78394a38 100644
--- a/engines/ags/lib/allegro/surface_avx2.cpp
+++ b/engines/ags/lib/allegro/surface_avx2.cpp
@@ -505,9 +505,7 @@ static void drawInner4BppWithConv(BITMAP::DrawInnerArgs &args) {
 	const byte *srcP = (const byte *)args.src.getBasePtr(
 	                       args.horizFlip ? args.srcArea.right - 8 : args.srcArea.left,
 	                       args.vertFlip ? args.srcArea.bottom - 1 - yCtr : args.srcArea.top + yCtr);
-
-	// When not scaling, last row is handled separately
-	for (; Scale ? (yCtr < yCtrHeight) : (yCtr < yCtrHeight - 1); ++destY, ++yCtr, scaleYCtr += args.scaleY) {
+	for (; yCtr < yCtrHeight; ++destY, ++yCtr, scaleYCtr += args.scaleY) {
 		__m256i xCtrWidthSIMD = _mm256_set1_epi32(xCtrWidth); // This is the width of the row
 
 		if (!Scale) {
diff --git a/engines/ags/lib/allegro/surface_neon.cpp b/engines/ags/lib/allegro/surface_neon.cpp
index ea414e287e3..5fa0f273fad 100644
--- a/engines/ags/lib/allegro/surface_neon.cpp
+++ b/engines/ags/lib/allegro/surface_neon.cpp
@@ -502,9 +502,7 @@ static void drawInner4BppWithConv(BITMAP::DrawInnerArgs &args) {
 	const byte *srcP = (const byte *)args.src.getBasePtr(
 	                       args.horizFlip ? args.srcArea.right - 4 : args.srcArea.left,
 	                       args.vertFlip ? args.srcArea.bottom - 1 - yCtr : args.srcArea.top + yCtr);
-
-	// When not scaling, last row is handled separately
-	for (; Scale ? (yCtr < yCtrHeight) : (yCtr < yCtrHeight - 1); ++destY, ++yCtr, scaleYCtr += args.scaleY) {
+	for (; yCtr < yCtrHeight; ++destY, ++yCtr, scaleYCtr += args.scaleY) {
 		uint32x4_t xCtrWidthSIMD = vdupq_n_u32(xCtrWidth); // This is the width of the row
 
 		if (!Scale) {
diff --git a/engines/ags/lib/allegro/surface_sse2.cpp b/engines/ags/lib/allegro/surface_sse2.cpp
index 651050ea4a8..80b161ddaa3 100644
--- a/engines/ags/lib/allegro/surface_sse2.cpp
+++ b/engines/ags/lib/allegro/surface_sse2.cpp
@@ -522,9 +522,7 @@ static void drawInner4BppWithConv(BITMAP::DrawInnerArgs &args) {
 	const byte *srcP = (const byte *)args.src.getBasePtr(
 	                       args.horizFlip ? args.srcArea.right - 4 : args.srcArea.left,
 	                       args.vertFlip ? args.srcArea.bottom - 1 - yCtr : args.srcArea.top + yCtr);
-
-	// When not scaling, last row is handled separately
-	for (; Scale ? (yCtr < yCtrHeight) : (yCtr < yCtrHeight - 1); ++destY, ++yCtr, scaleYCtr += args.scaleY) {
+	for (; yCtr < yCtrHeight; ++destY, ++yCtr, scaleYCtr += args.scaleY) {
 		__m128i xCtrWidthSIMD = _mm_set1_epi32(xCtrWidth); // This is the width of the row
 
 		if (!Scale) {


Commit: 1c34e1c614570f8b8870527d7af701a2190ddcec
    https://github.com/scummvm/scummvm/commit/1c34e1c614570f8b8870527d7af701a2190ddcec
Author: Kaloyan Chehlarski (strahy at outlook.com)
Date: 2023-10-07T23:32:11+03:00

Commit Message:
AGS: Fix for surface blitting-related crashes

This is a second attempt at fixing the crashes that pop up
when attempting to blit past the bottom-right corner of a
surface. The previous commit caused a bunch of surfaces
to not blit their last row, and introduced more drawing
errors in scrolling scenes. Also, made sure the size check in
AVX2 code is 8 bytes everywhere, as this was also likely to
cause issues.

Changed paths:
    engines/ags/lib/allegro/surface_avx2.cpp
    engines/ags/lib/allegro/surface_neon.cpp
    engines/ags/lib/allegro/surface_sse2.cpp


diff --git a/engines/ags/lib/allegro/surface_avx2.cpp b/engines/ags/lib/allegro/surface_avx2.cpp
index a3b78394a38..360dda71861 100644
--- a/engines/ags/lib/allegro/surface_avx2.cpp
+++ b/engines/ags/lib/allegro/surface_avx2.cpp
@@ -487,7 +487,7 @@ static void drawInner4BppWithConv(BITMAP::DrawInnerArgs &args) {
 		xCtrBppStart = xCtrStart * SrcBytesPerPixel;
 		args.xStart = 0;
 	}
-	int destY = args.yStart, srcYCtr = 0, yCtr = 0, scaleYCtr = 0, yCtrHeight = (xCtrWidth % 4 == 0) ? args.dstRect.height() : (args.dstRect.height() - 1);
+	int destY = args.yStart, srcYCtr = 0, yCtr = 0, scaleYCtr = 0, yCtrHeight = (xCtrWidth % 8 == 0) ? args.dstRect.height() : (args.dstRect.height() - 1);
 	if (Scale) yCtrHeight = args.dstRect.height();
 	if (args.yStart < 0) {
 		yCtr = -args.yStart;
@@ -498,7 +498,7 @@ static void drawInner4BppWithConv(BITMAP::DrawInnerArgs &args) {
 		}
 	}
 	if (args.yStart + yCtrHeight > args.destArea.h) {
-		yCtrHeight = args.destArea.h - args.yStart;
+		yCtrHeight = (xCtrWidth % 8 == 0) ? args.destArea.h - args.yStart : args.destArea.h - args.yStart - 1;
 	}
 
 	byte *destP = (byte *)args.destArea.getBasePtr(0, destY);
@@ -573,7 +573,7 @@ static void drawInner4BppWithConv(BITMAP::DrawInnerArgs &args) {
 
 	// Get the last x values of the last row
 	int xCtr = xCtrStart, xCtrBpp = xCtrBppStart, destX = args.xStart;
-	// We have a picture that is a multiple of 4, so no extra pixels to draw
+	// We have a picture that is a multiple of 8, so no extra pixels to draw
 	if (xCtrWidth % 8 == 0) return;
 	// Drawing the last few not scaled pixels here.
 	// Same as the loop above but now we check if we are going to overflow,
diff --git a/engines/ags/lib/allegro/surface_neon.cpp b/engines/ags/lib/allegro/surface_neon.cpp
index 5fa0f273fad..6a370522ee2 100644
--- a/engines/ags/lib/allegro/surface_neon.cpp
+++ b/engines/ags/lib/allegro/surface_neon.cpp
@@ -495,7 +495,7 @@ static void drawInner4BppWithConv(BITMAP::DrawInnerArgs &args) {
 		}
 	}
 	if (args.yStart + yCtrHeight > args.destArea.h) {
-		yCtrHeight = args.destArea.h - args.yStart;
+		yCtrHeight = (xCtrWidth % 4 == 0) ? args.destArea.h - args.yStart : args.destArea.h - args.yStart - 1;
 	}
 
 	byte *destP = (byte *)args.destArea.getBasePtr(0, destY);
diff --git a/engines/ags/lib/allegro/surface_sse2.cpp b/engines/ags/lib/allegro/surface_sse2.cpp
index 80b161ddaa3..72b2f5bb5ea 100644
--- a/engines/ags/lib/allegro/surface_sse2.cpp
+++ b/engines/ags/lib/allegro/surface_sse2.cpp
@@ -515,7 +515,7 @@ static void drawInner4BppWithConv(BITMAP::DrawInnerArgs &args) {
 		}
 	}
 	if (args.yStart + yCtrHeight > args.destArea.h) {
-		yCtrHeight = args.destArea.h - args.yStart;
+		yCtrHeight = (xCtrWidth % 4 == 0) ? args.destArea.h - args.yStart : args.destArea.h - args.yStart - 1;
 	}
 
 	byte *destP = (byte *)args.destArea.getBasePtr(0, destY);




More information about the Scummvm-git-logs mailing list