F3DEX3: support gSPMemset for graphics buffers #2895

aglab2 · 2025-01-10T15:40:06Z

This PR implements gSPMemset to handle graphics buffers. It is heavily HLE implementation that can be further improved but at least it fixes basically ZB issues.

aglab2 · 2025-01-10T15:49:51Z

CI seems to be broken, I do not believe this PR caused any issues on Linux build.

Jj0YzL5nvJ · 2025-01-12T17:37:46Z

libfreetype6-dev is missing as a dependency.

CMake Error at /usr/local/share/cmake-3.31/Modules/FindPackageHandleStandardArgs.cmake:233 (message):
  Could NOT find Freetype (missing: FREETYPE_LIBRARY)

It's not your fault =P

Edit:

Upon closer inspection, several dependencies are missing (libfreetype6-dev, libpng-dev, zlib1g-dev)... all this time the CI has been at the mercy of the ubuntu-latest image manager...

gonetz · 2025-01-25T08:04:59Z

src/gDP.cpp

@@ -792,6 +797,126 @@ void gDPSetScissor(u32 mode, s16 xh, s16 yh, s16 xl, s16 yl)
 #endif
 }

+template<typename T>
+class BackupRestore


We already have similar class: ValueKeeper in Types.h

gonetz · 2025-01-25T08:14:31Z

src/gDP.cpp

+	GraphicsDrawer& drawer = dwnd().getDrawer();
+	u32 fillColor = value | (value << 16);
+	const u32 depthImageStart = gDP.depthImageAddress;
+	const u32 depthImageEnd = gDP.depthImageAddress + 320 * 240 * 2;


320 * 240 * 2

Is depth buffer size hardcoded in the Ucode?
May be it should be screenWidth * screenHeight * 2 ?

gonetz · 2025-01-25T10:38:23Z

src/gDP.cpp

+	const u32 screenWidth = VI.width;
+	const u32 screenHeight = VI.height;


VI.width and VI.height are unreliable.
If gDPMemset is used for clearing existing color or depth buffer, the "addr" should point inside some buffer.
You may find that buffer by
FrameBuffer * pBuffer = frameBufferList().findBuffer(addr);
And then use pBuffer->m_width, pBuffer->m_height
You also may use
pBuffer->m_isDepthBuffer
instead of your check (depthImageStart <= addr && addr < depthImageEnd)

Yes, its most common use case is to clean ZB or FB. Here's an example to clear ZB in HackerOOT with respective f3dzex replacement https://github.com/HackerN64/HackerOoT/blob/e6fca950b79ec7561ac5cf52dcab14c84928a03f/src/code/z_rcp.c#L1569

I will try to implement the mentioned improvement.

I also do not believe it is possible to use the same approach for ZB because of a hack I had to explicitly save the buffer

frameBufferList().saveBuffer(gDP.depthImageAddress, (u16)G_IM_FMT_RGBA, (u16)G_IM_SIZ_16b, (u16)screenWidth, false);

Another issue I discovered trying to implement this is that no fb emulation mode breaks. I assume that is due to frameBufferList() being empty.

I have introduced a separate branch for fb emulation enabled. Implementation should be ready for review

gonetz · 2025-01-25T11:04:04Z

src/gDP.cpp

+	const u32 colorSize = gDP.colorImage.size < 3 ? 2 : 4;
+	const u32 colorImageStart = gDP.colorImage.address;
+	const u32 colorImageEnd = gDP.colorImage.address + screenWidth * screenHeight * colorSize;


Color image can be 8bit. So, it is better

Suggested change

const u32 colorSize = gDP.colorImage.size < 3 ? 2 : 4;

const u32 colorImageStart = gDP.colorImage.address;

const u32 colorImageEnd = gDP.colorImage.address + screenWidth * screenHeight * colorSize;

const u32 colorImageStart = gDP.colorImage.address;

const u32 colorImageEnd = gDP.colorImage.address + ((screenWidth * screenHeight) << gDP.colorImage.size >> 1);

gonetz · 2025-01-25T11:11:43Z

src/gDP.cpp

+		return;
+	}
+
+	// Either RGBA16 or RGBA32. This heavily assume "niceness" of developers to bind color buffer before memset...


If you use my suggestion with
FrameBuffer * pBuffer = frameBufferList().findBuffer(addr);
your code will not depend on that assumption, and developers can use any buffer, which was bound previously with gDPSetColorImage (or with gDPSetDepthImage).
If pBuffer is null, the addr doesn't point to any buffer, so you fallback to just memory set.

gonetz · 2025-01-25T11:17:41Z

src/gDP.cpp

+			// Pretend that we are drawing the rectangle over ZB with fill mode
+			BackupRestore backupColorImageAddress(gDP.colorImage.address);
+			BackupRestore backupWidth(gDP.colorImage.width);
+			const unsigned int backupDepthSource = gDP.otherMode.depthSource;
+			const unsigned int backupCycleType = gDP.otherMode.cycleType;
+
+			gDP.colorImage.address = gDP.depthImageAddress;
+			gDP.colorImage.width = screenWidth;
+			gDP.otherMode.cycleType = G_CYC_FILL;
+
+			drawer.drawRect(ulx, uly, lrx, lry);
+			frameBufferList().setBufferChanged(f32(lry));
+
+			gDP.otherMode.depthSource = backupDepthSource;
+			gDP.otherMode.cycleType = backupCycleType;


Suggested change

// Pretend that we are drawing the rectangle over ZB with fill mode

BackupRestore backupColorImageAddress(gDP.colorImage.address);

BackupRestore backupWidth(gDP.colorImage.width);

const unsigned int backupDepthSource = gDP.otherMode.depthSource;

const unsigned int backupCycleType = gDP.otherMode.cycleType;

gDP.colorImage.address = gDP.depthImageAddress;

gDP.colorImage.width = screenWidth;

gDP.otherMode.cycleType = G_CYC_FILL;

drawer.drawRect(ulx, uly, lrx, lry);

frameBufferList().setBufferChanged(f32(lry));

gDP.otherMode.depthSource = backupDepthSource;

gDP.otherMode.cycleType = backupCycleType;

// Pretend that we are drawing the rectangle over ZB with fill mode

ValueKeeper<u32> backupColorImageAddress(gDP.colorImage.address, gDP.depthImageAddress);

ValueKeeper<u32> backupWidth(gDP.colorImage.width, screenWidth);

ValueKeeper<u32> backupDepthSource(gDP.otherMode.depthSource, gDP.otherMode.depthSource);

ValueKeeper<u32> backupCycleType(gDP.otherMode.cycleType, G_CYC_FILL);

drawer.drawRect(ulx, uly, lrx, lry);

frameBufferList().setBufferChanged(f32(lry));

cycleType and depthSource are not allowed to be casted to non const references because they are bit fields so this code will not work.

gonetz · 2025-01-25T11:19:21Z

src/gDP.cpp

+		{
+			BackupRestore backupFillColor(gDP.fillColor.color);
+			const unsigned int backupDepthSource = gDP.otherMode.depthSource;
+			const unsigned int backupCycleType = gDP.otherMode.cycleType;
+
+			gDP.fillColor.color = fillColor;
+			gDP.otherMode.cycleType = G_CYC_FILL;
+
+			drawer.drawRect(ulx, uly, lrx, lry);
+			if (auto cur = frameBufferList().getCurrent())
+				cur->setBufferClearParams(gDP.fillColor.color, ulx, uly, lrx, lry);
+
+			frameBufferList().setBufferChanged(f32(lry));
+
+			gDP.otherMode.depthSource = backupDepthSource;
+			gDP.otherMode.cycleType = backupCycleType;
+		}


Suggested change

{

BackupRestore backupFillColor(gDP.fillColor.color);

const unsigned int backupDepthSource = gDP.otherMode.depthSource;

const unsigned int backupCycleType = gDP.otherMode.cycleType;

gDP.fillColor.color = fillColor;

gDP.otherMode.cycleType = G_CYC_FILL;

drawer.drawRect(ulx, uly, lrx, lry);

if (auto cur = frameBufferList().getCurrent())

cur->setBufferClearParams(gDP.fillColor.color, ulx, uly, lrx, lry);

frameBufferList().setBufferChanged(f32(lry));

gDP.otherMode.depthSource = backupDepthSource;

gDP.otherMode.cycleType = backupCycleType;

}

{

ValueKeeper<u32> backupFillColor(gDP.fillColor.color, fillColor);

ValueKeeper<u32> backupDepthSource(gDP.otherMode.depthSource, gDP.otherMode.depthSource);

ValueKeeper<u32> backupCycleType(gDP.otherMode.cycleType, G_CYC_FILL);

drawer.drawRect(ulx, uly, lrx, lry);

if (auto cur = frameBufferList().getCurrent())

cur->setBufferClearParams(gDP.fillColor.color, ulx, uly, lrx, lry);

frameBufferList().setBufferChanged(f32(lry));

}

Same here, this code will not work

gonetz · 2025-01-25T11:22:25Z

src/gDP.cpp

+			for (int i = 0; i < length / 4; i++) {
+				*(u32*)(RDRAM + addr + i * 4) = fillColor;
+			}


Suggested change

for (int i = 0; i < length / 4; i++) {

*(u32*)(RDRAM + addr + i * 4) = fillColor;

}

for (int i = 0; i < length; i+=4) {

*(u32*)(RDRAM + addr + i ) = fillColor;

}

It should be faster. Or

u32* pDest = interpret_cast<u32*>(RDRAM + addr); length /= 4; // or length >>= 2; for (int i = 0; i < length; i++) { pDest[i] = fillColor;

The same below.

aglab2 · 2025-01-26T03:56:33Z

Implementation is ready for another round of review

F3DEX3: support gSPMemset for graphics buffers

78486cb

Jj0YzL5nvJ mentioned this pull request Jan 12, 2025

CI: Add missing dependencies and other minor changes #2897

Merged

gonetz requested changes Jan 25, 2025

View reviewed changes

aglab2 added 2 commits January 26, 2025 11:05

F3DEX3: fixed nitpicks in gSPMemset

af51451

F3DEX3: support memset for color buffers when fb emulation is enabled

762d038

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

F3DEX3: support gSPMemset for graphics buffers #2895

F3DEX3: support gSPMemset for graphics buffers #2895

aglab2 commented Jan 10, 2025 •

edited

Loading

aglab2 commented Jan 10, 2025

Jj0YzL5nvJ commented Jan 12, 2025 •

edited

Loading

gonetz Jan 25, 2025

gonetz Jan 25, 2025

gonetz Jan 25, 2025

aglab2 Jan 26, 2025

aglab2 Jan 26, 2025

aglab2 Jan 26, 2025

aglab2 Jan 26, 2025

gonetz Jan 25, 2025

gonetz Jan 25, 2025

gonetz Jan 25, 2025

aglab2 Jan 26, 2025

gonetz Jan 25, 2025

aglab2 Jan 26, 2025

gonetz Jan 25, 2025

aglab2 commented Jan 26, 2025

		const u32 screenWidth = VI.width;
		const u32 screenHeight = VI.height;

F3DEX3: support gSPMemset for graphics buffers #2895

Are you sure you want to change the base?

F3DEX3: support gSPMemset for graphics buffers #2895

Conversation

aglab2 commented Jan 10, 2025 • edited Loading

aglab2 commented Jan 10, 2025

Jj0YzL5nvJ commented Jan 12, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aglab2 commented Jan 26, 2025

aglab2 commented Jan 10, 2025 •

edited

Loading

Jj0YzL5nvJ commented Jan 12, 2025 •

edited

Loading