Comment updates

GPU/Common/FramebufferManagerCommon.cpp

@@ -1664,6 +1664,9 @@ void FramebufferManagerCommon::FindTransferFramebuffer(VirtualFramebuffer *&buff
 			// Some games use mismatching bitdepths. But make sure the stride matches.
 			// If it doesn't, generally this means we detected the framebuffer with too large a height.
 			// Use bufferHeight in case of buffers that resize up and down often per frame (Valkyrie Profile.)
+
+			// TODO: Surely this first comparison should be <= ?
+			// Or does the exact match (byteOffset == 0) case get handled elsewhere?
 			bool match = memYOffset < yOffset && (int)memYOffset <= (int)vfb->bufferHeight - height;
 			if (match && vfb_byteStride != byteStride) {
 				// Grand Knights History copies with a mismatching stride but a full line at a time.
@@ -1902,7 +1905,7 @@ bool FramebufferManagerCommon::NotifyBlockTransferBefore(u32 dstBasePtr, int dst
 	int dstWidth = width;
 	int dstHeight = height;
 
-	// This looks at the compat flags BlockTransferAllowCreateFB*.
+	// These modify the X/Y/W/H parameters depending on the memory offset of the base pointers from the actual buffers.
 	FindTransferFramebuffer(srcBuffer, srcBasePtr, srcStride, srcX, srcY, srcWidth, srcHeight, bpp, false);
 	FindTransferFramebuffer(dstBuffer, dstBasePtr, dstStride, dstX, dstY, dstWidth, dstHeight, bpp, true);
 

GPU/Common/FramebufferManagerCommon.h

@@ -275,10 +275,15 @@ class FramebufferManagerCommon {
 	void UpdateFromMemory(u32 addr, int size, bool safe);
 	void ApplyClearToMemory(int x1, int y1, int x2, int y2, u32 clearColor);
 	bool PerformStencilUpload(u32 addr, int size, StencilUpload flags);
+
 	// Returns true if it's sure this is a direct FBO->FBO transfer and it has already handle it.
 	// In that case we hardly need to actually copy the bytes in VRAM, they will be wrong anyway (unless
 	// read framebuffers is on, in which case this should always return false).
+	// If this returns false, a memory copy will happen and NotifyBlockTransferAfter will be called.
 	bool NotifyBlockTransferBefore(u32 dstBasePtr, int dstStride, int dstX, int dstY, u32 srcBasePtr, int srcStride, int srcX, int srcY, int w, int h, int bpp, u32 skipDrawReason);
+
+	// This gets called after the memory copy, in case NotifyBlockTransferBefore returned false.
+	// Otherwise it doesn't get called.
 	void NotifyBlockTransferAfter(u32 dstBasePtr, int dstStride, int dstX, int dstY, u32 srcBasePtr, int srcStride, int srcX, int srcY, int w, int h, int bpp, u32 skipDrawReason);
 
 	bool BindFramebufferAsColorTexture(int stage, VirtualFramebuffer *framebuffer, int flags);

GPU/GPUCommon.cpp

@@ -2773,15 +2773,6 @@ void GPUCommon::SetDisplayFramebuffer(u32 framebuf, u32 stride, GEBufferFormat f
 }
 
 void GPUCommon::DoBlockTransfer(u32 skipDrawReason) {
-	// TODO: This is used a lot to copy data around between render targets and textures,
-	// and also to quickly load textures from RAM to VRAM. So we should do checks like the following:
-	//  * Does dstBasePtr point to an existing texture? If so maybe reload it immediately.
-	//
-	//  * Does srcBasePtr point to a render target, and dstBasePtr to a texture? If so
-	//    either copy between rt and texture or reassign the texture to point to the render target
-	//
-	// etc....
-
 	u32 srcBasePtr = gstate.getTransferSrcAddress();
 	u32 srcStride = gstate.getTransferSrcStride();