Update to partial texture updates #5718
Conversation
mimimi085181
force-pushed
the
partial-updates-update
branch
from
110004c
to
46fb780
Compare
|
It seems i found another bug. I was relying on entry->memory_stride to actually contain the stride of the texture, which seems not to be the case. I need to look into this further. Edit: Fixed it, and it fixed yet another visual issue in spyro. Now the only difference between efb2ram and efb2tex is the displaced bloom effect on the ground which efb2ram has. I didn't see any difference beyond that, but i also haven't tested any water scenes. There might be people who enjoy this more than if it was actually fixed, since the amount of bloom is quite bigger on console and people don't seem to like bloom very much. |
I've always considered this to be a bad design decision. Instead of writing zeros (which was chosen for it's ascetic qualities of blackness), which sometimes results in clashing hashes, we should really be writing unique data for each efb copy. Maybe we run the properties of the efb copy (partially width/stride/height) through a lightweight encryption algorithm to generate unique but repeatable random bytes. |
| u32 overlap_range = std::min(entry->addr + entry->size_in_bytes, dstAddr + covered_range) - | ||
| std::max(entry->addr, dstAddr); | ||
| if (entry->memory_stride != dstStride || !copy_to_vram || | ||
| (!strided_efb_copy && entry->size_in_bytes == overlap_range) || |
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.
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.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
@phire: If we use unique data for efb copies, it will make half of this pr pointless. If 3 efb copies partly overwrite each other, then we get the correct data in memory when using efb2ram, by copying the texture data each time to memory, partly overwriting the exiting one. Now with the pr, it does the same, but in the graphics memory, and it's delayed until (or better if) the resulting texture is loaded. If the efb copies all had unique data, the hashes for the older efb copies would be different. To get to the same result again, you'd have to implement somewhat complicated logic. Now with this pr, if the data is still intact and the hash still matches, it means that there's an efb copy in that area, and partial updates will make sure that the texture inside that memory is recreated the current state. (well unless a game breaks it intentionally like spyro, writing a narrower or wider texture into the same memory, and then try to load it with the old width/stride) |
|
Maybe I'm misunderstanding something but my thought is, we shouldn't try to solve the bad design decision of writing zeroes in this pr. If we want to do that, it should be a separate PR. (also it might end up solving SilentHill snow on efb2tex!) |
|
No, the snow in Silent Hill requires to read back data from a texture, if i remember that correctly. The game seems to access the memory, where the efb copy is, with the cpu, in order to check how much snow there is supposed to be on screen. Oh, and writing all 0s has the advantage of creating clear efb copies. In case of spyro it means no bloom, in case of Silent Hill no snow. If it was all 0xff, i think there would be bloom on everything in spyro, and snow everywhere in Silent Hill. Now if it was inconsistent data, you'd get random things on screen. |
mimimi085181
force-pushed
the
partial-updates-update
branch
from
a2e0e0b
to
b273534
Compare
|
Hyperbloom ftw! ... On a more serious note, Spyro: A Hero's Tail now looks the same on efb2tex and efb2ram, so i think this pr plays well together with #5726. I want to take another look over the pr, to make sure i didn't miss any obvious issues. Edit: Cleaned up the pr a bit. I hope it can get merged, even if its method is a bit messy. A really clean solution would be much more complicated. |
mimimi085181
force-pushed
the
partial-updates-update
branch
from
b273534
to
dffbdf1
Compare
mimimi085181
changed the title
|
Hmm, the pr causes a problem on wind waker: My guess is that it's caused by calculating the stride of texture cache entries. I'll look into that. |
|
You need to pass Same with Otherwise you are passing the tlut format in there too, and that will mess things up. Same for the any other |
|
@mimimi085181 - just ask for #5689 to be merged 😄 |
|
@mimimi085181 - if you can rebase, you shouldn't see the issue with Wind Waker now? There were some nice changes in #5849. Please let us know if you think the code is ready to be merged.. |
In this case, comparing the hash is not enough to check, if two textures are identical.
This is supposed to get efb2tex to the same texture as efb2ram, by applying the related efb copies as updates after each other, in the order of their creation.
mimimi085181
force-pushed
the
partial-updates-update
branch
from
dffbdf1
to
cdbd986
Compare
|
Rebased on top of latest master and it seems to be working for wind waker, but i haven't tested actual gameplay or tested if it still works for spyro. |
|
Ok, i can't find any issues with the pr, so from my side, it's good for review and merge. |
|
Thank's for the great work. Spyro: A Hero's Tail works great here with efb2tex (tested in D3D). |
|
Does this flickering happen only with this pr, or does it happen in master too? It's important to determine if it's an issue with the pr. |
|
The flickering does occure on master too. The issue is not introduced by that pr. |
|
LGTM |
|
What's the state of this PR? |
Partial updates had a bug, that updated textures could be loaded by hash, ignoring the address(textures_by_hash). This could result in unexpected results, if a game had 2 textures where the base is identical, but they are stored in different memory locations and differ because of parts that are updated via partial updates. Since efb copies always write the same dummy data, both textures would have the same hash, and Dolphin would load one of them for both textures.
This pr also changes the logic about which textures are removed from the cache when they overlap new efb copies. Dolphin now keeps all textures that are partly overwritten and have the same stride as the efb copy. This is supposed to get efb2tex to the same texture as efb2ram, by applying the related efb copies as updates after each other, in the order of their creation.
The main purpose of this pr is to improve partial updates for hybrid xfb. Other than that, it does "fix" Spyro: A Hero's Tail, so efb2tex looks the same as efb2ram now. "fix", because the amount of bloom the game uses is high and it gets increased with higher IR, resulting at 3xIR in something i'd call hyperbloom... Anyways, this pr together with #5726 gets efb2tex to the same output as efb2ram, while it's actually correct emulation, as far as i can see.