Closed jdgleaver closed 3 years ago
This pull request fixes 2 alerts when merging 62a06c10b23c3ff3b072866d82ca6ecda133c0aa into d9456ae1c206201fdd8375986dde63735f6e1d3d - view on LGTM.com
fixed alerts:
Funny thing no one even noticed (or tried, tested, used) 24bpp was broken on *nix platforms. And seems likely that nearly all Windows people don't use it either?
If it was removed from the core completely, would anyone even complain? A bonus would be the framebuffer could be forced down to 16bpp and save precious memory on microcontroller platforms. And maybe some paths could be cut down further (don't compile 555 if 565 supported) but just micro-ideas.
I also asked someone about the PS Mini and they mentioned that it's PS1OGL textures should be aligned. Which isn't a problem actually because core creates a square box of RETRO_LYNX_WIDTH (160) x RETRO_LYNX_WIDTH (160).
Nice PR updates all-around!
Thanks, I'm glad you approve :)
Funny thing no one even noticed (or tried, tested, used) 24bpp was broken on *nix platforms. And seems likely that nearly all Windows people don't use it either?
You'd be surprised how many people never even look at core options... (or any RA options...)
If it was removed from the core completely, would anyone even complain? A bonus would be the framebuffer could be forced down to 16bpp and save precious memory on microcontroller platforms. And maybe some paths could be cut down further (don't compile 555 if 565 supported) but just micro-ideas.
I really don't think anyone would complain. However, we could just leave the option there, and allocate the framebuffer on the heap (with the proper size, right after we first check the colour depth setting) - that would save the wasted memory. I'll put this on the TODO list!
I also asked someone about the PS Mini and they mentioned that it's PS1OGL textures should be aligned. Which isn't a problem actually because core creates a square box of RETRO_LYNX_WIDTH (160) x RETRO_LYNX_WIDTH (160).
Nice, that's one less platform to worry about :)
However, we could just leave the option there, and allocate the framebuffer on the heap (with the proper size, right after we first check the colour depth setting) - that would save the wasted memory. I'll put this on the TODO list!
That's brilliant! 👍
(in truth, there is little point enabling it anywhere - all it does is reduce performance...)
I benchmarked the two depths and I got a 50% drop for a miniscule benefit. For a non-engineer, that actually kinda shocked me. :whoa: There should be a disclaimer on the website readme for cores. 🤣
Also was looking at mikie.cpp and was amazed at all the copy-paste repeating. Wonder if it really was faster to do it that way decades ago compared to structs or something similar. I think it's hard to follow as-is.
Yeah - on my RG350M OpenDingux device, 16bit mode runs full speed with a 3x upscaling video filter. In 24bit mode, it's ~40fps at native resolution. An incredible difference!
And yes, mikie.cpp
is indeed... interesting. I agree that it's difficult to follow, but I guess it probably was faster, back when compilers weren't so smart :)
New question. Noticed beetle-wswan
repo now has both hardware and software rotation support, which got me wondering if same could be done in Handy. Could cleanup the mikie.cpp
file some bit. Curiously both cores have that color depth toggle problem.
We could indeed add hardware rotation support to Handy - but I think we'd still need the old 'software' rotation code paths in mikie.cpp
. Performing rotation as a post-processing effect (like I did for beetle-wswan) is pretty fast, but still slower than Handy's inbuilt version - and I'd hate to reduce the performance of the core!
...and I see that negativeExponent has just made a PR to prevent runtime toggling of colour depth in beetle-wswan (although we could probably clean that up a little further, like I did here...)
This PR makes the following changes to the core:
The OpenDingux-specific padding hack added in #82 has been removed, since this is now handled by the frontend
A new
FRONTEND_SUPPORTS_XRGB8888
compiler flag has been added. The 24 bit colour depth mode and core option are only available when this is set. At present, it is only enabled for Linux and Windows builds (in truth, there is little point enabling it anywhere - all it does is reduce performance...)The colour depth core option now requires a core restart, since it is reported that toggling colour depth at runtime causes system instability on some platforms/gfx drivers
At present, the core makes use of poorly defined data types which have different widths on different platform. This, for example, completely breaks the 24 bit colour depth mode on Linux, causing buffer overflows/stack corruption (!). This PR fixes the issue.
Core options have been updated to v1.3