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

OPENGL: Support RGBA8888 swapped textures when using OpenGL ES #1766

Merged
merged 1 commit into from Aug 4, 2019

Conversation

@ccawley2011
Copy link
Member

commented Jul 23, 2019

OpenGL ES only supports RGBA8888 on big endian systems, which means that any games requiring a 32-bit colour mode will fail to start when using OpenGL ES on little endian systems.

@bluegr

This comment has been minimized.

Copy link
Member

commented Jul 24, 2019

I believe that we don’t support any games with 32-bit color, which might explain why this went unnoticed up to now.
The changes are quite straightforward, and can be merged. Any further feedback?

#else
} else if (isGLESContext() && format == Graphics::PixelFormat(4, 8, 8, 8, 8, 0, 8, 16, 24)) { // ABGR8888
#endif
return new TextureRGBA8888Swap();

This comment has been minimized.

Copy link
@criezy

criezy Jul 24, 2019

Member

Do we need to also systematically use the TextureRGBA8888Swap if either USE_FORCED_GLES or USE_FORCED_GLES2 is defined (i.e. have the same conditions that what we had before when checking if those pixel formats were supported)?

This comment has been minimized.

Copy link
@ccawley2011

ccawley2011 Jul 30, 2019

Author Member

This code is already disabled when built with USE_FORCED_GL, so I don't think any additional checks are necessary.

@TheQL

This comment has been minimized.

Copy link

commented Jul 30, 2019

I am getting "Could not initialize color format." when running e.g. Blade Runner or Gabriel Knight on iOS. I included this PR and created a new build, error is still there. Not sure if this should have fixed it, just FYI.

@criezy

This comment has been minimized.

Copy link
Member

commented Jul 30, 2019

@TheQL: I just checked the iOS code, and indeed it has its own custom OpenGL ES code and is not using the common code being modified in this pull request. So unfortunately it will indeed not be fixed by the change here.

@ccawley2011

This comment has been minimized.

Copy link
Member Author

commented Jul 30, 2019

I believe that we don’t support any games with 32-bit color, which might explain why this went unnoticed up to now.

We do have several engines that require 32-bit colour modes, including fullpipe, gnap, groovie2, sci (when playing HQ videos), sludge, sword25 and wintermute. I only tested the last two, though.

@bluegr

This comment has been minimized.

Copy link
Member

commented Jul 30, 2019

I believe that we don’t support any games with 32-bit color, which might explain why this went unnoticed up to now.

We do have several engines that require 32-bit colour modes, including fullpipe, gnap, groovie2, sci (when playing HQ videos), sludge, sword25 and wintermute. I only tested the last two, though.

I thought that these required 24bpp graphics modes, not 32bpp graphics modes, which is why I made the original comment. Anyway, it doesn't matter

@TheQL

This comment has been minimized.

Copy link

commented Jul 31, 2019

@TheQL: I just checked the iOS code, and indeed it has its own custom OpenGL ES code and is not using the common code being modified in this pull request. So unfortunately it will indeed not be fixed by the change here.

Should I post the iOS issue with Blade Runner and Gabriel Knight and whatever other games might be affected as a bug?

@bluegr

This comment has been minimized.

Copy link
Member

commented Jul 31, 2019

@TheQL: I just checked the iOS code, and indeed it has its own custom OpenGL ES code and is not using the common code being modified in this pull request. So unfortunately it will indeed not be fixed by the change here.

Should I post the iOS issue with Blade Runner and Gabriel Knight and whatever other games might be affected as a bug?

Yes, please do

@criezy

criezy approved these changes Aug 3, 2019

Copy link
Member

left a comment

This seems good to me. If there is no objections in the next couple of days I propose to merge this on Sunday.

@bluegr

This comment has been minimized.

Copy link
Member

commented Aug 4, 2019

This seems good to me. If there is no objections in the next couple of days I propose to merge this on Sunday.

Agree

@criezy criezy merged commit d765440 into scummvm:master Aug 4, 2019

2 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ccawley2011 ccawley2011 deleted the ccawley2011:opengl-rgba8888 branch Aug 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.