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

SDL2 fails to display image files with no alpha channel #239

Closed
aquawicket opened this issue Sep 28, 2021 · 12 comments
Closed

SDL2 fails to display image files with no alpha channel #239

aquawicket opened this issue Sep 28, 2021 · 12 comments
Labels
backends Platforms and renderers samples
Milestone

Comments

@aquawicket
Copy link
Contributor

Started messing around with RML again and I noticed something changed in SDL_Image that breaks Image files without alpha data. It's just a small fix, but worth a mention to anyone using SDL.

RenderInterfaceSDL2.cpp
--------------------------
82  glTexEnvf(GL_TEXTURE_ENV, GL_TEXTURE_ENV_MODE, GL_MODULATE);
83   glEnable(GL_BLEND);
84   glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA);
85   glDrawElements(GL_TRIANGLES, num_indices, GL_UNSIGNED_INT, indices);

For some reason, calling glBlend states on a graphic with no alpha channel results in "white out" textures. I tested it out on the RmlUi sdl2 example and found the same results. BMP and JPG files will no work with the original implementation.. I'm not an OpenGL guy, but I found that, if you check the texture blend mode, turning off glBlend for such images will allow them to display again..

82  glTexEnvf(GL_TEXTURE_ENV, GL_TEXTURE_ENV_MODE, GL_MODULATE);
+     if(sdl_texture){
+           SDL_BlendMode bm;
+           SDL_GetTextureBlendMode(sdl_texture, &bm);
+            if(bm == SDL_BLENDMODE_BLEND){
+                  glEnable(GL_BLEND);
+                  glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA);
+            }
+     }
85   glDrawElements(GL_TRIANGLES, num_indices, GL_UNSIGNED_INT, indices);

The issue lands on RML's side for this, because the texture is loaded just fine, yet, the RML example fails to show the image.
I can do a pull request if needed, but i'd like to do some more tests.

@aquawicket
Copy link
Contributor Author

For anyone to test, this is is one of the image files in question..
test
.

@mikke89
Copy link
Owner

mikke89 commented Sep 28, 2021

Hi, and thanks for reporting this. I do see the same issue when testing this on my end.

Not sure what SDL is doing here, normally OpenGL attaches 1 to the alpha channel, so it should work perfectly fine with blending. I did also test your changes but it didn't help on my end, I only get white even with blending completely disabled. So I think there is something else going on. I'm not too familiar with the SDL API, I would try to bring out a graphics debugger at this point.

@aquawicket
Copy link
Contributor Author

I have a few images still showing white boxes as well. The blend disable only worked on a couple images for me, so it's a hack. From what you said, "didn't help on my end", I'm guessing this is more of an OpenGL issue, or the states SDL may have changed. I'm at latest SDL version 2.0.16. I'll roll back to an earlier version see if I can find the suspect.

@mikke89
Copy link
Owner

mikke89 commented Sep 29, 2021

If you do figure this one out, a PR would be very welcome :)

@aquawicket
Copy link
Contributor Author

aquawicket commented Oct 28, 2021

Can you send a couple image files that are not displaying on your end. I recompiled my code base with the latest SDL2 and SDLImage, now all of my images are working with just that conditional blend fix.

@mikke89
Copy link
Owner

mikke89 commented Oct 28, 2021

I'm on a new machine now, and I can't reproduce it here. I seem to be getting correct results with your proposed changes now. Soo, I don't know. Did you try a graphics debugger to look at the states without the fix?

Only semi-related, but I also just noticed that I'm getting crashes whenever there are more than one image loaded. Eg. try replacing assets/demo.rml with the following:

<rml>
	<head>
		<title>Demo</title>
		<!--link type="text/template" href="window.rml" /-->
		<style>
			body
			{
				width: 300dp;
				height: 225dp;
				margin: auto;
				
				font-family: LatoLatin;
				font-weight: normal;
				font-style: normal;
				font-size: 15dp;
				color: white;
			}
			
			div#title_bar div#icon
			{
				display: none;
			}
			
			div#content
			{
				text-align: left;
			}
		</style>
	</head>
	<body>
		This is a sample.
		<img src="high_scores_alien_1.tga"/>
		<img src="high_scores_alien_2.tga"/>
	</body>
</rml>

@aquawicket
Copy link
Contributor Author

Yea.. i'll check the graphics debug today actually and see if I can reproduce it.

@RWD-Biggs
Copy link

I've found that modifying RenderInterfaceSDL2::LoadTexture() to convert the image surface before a texture is created from it solves the issue for me. It's not ideal, but it works and shouldn't be too expensive as it only happens once per texture.

...

	SDL_Surface* Surface = IMG_LoadTyped_RW(SDL_RWFromMem(Buffer, int(BufferLen)), 1, FileExt.c_str());

	if (!Surface)
	{
		return false;
	}

	// Fix for 24-bit images (ie. no alpha channel) not rendering
	SDL_Surface* ConvertedSurface = SDL_ConvertSurfaceFormat(Surface, SDL_PixelFormatEnum::SDL_PIXELFORMAT_RGBA32, 0);

	if (!ConvertedSurface)
	{
		return false;
	}

	SDL_Texture* Texture = SDL_CreateTextureFromSurface(Renderer, ConvertedSurface);

...

        SDL_FreeSurface(ConvertedSurface);
	SDL_FreeSurface(Surface);

@aquawicket
Copy link
Contributor Author

aquawicket commented Nov 17, 2021

Nice. I'll toss that in and throw some images at it on my end. Besides some animated gif frames, the small fix I had earlier seems to be doing the trick for me at the moment. I used GLIntercept to debug the OpenGL context calls, but I didn't notice any errors related to the blend states on the textures. I think GL is just accepting the processing as valid or intentional. Tested on windows7 x86/x64, windows10 x86/x64, MacOs 11.5.2 x64, Linux Lxle x32 & x64, Raspbery Pi 4 arm32. GLIntercept is a bit dated so I'm trying to get RenderDoc up and debugging, but It always complains about lack of a 3.2+ context.
"OpenGL. Context not created via CreateCont Only OpenGL 3.2+ contexts are supported"
This led me to another discovery that, SDL_CreateRenderer() hard codes the GL Context to 2.1. So calling SDL_CreateWindow, SDL_GL_CreateContext and version setting attributes will always be reset back to GL_2.1 once SDL_CreateRenderer() is called. I ran a bunch of configurations, And sure enough, no matter what version flags I set, this was the case.

Original Code: OpenGL context will always be reset to version 2.1 by call to SDL_CreateRenderer()

SDL_Init(SDL_INIT_VIDEO);
	SDL_Window* screen = SDL_CreateWindow("LibRmlUi SDL2 test", 20, 20, window_width, window_height, SDL_WINDOW_OPENGL | SDL_WINDOW_RESIZABLE);
	SDL_GLContext glcontext = SDL_GL_CreateContext(screen);
	int oglIdx = -1;
	int nRD = SDL_GetNumRenderDrivers();
	for (int i = 0; i < nRD; i++)
	{
		SDL_RendererInfo info;
		if (!SDL_GetRenderDriverInfo(i, &info))
		{
			if (!strcmp(info.name, "opengl"))
			{
				oglIdx = i;
			}
		}
	}
        //NOTE: Setting GL Context version will have NO effect anywhere in this code. 
	SDL_Renderer* renderer = SDL_CreateRenderer(screen, oglIdx, SDL_RENDERER_ACCELERATED | SDL_RENDERER_PRESENTVSYNC);

	GLenum err = glewInit();

	if (err != GLEW_OK)
		fprintf(stderr, "GLEW ERROR: %s\n", glewGetErrorString(err));
.
         //OpenGL context version reports 2.1, regardless of any flags

The fix for this was to actually, create the Window and Renderer first GL is now 2.1, Then, set the version attributes after. And finally, create the context last. GL is now closest valid requested version . This works, and now SDL picks the correct context common denominator. On 4 machines tested here, all windows, the result is the same. I still have more boxes to test. Here is the context creation order I have now.

New Code: OpenGL context version can be set After SDL_CreateRenderer() with SDL_GL_CreateContext() last

SDL_Init(SDL_INIT_VIDEO);
	SDL_Window* screen = SDL_CreateWindow("LibRmlUi SDL2 test", 20, 20, window_width, window_height, SDL_WINDOW_OPENGL | SDL_WINDOW_RESIZABLE);

	int oglIdx = -1;
	int nRD = SDL_GetNumRenderDrivers();
	for (int i = 0; i < nRD; i++)
	{
		SDL_RendererInfo info;
		if (!SDL_GetRenderDriverInfo(i, &info))
		{
			if (!strcmp(info.name, "opengl"))
			{
				oglIdx = i;
			}
		}
	}
	SDL_Renderer* renderer = SDL_CreateRenderer(screen, oglIdx, SDL_RENDERER_ACCELERATED | SDL_RENDERER_PRESENTVSYNC);   //<-- rolls back GL context version on it's own
	
        ////Only now, after the render is created, can we effectively alter the context version
	SDL_GL_SetAttribute(SDL_GL_CONTEXT_PROFILE_MASK, SDL_GL_CONTEXT_PROFILE_COMPATIBILITY);
	SDL_GL_SetAttribute(SDL_GL_CONTEXT_MAJOR_VERSION, 4);
	SDL_GL_SetAttribute(SDL_GL_CONTEXT_MINOR_VERSION, 6);

	SDL_DisplayMode current;
	SDL_GetCurrentDisplayMode(0, &current);
	SDL_GLContext glcontext = SDL_GL_CreateContext(screen);   //<-- Set Last 
	SDL_GL_MakeCurrent(screen, glcontext);

	GLenum err = glewInit();
	if (err != GLEW_OK)
		fprintf(stderr, "GLEW ERROR: %s\n", glewGetErrorString(err));

        //gl reports version 4.6

This can have implications on available GL extensions, shader version and which context pipeline functions SDL is using for GL. When I use a proper context version, I no longer need glew extensions to turn off shaders, as SDL has then built in.

RmlRenderer.cpp

#if !defined(IOS) && !defined(ANDROID)
static PFNGLUSEPROGRAMOBJECTARBPROC glUseProgramObjectARB; //available with SDL2
#endif
.
.
// Called by Rml when it wants to render geometry that it does not wish to optimise.
void RmlSDL2Renderer::RenderGeometry(Rml::Vertex* vertices, int num_vertices, int* indices, int num_indices, const Rml::TextureHandle texture, const Rml::Vector2f& translation) {
#if !defined(IOS) && !defined(ANDROID)
    // DISABLE SDL Shaders
	glUseProgramObjectARB = (PFNGLUSEPROGRAMOBJECTARBPROC) SDL_GL_GetProcAddress("glUseProgramObjectARB");
	glUseProgramObjectARB(0);  
#endif
.

I guess the real motivation here is to rework the SDL example. I've taken a look at 1bysl's PR #252 using SDL_RenderGeometry() and it looks promising. When more time is available, I'll be able to work up some more solid code and test across all OS's. Any snippets of code or info to help the process is more than welcome :)

@mikke89
Copy link
Owner

mikke89 commented Nov 17, 2021

Wow, quite the journey! Thanks for the deep dive into this.

Looking forward to the reworked SDL sample, having a solid SDL backend would mean a lot.

Here are some thoughts and ideas/wishlist:

  • I don't think the SDL sample currently supports transforms, that would be nice to have.
  • Maybe we should consider moving to eg. OpenGL 3.3 Core or GL ES2 and use shaders instead of the legacy pipeline?
  • Ideally, we would construct the samples so that all of them can be run with any backend of choice. One selection for platform and setting up a window (these would be eg. Windows, Linux, macOS, GLFW, SDL, SFML), and then choice of renderer (OpenGL legacy, OpenGL 3.3+, DirectX, Vulkan, Metal) on top of the window. Finally, some backends do both together (SDL_RenderGeometry, maybe SFML?).
  • The backends are structured such that they can easily be copied into a user's own project, like with Dear Imgui.

Certainly not saying these suggestions need to be part of this work, just dumping my thoughts here for now :) I can't really contribute anything meaningful SDL-specific. Although let me know if I can contribute with OpenGL later on.

@mikke89 mikke89 changed the title SLD2 fails to display image files with no alpha channel SDL2 fails to display image files with no alpha channel Dec 14, 2021
@mikke89 mikke89 added this to the 5.0 milestone Apr 2, 2022
@mikke89
Copy link
Owner

mikke89 commented Jun 1, 2022

It would be great to have another look and solve this now that we have merged the backends branch.

So now we essentially have three SDL backends with different renderers:

I'm only having this issue on the SDL/GL2 backend. It would be really nice if we could also remove the dependency on GLEW here. Could you help with this?

I didn't even add an SDL_Renderer to the SDL/GL3 backend, there I am only using SDL to open and handle the window and provide input, as well as image loading. Here I decided to use surface blitting to ensure correct texture color channels.

@mikke89 mikke89 added the backends Platforms and renderers label Dec 8, 2022
@mikke89
Copy link
Owner

mikke89 commented Dec 9, 2022

I made a fix for this for the SDL_GL2 backend, so RGB-textures should now work for all the SDL backends. I went with a variation of the suggestion from @RWD-Biggs, except I'm only converting the surface if no alpha channel is detected.

I'm closing this and consider the issue fixed. With that said, I'll still happily take contributions to further improve the SDL backend(s) as discussed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backends Platforms and renderers samples
Projects
None yet
Development

No branches or pull requests

3 participants