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

COMMON: Use a reference to Graphics::PixelFormat in OSystem functions #3879

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ccawley2011
Copy link
Member

@ccawley2011 ccawley2011 commented May 14, 2022

No description provided.

Copy link
Member

@criezy criezy left a comment

I think passing the PixelFormat by reference might be a good idea and this simplifies some code in the backends.
However I think it may be clearer in a few cases to pass a null pixel format (with Graphics::PixelFormat()) rather than CLUT8 when the format is not actually used. I have added comments in a few such places.

@@ -421,7 +421,7 @@ void initGraphics(int width, int height) {
void initGraphics3d(int width, int height) {
g_system->beginGFXTransaction();
g_system->setGraphicsMode(0, OSystem::kGfxModeRender3d);
g_system->initSize(width, height);
g_system->initSize(width, height, Graphics::PixelFormat::createFormatCLUT8());
Copy link
Member

@criezy criezy May 16, 2022

Choose a reason for hiding this comment

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

It's a bit strange to request CLUT8 for 3D games. Since the 3D graphics manager ignore that parameters, maybe it would make more sense to pass a null pixel format with Graphics::PixelFormat().

@@ -356,7 +356,7 @@ static void setupGraphics(OSystem &system) {
system.setShader(ConfMan.get("shader").c_str());
system.setScaler(ConfMan.get("scaler").c_str(), ConfMan.getInt("scale_factor"));

system.initSize(320, 200);
system.initSize(320, 200, Graphics::PixelFormat::createFormatCLUT8());
Copy link
Member

@criezy criezy May 16, 2022

Choose a reason for hiding this comment

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

This gives the impression the launcher is using CLUT8, but that we forget to set the palette. Actually the format is not used since the overlay has its own pixel format, so maybe it would be clearer to use a null pixel format with Graphics::PixelFormat()?

} else {
g_system->setMouseCursor(nullptr, 0, 0, 0, 0, 0);
g_system->setMouseCursor(nullptr, 0, 0, 0, 0, 0, false, Graphics::PixelFormat::createFormatCLUT8());
Copy link
Member

@criezy criezy May 16, 2022

Choose a reason for hiding this comment

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

Maybe for that one (and the one below, a null pixel format (Graphics::PixelFormat()) would make more sense?

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