Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port for PlayStation Portable support #5869

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft

Port for PlayStation Portable support #5869

wants to merge 7 commits into from

Conversation

AJenbo
Copy link
Member

@AJenbo AJenbo commented Mar 10, 2023

Things are now building pretty smoothly.

image

Running in the PPSSPP emulator:
https://www.youtube.com/watch?v=UmAv7nrME-I

It does look like it's unable to reinitialize the graphics system so we need to disable related options.
Also we need to find a good way to downscale to 480x272 on SDL2. The PSP only supports textures of 512x512 so it won't be able to do it using a single 847x480 texture. One option would running at 960x544 and then 1/2 scale it in software.

Todo

  • Fix resolution
  • Fix running on hardware
  • Performance is about 7 FPS

This was referenced Mar 10, 2023
@@ -662,8 +662,10 @@ void multi_process_network_packets()
}
HandleAllPackets(playerId, (const byte *)(pkt + 1), dwMsgSize - sizeof(TPktHdr));
}
#ifndef PSP
if (SErrGetLastError() != STORM_ERROR_NO_MESSAGES_WAITING)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not exactly sure why the port is choking on this one, but we should probably investigate it a bit before just ignoring it like it did here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be fixed now and the workaround can be removed.

@@ -361,7 +361,11 @@ void ReinitializeTexture()
auto quality = StrCat(static_cast<int>(*sgOptions.Graphics.scaleQuality));
SDL_SetHint(SDL_HINT_RENDER_SCALE_QUALITY, quality.c_str());

#ifdef PSP
texture = SDLWrap::CreateTexture(renderer, SDL_PIXELFORMAT_ABGR1555, SDL_TEXTUREACCESS_STREAMING, gnScreenWidth, gnScreenHeight);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit raw for now until I get a chance to test what works best on hardware, should move it to a define before merging.

Copy link
Collaborator

@glebm glebm Nov 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We now have DEVILUTIONX_DISPLAY_TEXTURE_FORMAT in CMake that we can use for this:

set(DEVILUTIONX_DISPLAY_TEXTURE_FORMAT "SDL_PIXELFORMAT_ABGR1555")

Just this line needs to be updated as well to use DEVILUTIONX_DISPLAY_TEXTURE_FORMAT:

texture = SDLWrap::CreateTexture(renderer, SDL_PIXELFORMAT_RGB888, SDL_TEXTUREACCESS_STREAMING, gnScreenWidth, gnScreenHeight);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to check what pixel formats are supported at runtime using SDL_GetRendererInfo?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it might. psp supports 565, 5551 and 8888 afaik. I guess the problem is should we always select the lowest, highest or another? Or should we make this an option for the user in settings.
Often it really just makes sens to pick 1 for a given platform rather then having each user experiment with it.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the important thing is to ensure that the requested pixel format is supported natively - the PSP (and PS2) supports SDL_PIXELFORMAT_ABGR8888 but not SDL_PIXELFORMAT_XRGB888, so requesting the latter format requires any data copied to the texture to be converted in software before it can be uploaded to the GPU. There might also be bandwidth advantage to using a 16-bit format over a 32-bit one, but other than that the specifics shouldn't matter too much.

That said, since devilutionX seems to be designed around palette-based displays, libsdl-org/SDL#6192 may be of interest here. It still needs more work before it can be merged, though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, that isn't to much of a worry though, SDL_PIXELFORMAT_ABGR8888 is simply to slow on both platforms.

Hope you manage to finish the PR, could be useful if supported on most platforms.

@glebm
Copy link
Collaborator

glebm commented Mar 10, 2023

PSP only supports power-of-two textures, but can't we use 1024x1024?
If not, perhaps we can have 2 separate textures: one for the bottom panel and one of the rest of the screen.

@AJenbo
Copy link
Member Author

AJenbo commented Mar 10, 2023

Well the issue is width, not height :)
847x480 vs 512x512

https://github.com/libcg/gLib2D/blob/master/glib2d.h#L418

@stefanmielke
Copy link
Contributor

stefanmielke commented Mar 10, 2023

@AJenbo looks like it didn't upload the artifact on CircleCI. Let me know when I can grab that and I can do a quick test on hardware (also if you want to just send me a few builds to test different configurations, just let me know).

@glebm
Copy link
Collaborator

glebm commented Mar 10, 2023

Well the issue is width, not height :)

Oh, right. We can just blit the final surface to 2 side-by-side textures actually, since we render everything in software to an 8-bit surface first anyway.

@AJenbo
Copy link
Member Author

AJenbo commented Mar 10, 2023

@stefanmielke thanks, I corrected the artifact path.

@stefanmielke
Copy link
Contributor

stefanmielke commented Mar 10, 2023

@AJenbo got this error when running it: "The game could not be started (80020148)". Other homebrew run fine (also tested the Tyrian port).

Additionally, I copied the other files next to the game at "PSP\GAME\DevilutionX", not sure if it was expecting them somewhere else.

This is how it ended:
PSP\GAME\DevilutionX\devilutionx.mpq (grabbed from the Linux build)
PSP\GAME\DevilutionX\DIABDAT.MPQ
PSP\GAME\DevilutionX\EBOOT.PBP

To add more info, the game show the PSP logo at the start, then goes straight to a black screen, stays there for a couple seconds, and then crashes to the PSP homescreen with the message.

@diasurgical diasurgical deleted a comment from stefanmielke Mar 10, 2023
@diasurgical diasurgical deleted a comment from stefanmielke Mar 10, 2023
@diasurgical diasurgical deleted a comment from stefanmielke Mar 10, 2023
@diasurgical diasurgical deleted a comment from stefanmielke Mar 10, 2023
@AJenbo
Copy link
Member Author

AJenbo commented Nov 12, 2023

@stefanmielke we got it running on hardware in case you want to mess a bit more with it.

@AJenbo
Copy link
Member Author

AJenbo commented Nov 14, 2023

Everything is now working, only issue left is performance which is a good deal slower then what is ideal.

@@ -396,7 +400,11 @@ void RemoveFile(const char *path)
FILE *f = fopen(name.c_str(), "r+");
if (f != nullptr) {
fclose(f);
#ifdef PSP
sceIoRemove(name.c_str());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be nice to add remove to the SDK instead. Filed pspdev/newlib#7

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove is now in the SDK! pspdev/pspsdk#162

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants