We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 983 - Render target implementation for GL renderer, plus basic API
Summary: Render target implementation for GL renderer, plus basic API
Status: RESOLVED FIXED
Alias: None
Product: SDL
Classification: Unclassified
Component: video (show other bugs)
Version: 2.0.0
Hardware: Other All
: P2 API change
Assignee: Gabriel Jacobo
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-15 20:51 UTC by Mason Wheeler
Modified: 2012-01-31 18:04 UTC (History)
2 users (show)

See Also:


Attachments
patch to implement the render targets (10.76 KB, patch)
2010-04-15 20:51 UTC, Mason Wheeler
Details | Diff
reworked patch for render targets (10.55 KB, patch)
2011-07-28 07:50 UTC, Gabriel Jacobo
Details | Diff
Implements render targets for Open GL and GL ES (17.21 KB, patch)
2011-08-26 08:59 UTC, Gabriel Jacobo
Details | Diff
Render to texture target support for OpenGL, OpenGL ES and Direct3D (22.31 KB, patch)
2011-09-05 15:25 UTC, Gabriel Jacobo
Details | Diff
Render to texture target support for OpenGL, OpenGL ES 1.1 and 2.0, Direct3D (29.35 KB, patch)
2011-09-16 18:02 UTC, Gabriel Jacobo
Details | Diff
Render to texture target support for OpenGL, OpenGL ES 1.1 and 2.0, Direct3D (36.84 KB, patch)
2012-01-18 07:11 UTC, Gabriel Jacobo
Details | Diff
Render to texture target support for OpenGL, OpenGL ES 1.1 and 2.0, Direct3D (38.57 KB, patch)
2012-01-18 13:04 UTC, Gabriel Jacobo
Details | Diff
Modified testscale.c to test render targets (5.51 KB, text/x-csrc)
2012-01-18 13:07 UTC, Gabriel Jacobo
Details
Fix SetRenderTarget for OpenGL ES 1.1 and 2 (1.56 KB, patch)
2012-01-22 16:41 UTC, Gabriel Jacobo
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mason Wheeler 2010-04-15 20:51:07 UTC
Created attachment 505 [details]
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.
Comment 1 Sam Lantinga 2011-02-16 03:47:24 UTC
Ken is working on this.
Comment 2 Sam Lantinga 2011-03-11 18:32:20 UTC
Ken doesn't have time to work on this, so I'll take a crack at it.
Comment 3 Gabriel Jacobo 2011-07-28 07:50:06 UTC
Created attachment 661 [details]
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).
Comment 4 Ryan C. Gordon 2011-08-25 11:50:59 UTC
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.
Comment 5 Gabriel Jacobo 2011-08-26 08:59:39 UTC
Created attachment 688 [details]
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
Comment 6 Gabriel Jacobo 2011-09-05 15:25:29 UTC
Created attachment 698 [details]
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.
Comment 7 Gabriel Jacobo 2011-09-16 18:02:31 UTC
Created attachment 704 [details]
Render to texture target support for OpenGL, OpenGL ES 1.1 and 2.0, Direct3D

Updated patch to support OpenGL ES 2.0
Comment 8 Ryan C. Gordon 2011-12-08 09:54:37 UTC
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.
Comment 9 Mason Wheeler 2011-12-08 10:31:06 UTC
(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.
Comment 10 Gabriel Jacobo 2011-12-08 10:45:11 UTC
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.
Comment 11 Mason Wheeler 2011-12-17 07:42:36 UTC
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.
Comment 12 Gabriel Jacobo 2012-01-08 06:15:35 UTC
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.
Comment 13 Sam Lantinga 2012-01-08 10:58:01 UTC
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!
Comment 14 Gabriel Jacobo 2012-01-18 07:11:48 UTC
Created attachment 782 [details]
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!
Comment 15 Gabriel Jacobo 2012-01-18 13:04:26 UTC
Created attachment 783 [details]
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.
Comment 16 Gabriel Jacobo 2012-01-18 13:07:18 UTC
Created attachment 784 [details]
Modified testscale.c to test render targets

Attaching a simple test case, this is a slightly modified version of testscale.c
Comment 17 Sam Lantinga 2012-01-18 19:47:02 UTC
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! :)
Comment 18 Gabriel Jacobo 2012-01-19 08:23:41 UTC
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?
Comment 19 Sam Lantinga 2012-01-19 18:07:31 UTC
Yep, I added it:
http://hg.libsdl.org/SDL/rev/240f1bced46b
Comment 20 Gabriel Jacobo 2012-01-20 06:00:32 UTC
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.
Comment 21 Sam Lantinga 2012-01-20 16:58:41 UTC
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.
Comment 22 Gabriel Jacobo 2012-01-21 09:15:18 UTC
Done, it's GabrielJacobo
Comment 23 Sam Lantinga 2012-01-21 09:46:29 UTC
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
Comment 24 Sam Lantinga 2012-01-21 22:33:52 UTC
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!
Comment 25 Gabriel Jacobo 2012-01-22 07:43:08 UTC
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.
Comment 26 Sam Lantinga 2012-01-22 07:48:08 UTC
*laugh*  Thanks!
Comment 27 Gabriel Jacobo 2012-01-22 16:41:09 UTC
Created attachment 797 [details]
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...
Comment 28 Sam Lantinga 2012-01-23 04:41:24 UTC
Comment on attachment 797 [details]
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.
Comment 29 Gabriel Jacobo 2012-01-23 07:11:14 UTC
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?
Comment 30 Gabriel Jacobo 2012-01-25 05:49:14 UTC
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?)
Comment 31 Sam Lantinga 2012-01-28 11:16:19 UTC
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
Comment 32 Sam Lantinga 2012-01-28 11:25:38 UTC
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.
Comment 33 Gabriel Jacobo 2012-01-29 16:03:29 UTC
Sam, your change works fine here in GLES2/Android.
Comment 34 Sam Lantinga 2012-01-29 19:30:57 UTC
Great, thanks!
Comment 35 Gabriel Jacobo 2012-01-29 19:38:27 UTC
Did you figure out the magenta issue on the iPhone?
Comment 36 Gabriel Jacobo 2012-01-30 08:59:17 UTC
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.
Comment 37 Sam Lantinga 2012-01-30 17:23:01 UTC
Oh hey, you're right!  If I switch it to framebuffer 1 instead of 0 it works correctly.
Now how to detect this situation...
Comment 38 Sam Lantinga 2012-01-30 18:05:38 UTC
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.
Comment 39 Gabriel Jacobo 2012-01-31 06:24:09 UTC
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.
Comment 40 Sam Lantinga 2012-01-31 18:04:15 UTC
Fixed, thanks!
http://hg.libsdl.org/SDL/rev/246cde9a6cf3