ducalex / retro-go

Retro emulation for the ODROID-GO and other ESP32 devices
GNU General Public License v2.0
489 stars 114 forks source link

Why update screen mode (FULL vs Partial) has been removed? #103

Closed Akrobate closed 3 months ago

Akrobate commented 3 months ago

Hello,

Today I've tested the last dev version. Everything is working fine.

I noticed that the update of screen mode has been removed from the menu. Is it intentionnal? Why this choice? I suppose partial is used by default?

thank you =)

ducalex commented 3 months ago

Hey! I wrote extensive messages for those changes in the following commits:

Let me know if you have questions!

Akrobate commented 3 months ago

Hi! Thank you for your answer. I haven't yet read the code, but your comments are clear. I understand the reasons of the change.

And keeping the option adding your new screen update as a possible choice doesn't make sense? Someting like "Partial v2"

ducalex commented 3 months ago

And keeping the option adding your new screen update as a possible choice doesn't make sense? Someting like "Partial v2"

I don't think it's worth having an option because the new code "just works". The hashing adds some overhead (on core 1, which is underutilised) but I've done extensive measuring and I haven't found any instance where partial code was slower than always doing full frame. I've also tested all combinations of scaling and filtering and the image is identical with or without partial enabled.

There is a measurement in the debug menu called "Blit time". For comparison a full update takes ~32-36ms to send to the screen. So, as long as that number is less than that, it means that partial frame is helping or at least not hindering.

So at this point an option wouldn't benefit the user and it would waste my time because on every change I make I'd have to test partial+filtering1, full+filtering1, partial+filtering2, full+filtering2, partial, full, partial+scaling+filtering, full+scaling+filtering, etc.

All that being said! if you truly don't want partial, I can add a #define in the target file to disable it at compile time :)

Akrobate commented 3 months ago

No no you're right it would be a waste of time. I close this issue, it was just by curiosity. Your new implementation of screen update works fine