This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

• patch to implement the render targets (gl_rendertarget.diff, text/plain, 2010-04-15 20:51:07 +0000, 11021 bytes)
• reworked patch for render targets (gl_rendertarget.new.diff, text/plain, 2011-07-28 07:50:06 +0000, 10803 bytes)
• Implements render targets for Open GL and GL ES (gl_rendertarget.new.diff, text/plain, 2011-08-26 08:59:39 +0000, 17623 bytes)
• Render to texture target support for OpenGL, OpenGL ES and Direct3D (sdl_rendertotarget.diff, text/plain, 2011-09-05 15:25:29 +0000, 22848 bytes)
• Render to texture target support for OpenGL, OpenGL ES 1.1 and 2.0, Direct3D (sdl_rendertotarget.diff, text/plain, 2011-09-16 18:02:31 +0000, 30053 bytes)
• Render to texture target support for OpenGL, OpenGL ES 1.1 and 2.0, Direct3D (sdl_rendertotarget.diff, text/plain, 2012-01-18 07:11:48 +0000, 37722 bytes)
• Render to texture target support for OpenGL, OpenGL ES 1.1 and 2.0, Direct3D (sdl_rendertotarget.diff, text/plain, 2012-01-18 13:04:26 +0000, 39499 bytes)
• Modified testscale.c to test render targets (testrendertarget.c, text/x-csrc, 2012-01-18 13:07:18 +0000, 5646 bytes)
• Fix SetRenderTarget for OpenGL ES 1.1 and 2 (setrendertarget.fix.diff, text/plain, 2012-01-22 16:41:09 +0000, 1602 bytes)

Reported in version: 2.0.0
Reported for operating system, platform: All, Other

Comments on the original bug report:

On 2010-04-15 20:51:07 +0000, Mason Wheeler wrote:

Created attachment 505
patch to implement the render targets

This introduces a basic API for render targets, plus an implementation for the OpenGL renderer.

It allows for multiple render targets, and as per the OpenGL wiki, uses a separate FBO for each different texture size. For performance reasons, it is best to keep the distinct texture sizes down to a minimum.

http://www.opengl.org/wiki/GL_EXT_framebuffer_object# 1_FBO_or_more

NOTE: C is not my strong point. This compiles, and appears to run correctly, but please review this very carefully to make sure I didn't make any embarrassing mistakes.

On 2011-02-16 03:47:24 +0000, Sam Lantinga wrote:

Ken is working on this.

On 2011-03-11 18:32:20 +0000, Sam Lantinga wrote:

Ken doesn't have time to work on this, so I'll take a crack at it.

On 2011-07-28 07:50:06 +0000, Gabriel Jacobo wrote:

Created attachment 661
reworked patch for render targets

I've reworked this patch and now it applies cleanly on the hg trunk.
Adjusting it to the new code layout wasn't so much work, but still I spent a whole day trying to figure out why instead of a rendered texture I was getting a gray rectangle...it turns out that with my Nvidia card under Ubuntu Natty with propietary Nvidia drivers, if the target texture is created with TEXTURE_2D, this patch doesn't work! And I still can't figure out why...
To verify if it fails for others, apply the patch, then go to SDL_render_gl line 566 and uncomment the condition texture->access != SDL_TEXTUREACCESS_TARGET

The patch adds 3 functions:
SDL_bool SDL_RenderTargetSupported(SDL_Renderer *renderer);
int SDL_SetTargetTexture(SDL_Texture *texture);
SDL_ResetTargetTexture(SDL_Renderer *renderer);

In SetTargetTexture the texture is assigned to its renderer.

If there's interest in this I can probably extend this implementation to the OPENGL ES backend at least, or add a workaround for a non FBO dependent solution (slower as it may be).

On 2011-08-25 11:50:59 +0000, Ryan C. Gordon wrote:

I'll try to find some time to see about the non-GL paths. I can't put this revision control with just OpenGL support.

--ryan.

On 2011-08-26 08:59:39 +0000, Gabriel Jacobo wrote:

Created attachment 688
Implements render targets for Open GL and GL ES

Attaching new version of the patch with OpenGL ES support. Requires the fix for bug # 1290

On 2011-09-05 15:25:29 +0000, Gabriel Jacobo wrote:

Created attachment 698
Render to texture target support for OpenGL, OpenGL ES and Direct3D

Uploading a new patch that adds the feature to the Direct3D backend. Is this enough for acceptance? I'll probably add OpenGL ES 2 in a couple of days as well.

On 2011-09-16 18:02:31 +0000, Gabriel Jacobo wrote:

Created attachment 704
Render to texture target support for OpenGL, OpenGL ES 1.1 and 2.0, Direct3D

Updated patch to support OpenGL ES 2.0

On 2011-12-08 09:54:37 +0000, Ryan C. Gordon wrote:

One note: Do we need SDL_ResetTargetTexture()? Is there a reason that calling SDL_SetTargetTexture(NULL) wouldn't be better?

Rest of the patch looks good to me.

--ryan.

On 2011-12-08 10:31:06 +0000, Mason Wheeler wrote:

(In reply to comment # 8)

One note: Do we need SDL_ResetTargetTexture()? Is there a reason that calling
SDL_SetTargetTexture(NULL) wouldn't be better?

Rest of the patch looks good to me.

--ryan.

You're right, it's probably not necessary.

On 2011-12-08 10:45:11 +0000, Gabriel Jacobo wrote:

There's a chance that the patch won't apply cleanly given recent changes done by Sam in the GL backends. I did keep my experimental repo (https://bitbucket.org/gabomdq/sdl-1.3-experimental) updated with these changes merged in, so I can (if merging the patch as is proves too complicated) make a new patch, the problem is I think I'd have to extract manually from it the rotation functions I've added in the RenderCopyEx branch as well (unless you want to incorporate that as well...hey, don't blame me for trying!)

I agree that the ResetTarget is redundant.

On 2011-12-17 07:42:36 +0000, Mason Wheeler wrote:

Actually, as written, SDL_ResetTargetTexture is necessary. Calling SDL_SetTargetTexture(NULL) wouldn't work, because there is no way to determine the renderer. The call would fail inside the CHECK_TEXTURE_MAGIC macro.

On 2012-01-08 06:15:35 +0000, Gabriel Jacobo wrote:

Sam/Ryan, if you want me to rework the patch to go from:

SDL_bool SDL_RenderTargetSupported(SDL_Renderer *renderer);
int SDL_SetTargetTexture(SDL_Texture *texture);
SDL_ResetTargetTexture(SDL_Renderer *renderer);

to:

SDL_bool SDL_RenderTargetSupported(SDL_Renderer *renderer);
int SDL_SetTargetTexture(SDL_Texture *texture, SDL_Renderer *renderer);

Where you can do SDL_SetTargetTexture(NULL, renderer) to go back to the default target, let me know and I'll handle it.

On 2012-01-08 10:58:01 +0000, Sam Lantinga wrote:

Sure, if you can update it for the current hg and change the API, that would be great!

The API should probably be (note the renderer is always the first parameter):
SDL_bool SDL_RenderTargetSupported(SDL_Renderer *renderer);
int SDL_SetTargetTexture(SDL_Renderer *renderer, SDL_Texture *texture);

We should add something to SDL_RendererInfo which indicates whether render targets are supported and just return that from SDL_RenderTargetSupported()

Also, the render viewport should be taken into account when resetting the viewport. Right now it looks like it's just clobbering it with the window size.

Can someone make a simple program in the test directory that shows this functionality?

Thanks!

On 2012-01-18 07:11:48 +0000, Gabriel Jacobo wrote:

Created attachment 782
Render to texture target support for OpenGL, OpenGL ES 1.1 and 2.0, Direct3D

Patch updated to the apply cleanly on the current tip. I changed the API per Sam's suggestion and I had to add some cumbersome texture format mapping logic in RenderCopy of GLES2 because of the recent changes made to support additional texture formats (I suspect there's a cleaner solution somewhere out there). Also, I had to manually remove parts that are RenderCopyEx centric (rotation/flipping), so some (minor) measure of caution and testing is needed!

On 2012-01-18 13:04:26 +0000, Gabriel Jacobo wrote:

Created attachment 783
Render to texture target support for OpenGL, OpenGL ES 1.1 and 2.0, Direct3D

Updated patch to preserve the renderer viewport instead of clobbering it with the window size.

On 2012-01-18 13:07:18 +0000, Gabriel Jacobo wrote:

Created attachment 784
Modified testscale.c to test render targets

Attaching a simple test case, this is a slightly modified version of testscale.c

On 2012-01-18 19:47:02 +0000, Sam Lantinga wrote:

Okay, the first pass is in!
http://hg.libsdl.org/SDL/rev/37e8d0736366

It still needs a bit in SDL_RenderInfo and a software implementation, but it looks great!

Thanks guys! :)

On 2012-01-19 08:23:41 +0000, Gabriel Jacobo wrote:

By the bit in RenderInfo, do you mean adding an entry in http://wiki.libsdl.org/moin.cgi/SDL_RendererFlags and setting that accordingly in SDL_RenderInfo?

On 2012-01-19 18:07:31 +0000, Sam Lantinga wrote:

Yep, I added it:
http://hg.libsdl.org/SDL/rev/240f1bced46b

On 2012-01-20 06:00:32 +0000, Gabriel Jacobo wrote:

We also need at least a short paragraph documenting the two new functions, I can write it, should I send it to you Sam? I noticed the wiki mentions a documentation mailing list but I don't know if that's still functional.

On 2012-01-20 16:58:41 +0000, Sam Lantinga wrote:

The documentation mailing list is functional, but more for discussion between contributors. If you'd like to write the wiki pages yourself, you're more than welcome to and I can add you as a contributor once you have a wiki login.

On 2012-01-21 09:15:18 +0000, Gabriel Jacobo wrote:

Done, it's GabrielJacobo

On 2012-01-21 09:46:29 +0000, Sam Lantinga wrote:

Okay, you have access! Please review the style guides and try to maintain the same form as the other pages. Let me know if you have any questions!

http://wiki.libsdl.org/moin.cgi/Contributing

On 2012-01-21 22:33:52 +0000, Sam Lantinga wrote:

I added a software implementation and refactored the code a bit, but I think I broke OpenGL ES 1 and 2.

Can you take a quick look and see what I did?

Thanks!

On 2012-01-22 07:43:08 +0000, Gabriel Jacobo wrote:

I tried to do that Neo thing where I figured the problem by just looking at the diffs but it only works in the movies. I'll merge it with my SDL copy tonight and let you know what I get.

On 2012-01-22 07:48:08 +0000, Sam Lantinga wrote:

laugh Thanks!

On 2012-01-22 16:41:09 +0000, Gabriel Jacobo wrote:

Created attachment 797
Fix SetRenderTarget for OpenGL ES 1.1 and 2

This fixes OpenGL ES 1.1 and 2 under Android, I'd like to verify OpenGL but currently compilation in Linux seems to be broken, looking into that...

On 2012-01-23 04:41:24 +0000, Sam Lantinga wrote:

Comment on attachment 797
Fix SetRenderTarget for OpenGL ES 1.1 and 2

Gabriel and I went over this and it's a red herring. The existing OpenGL ES code works for him.

On 2012-01-23 07:11:14 +0000, Gabriel Jacobo wrote:

I just re tested everything, with SetRenderTarget setting the viewport BEFORE calling the internal implementation, that is with this patch applied:

diff -r e0521de318b2 -r 292a0bce2ae3 src/render/SDL_render.c
--- a/src/render/SDL_render.c dom ene 22 19:47:17 2012 -0300
+++ b/src/render/SDL_render.c dom ene 22 21:36:27 2012 -0300
@@ -847,9 +847,7 @@
}
renderer->target = texture;

• if (renderer->SetRenderTarget(renderer, texture) < 0) {

•    return -1;
  

• }

 if (texture) {
     viewport.x = 0;

@@ -863,6 +861,10 @@
return -1;
}

• if (renderer->SetRenderTarget(renderer, texture) < 0) {

•    return -1;
  

• }
• /* All set! */
  return 0;
  }

I got OpenGL under Linux, GLES1 and 2 under Android and D3D under Win7 working as expected. It seems the problem remains under iOS GLES1 and 2?

On 2012-01-25 05:49:14 +0000, Gabriel Jacobo wrote:

The documentation is available at http://wiki.libsdl.org/moin.cgi/SDL_SetRenderTarget (I couldn't figure out how to add it to the index of API by name/category...does that index need to be re generated somehow?)

On 2012-01-28 11:16:19 +0000, Sam Lantinga wrote:

Hey Gabriel, can you back out your change switching the order of setting the viewport and see if this change works instead?
http://hg.libsdl.org/SDL/rev/3aa24ec6bc39

On 2012-01-28 11:25:38 +0000, Sam Lantinga wrote:

Thanks for updating the documentation, it looks good!
I updated the API and Render pages by using the Page Actions -> More Actions -> Delete Cache menu option on each of those pages.

On 2012-01-29 16:03:29 +0000, Gabriel Jacobo wrote:

Sam, your change works fine here in GLES2/Android.

On 2012-01-29 19:30:57 +0000, Sam Lantinga wrote:

Great, thanks!

On 2012-01-29 19:38:27 +0000, Gabriel Jacobo wrote:

Did you figure out the magenta issue on the iPhone?

On 2012-01-30 08:59:17 +0000, Gabriel Jacobo wrote:

Sam, if the magenta issue in iOS devices persists, there's a comment that might be useful in this StackOverflow response... http://stackoverflow.com/questions/1024603/opengl-es-render-to-texture/1024634# 1024634

"Binding to Framebuffer Id 0 isn't always the Id of the one intialized as your primary framebuffer of the GL session. My ES2 session definitely stores a different one. I advise people to reference the one given upon the creation of your GL session. – Kyle Jan 26 '11 at 10:02"

I can't verify as I don't have a iOS device.

On 2012-01-30 17:23:01 +0000, Sam Lantinga wrote:

Oh hey, you're right! If I switch it to framebuffer 1 instead of 0 it works correctly.
Now how to detect this situation...

On 2012-01-30 18:05:38 +0000, Sam Lantinga wrote:

Okay, this is fixed for OpenGL ES 2.0:
http://hg.libsdl.org/SDL/rev/d1ba86bc57a5

That fix didn't help with OpenGL ES 1.1, and I'm not sure why.

On 2012-01-31 06:24:09 +0000, Gabriel Jacobo wrote:

First the good news, your fix works in GLES2 Android...however you broke GLES 1.1...

I decided to give up and read the manual:

http://www.khronos.org/registry/gles/extensions/OES/OES_framebuffer_object.txt
http://www.khronos.org/opengles/sdk/docs/man/xhtml/glBindFramebuffer.xml

They keep mentioning that framebuffer 0 is reserved for the system window, so it may be an iPhone GLES2 quirk not using 0 as the default framebuffer.

What I did notice is that calling data->glGetIntegerv(GL_FRAMEBUFFER_BINDING_OES, &value); doesn't produce any result in Android GLES1.1 if the active framebuffer is the default one, ie, whatever is in value stays unmodified.

So perhaps the solution for GLES 1.1 is just default window_framebuffer to zero ? That certainly fixes it in Android, but I'm afraid you'll be back at square one in iOS.

On 2012-01-31 18:04:15 +0000, Sam Lantinga wrote:

Fixed, thanks!
http://hg.libsdl.org/SDL/rev/246cde9a6cf3