Avoid the "Memory stride too small" assert #3480
Conversation
EFB2Tex still has no idea what to do with these weird textures so we simply disable EFB2Tex when one is encountered.
| @@ -1107,6 +1107,21 @@ void TextureCacheBase::CopyRenderTargetToTexture(u32 dstAddr, unsigned int dstFo | |||
| } | |||
| } | |||
|
|
|||
| if (dstStride < bytes_per_row) | |||
| { | |||
| // This kind of efb copy results in a scrambled image. | |||
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
FifoCI detected that this change impacts graphical rendering. Here are the behavior differences detected by the system: automated-fifoci-reporter |
|
I just checked this by Spider-Man (https://bugs.dolphin-emu.org/issues/9095) and the problem was completely fixed. The .zip file with the build I used is attached. |
|
@dolphin-emu-bot rebuild |
|
Review status: 0 of 1 files reviewed at latest revision, all discussions resolved. Source/Core/VideoCommon/TextureCacheBase.cpp, line 1112 [r1] (raw file): Comments from the review on Reviewable.io |
|
Reviewed 1 of 1 files at r1. Comments from the review on Reviewable.io |
|
Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful. Source/Core/VideoCommon/TextureCacheBase.cpp, line 1112 [r1] (raw file): Comments from the review on Reviewable.io |
|
Is there any reason that this is not merged to the main branch? If it helps, I have tested this fix with an Intel Core i3-6100 and Radeon R7 360 on Spider-Man, and it worked. |
|
I'm still waiting for an answer from @phire regarding hardware testing. |
|
I don't really want to do a hardware test for this. There is a good reason to have a copy with the row stride larger than the width of an EFB copy region, it lets you put an EFB copy inside a larger texture. NSMBWii uses this to update a single tile within a megatexture of all tiles with spinning coins. Sonic Riders: Zero Gravity uses this to pad the 640x480 efb copy to 1024x512, because it's engine appears to be incapable of using textures without power of two dimensions. On the other hand, an EFB copy with a row stride smaller than the width of the EFB copy region makes no sense. There is no such thing as packing a larger copy into a smaller texture. Conceivably it could represent a horizontal crop of the EFB copy to the row stride, discarding the excess data at the end of the row, but this would be a was of gates, as flipper already has explicit registers that allow cropping the size of the EFB copy (in both the horizontal and vertical directions). It's also possible fipper corrects the code submitted invalid values with a max(stride, width_in_bytes) but that too would be a waste of gates. So the most logical assumption is that flipper just accepts the invalid values and uses them resulting in some kind of invalid EFB copy. We don't know the exact implementation of the EFB copy DMA, but it would make a lot of sense to have two registers, one representing the address of the start of the row and then a column offset. For each 64bit write, add the two registers together to get the address then increment the column offset. When the column offset exceeds the width, reset it to zero and add the stride to the row address. This would result in the EFB copy DMA writing one row of the texture to memory, then "rewinding" and writing the next row of the texture over the 2nd part of the previous row. This PR replicates this behavior. (Actually, now that I think about it, this would result in a horizontal crop, assuming stride was aligned with the block size, but there would be a lot of extra data copied and junk at the end of the texture) One alternative implementation would result in the EFB copy DMA underflowing a subtraction scattering each row of the texture across the memory. All the games I looked at that did this kind of copy all appeared to be doing it in error. In many cases It happened as a cutscene or animation was finishing a fade out, they then don't appear to use the corrupted copy. I've put a lot of thought into this and think this is the correct solution. If this PR causes any of the effected games to have corrupted graphics, I'll be happy to debug the games more and/or do a Hardware test to confirm the behavior. |
|
I'm not fully happy about that, but I'll put "releasing something" in front of "perfect development workflow". Review status: Comments from the review on Reviewable.io |
all files reviewed at latest revision, all discussions resolved, all commit checks successful.Avoid the "Memory stride too small" assert
EFB2Tex still has no idea what to do with these weird textures so we simply disable EFB2Tex when one is encountered.