Fix copy filter clamping (again)

[Pokechu22]

Fixes https://bugs.dolphin-emu.org/issues/12813, which regressed in #10204. The clamping logic was improved in #10222, but was off by half a pixel, which causes issues for the previous line when the copy filter is enabled.

Here's the EFB it's working with (as with before, the colors are weird because the EFB is RGBA6 for normal game logic, but RGB8 for the broken effect):

The EFB copy is for 152 by 85 at (336, 224). The broken version produces this (upscaled 4x with nearest neighbor):

Now, it produces this:

The comment about clamping behavior is from the software renderer; I myself haven't performed the tests it is referring to:

dolphin/Source/Core/VideoBackends/Software/EfbInterface.cpp

Lines 589 to 597 in b237c74

	// Clamping behavior
	// NOTE: when the clamp bits aren't set, the hardware will happily read beyond the EFB,
	// which returns random garbage from the empty bus (confirmed by hardware tests).
	//
	// In our implementation, the garbage just so happens to be the top or bottom row.
	// Statistically, that could happen.
	const u16 y_prev = static_cast<u16>(std::max(clamp_top ? source_rect.top : 0, y - 1));
	const u16 y_next = static_cast<u16>(
	std::min<int>((clamp_bottom ? source_rect.bottom : EFB_HEIGHT) - 1, y + 1));

I also fixed an off-by-one when clamp is not enabled, mentioned in #10222 (comment) (at least, I think it's fixed; I'm not 100% confident in this).

FifoCI probably won't show any interesting differences from this change since the copy filter is disabled by default, and this issue only exists when the copy filter is enabled.

[dolphin-emu-bot]

FifoCI detected that this change impacts graphical rendering. Here are the behavior differences detected by the system:

• ab11-homebrew on ogl-lin-mesa: diff
• last-story-shadows on ogl-lin-mesa: diff
• lego-star-wars-crane-shadow on ogl-lin-mesa: diff
• mario-golf-oob on ogl-lin-mesa: diff
• mkdd-efb on ogl-lin-mesa: diff
• mp4-vertexcache on ogl-lin-mesa: diff
• sw3-dt on ogl-lin-mesa: diff
• ab11-homebrew on ogl-lin-radeon: diff
• last-story-shadows on ogl-lin-radeon: diff
• lego-star-wars-crane-shadow on ogl-lin-radeon: diff
• mario-golf-oob on ogl-lin-radeon: diff
• mkdd-efb on ogl-lin-radeon: diff
• sw3-dt on ogl-lin-radeon: diff
• ab11-homebrew on uberogl-lin-radeon: diff
• last-story-shadows on uberogl-lin-radeon: diff
• lego-star-wars-crane-shadow on uberogl-lin-radeon: diff
• mario-golf-oob on uberogl-lin-radeon: diff
• mkdd-efb on uberogl-lin-radeon: diff
• sw3-dt on uberogl-lin-radeon: diff

_{^{automated-fifoci-reporter}}

[Pokechu22]

Looking at the fifoci differences, they all seem to concern the top/bottom rows, but I can't reproduce the differences locally (I did have the copy filter off like with fifoci) and I can't really tell which form is better. There may be some form of GPU dependence here (similar to what Manual Texture Sampling fixes).

[Pokechu22]

Oh, one other thing: this change hasn't been made in the software renderer because the software renderer doesn't implement the copy filter with EFB copies (the existing EFB copy logic in the software renderer was too tricky to modify). When that's implemented, it probably will be with integer coordinates, so it should be easy to avoid this mistake there.

[iwubcode]

FifoCI probably won't show any interesting differences from this change since the copy filter is disabled by default, and this issue only exists when the copy filter is enabled.

Well there was a slight change in behavior from what I can tell. Instead of 0 (top) and 1 (bottom) when the filter is off. We now do 0.5 / height (top) and (height - 0.5) / height (bottom). So that could be the the differences you see? I'm assuming that was intentional.

(I'm having a hard time seeing those changes personally even with 'bright' settings on).

Code changes look ok to me. I pulled it down and will do a bit of testing before approving.

[philou-felin]

Hi, @iwubcode ! I'm the one who opened the bug report on the platform. Let me know if you have questions. As I keep writing, I'm just a "dumb" end-user. 😅

The "unwanted" behaviour that I reported becomes less visible if the user increases the internal resolution, but I play with native resolution.

[Pokechu22]

Here's an example of the differences:

Old:

New:

Comparison:

[iwubcode]

@Pokechu22 - can you confirm if you mean to impact non-clamp behavior?

Also what is that image you showed? I didn't see that in any of the fifo logs but maybe I missed it.

[Pokechu22]

I did intentionally impact non-clamp behavior, since it seemed like that wasn't correct (see #10222 (comment)), but I don't think that's what's causing the differences.

The image is from lego-star-wars-crane-shadow (it's the very top of it, magnified, and even with that it's pretty hard to see the difference). That fifolog only has 2 EFB copies, and both of them have clamping enabled. All of the differences are only on the top or bottom row of pixels, and since fifoci resizes images when displaying them so that all of them are visible, you might have trouble seeing the difference.

[iwubcode]

Thanks I see it now (jumping back and forth between frames in browser). It'd be neat if we added a "animation" option that toggled between the two at some speed. But maybe that'd be distracting with multiple frames.

But that helps me know what to look for.