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 4766 - Rendering with clip rect and view port will produce different results in version 2.0.10
Summary: Rendering with clip rect and view port will produce different results in vers...
Status: RESOLVED FIXED
Alias: None
Product: SDL
Classification: Unclassified
Component: render (show other bugs)
Version: 2.0.10
Hardware: All All
: P2 major
Assignee: Ryan C. Gordon
QA Contact: Sam Lantinga
URL:
Keywords: target-2.0.12
Depends on:
Blocks:
 
Reported: 2019-08-17 20:59 UTC by Moritz Bruder
Modified: 2020-03-09 04:12 UTC (History)
4 users (show)

See Also:


Attachments
File that reproduces the behavior. (1.24 KB, text/x-c++src)
2019-08-17 20:59 UTC, Moritz Bruder
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Moritz Bruder 2019-08-17 20:59:17 UTC
Created attachment 3925 [details]
File that reproduces the behavior.

I have an example where using SDL_RenderCopy with a clip rect and viewport will work in SDL 2.0.9 (i.e it renders the text properly) but not with SDL 2.0.10 (no text visible). I couldn't find anything related in the changelog.

Either document the behavior or change the behavior.
Comment 1 Sylvain 2019-08-19 09:02:51 UTC
I just checked and indeed behaviour has changed, which may be a regression - or a bug fix.

I noticed that if you write the example by first setting the viewport, then the clip rect:

SDL_RenderSetViewport(renderer, &r);
SDL_RenderSetClipRect(renderer, &r);

then, it would also fail on older SDL.



What happens (I only looked at opengl renderer), 
is that glScissor is called with.

data->glScissor(renderer->viewport.x + rect->x, h - renderer->viewport.y - rect->y - rect->h, rect->w, rect->h);

(or 
  data->glScissor(viewport->x + rect->x,
                 data->drawstate.target ?
                     viewport->y + rect->y : data->drawstate.drawableh - viewport->y - rect->y - rect->h,
                 rect->w, rect->h);
)

If viewport.{x,y} == {0,0}, then it's displaying something. 

I can't tell whether viewport.x/y should be actually given to glScissor
Comment 2 Nikola 2019-08-25 23:54:35 UTC
I have similar problem.

Goal: turn off clipping region before clear.

SDL_RenderSetClipRect(ren, 0);
SDL_RenderClear(ren);

Result: Clip region is still active.

-------

Fix is to activate clip region before clear in order to deactivate it. This is very weird and is different from previous sdl2 versions.

SDL_Rect r;
SDL_RenderSetClipRect(ren, &r);
SDL_RenderClear(ren); // now renders with deactivated
SDL_RenderSetClipRect(ren, 0);

--------

I believe problem is here:

SDL_render_gl.c:1169
if (data->drawstate.cliprect_enabled) {
                    data->glDisable(GL_SCISSOR_TEST);
                    data->drawstate.cliprect_enabled_dirty = SDL_TRUE;
                }
Comment 3 Nikola 2019-08-26 00:17:10 UTC
(In reply to Nikola from comment #2)
> I have similar problem.
> 
> Goal: turn off clipping region before clear.
> 
> SDL_RenderSetClipRect(ren, 0);
> SDL_RenderClear(ren);
> 
> Result: Clip region is still active.
> 
> -------
> 
> Fix is to activate clip region before clear in order to deactivate it. This
> is very weird and is different from previous sdl2 versions.
> 
> SDL_Rect r;
> SDL_RenderSetClipRect(ren, &r);
> SDL_RenderClear(ren); // now renders with deactivated
> SDL_RenderSetClipRect(ren, 0);
> 
> --------
> 
> I believe problem is here:
> 
> SDL_render_gl.c:1169
> if (data->drawstate.cliprect_enabled) {
>                     data->glDisable(GL_SCISSOR_TEST);
>                     data->drawstate.cliprect_enabled_dirty = SDL_TRUE;
>                 }

Actually this code is ok, but applying cliprect_enabled_dirty state is missing before this I think.
Comment 4 Sylvain 2019-08-26 10:06:53 UTC
Maybe to call SetDrawState in SDL_RENDERCMD_SETCLIPRECT



--- a/src/render/opengl/SDL_render_gl.c	Sat Aug 24 20:40:37 2019 +0200
+++ b/src/render/opengl/SDL_render_gl.c	Mon Aug 26 12:06:23 2019 +0200
@@ -1148,6 +1148,7 @@
                     SDL_memcpy(&data->drawstate.cliprect, rect, sizeof (SDL_Rect));
                     data->drawstate.cliprect_dirty = SDL_TRUE;
                 }
+                SetDrawState(data, cmd, SHADER_SOLID);
                 break;
             }
Comment 5 Alex Szpakowski 2019-08-26 21:49:52 UTC
I think this fixes the RenderClear issues: https://hg.libsdl.org/SDL/rev/d465f3a64af6
Let me know if it works!
Comment 6 Nikola 2019-08-26 23:58:55 UTC
(In reply to Alex Szpakowski from comment #5)
> I think this fixes the RenderClear issues:
> https://hg.libsdl.org/SDL/rev/d465f3a64af6
> Let me know if it works!

I have tested it, and in my case it fixed the problem.
Comment 7 Nikola 2019-08-27 00:30:47 UTC
And for original problem:

--- test.cpp	2019-08-27 02:24:39.330762694 +0200
+++ test2.cpp	2019-08-27 02:17:43.990752777 +0200
@@ -21,9 +21,9 @@
     // 
 
     SDL_Rect r { 20, 20, 50, 50 };
-    SDL_Rect r_relative { 0, 20, 50, 50 };
+    SDL_Rect r_relative { 0, 0, 50, 50 };
 
-    SDL_RenderSetClipRect(renderer, &r);
+    SDL_RenderSetClipRect(renderer, &r_relative);
     SDL_RenderSetViewport(renderer, &r);


My guess is that SDL_RenderSetClipRect is independent of viewport, its relative that is. Doesn't seem like a bug in my opinion.
Comment 8 Sylvain 2019-08-27 08:09:29 UTC
also, is the software renderer affected ?
Comment 9 Nikola 2019-08-27 09:09:35 UTC
(In reply to Sylvain from comment #8)
> also, is the software renderer affected ?

Software renderer was working correctly. So it is not affected.

And problem with:

SDL_RenderSetViewport(renderer, &r);
SDL_RenderSetClipRect(renderer, &r);

is behaving same way in software renderer.

(In reply to Nikola from comment #7)
> And for original problem:
> 
> --- test.cpp	2019-08-27 02:24:39.330762694 +0200
> +++ test2.cpp	2019-08-27 02:17:43.990752777 +0200
> @@ -21,9 +21,9 @@
>      // 
>  
>      SDL_Rect r { 20, 20, 50, 50 };
> -    SDL_Rect r_relative { 0, 20, 50, 50 };
> +    SDL_Rect r_relative { 0, 0, 50, 50 };
>  
> -    SDL_RenderSetClipRect(renderer, &r);
> +    SDL_RenderSetClipRect(renderer, &r_relative);
>      SDL_RenderSetViewport(renderer, &r);
> 
> 
> My guess is that SDL_RenderSetClipRect is independent of viewport, its
> relative that is. Doesn't seem like a bug in my opinion.
Comment 10 Sylvain 2019-08-27 09:36:10 UTC
ok. (I think I didn't notice the RenderClear issue).

And what about this modification:

--- a/src/render/opengl/SDL_render_gl.c	Sat Aug 24 20:40:37 2019 +0200
+++ b/src/render/opengl/SDL_render_gl.c	Mon Aug 26 12:06:23 2019 +0200
@@ -1148,6 +1148,7 @@
                     SDL_memcpy(&data->drawstate.cliprect, rect, sizeof (SDL_Rect));
                     data->drawstate.cliprect_dirty = SDL_TRUE;
                 }
+                SetDrawState(data, cmd, SHADER_SOLID);
                 break;
             }

It seems to me the text is displayed.
Comment 11 Nikola 2019-08-27 09:55:37 UTC
(In reply to Sylvain from comment #10)
> ok. (I think I didn't notice the RenderClear issue).
> 
> And what about this modification:
> 
> --- a/src/render/opengl/SDL_render_gl.c	Sat Aug 24 20:40:37 2019 +0200
> +++ b/src/render/opengl/SDL_render_gl.c	Mon Aug 26 12:06:23 2019 +0200
> @@ -1148,6 +1148,7 @@
>                      SDL_memcpy(&data->drawstate.cliprect, rect, sizeof
> (SDL_Rect));
>                      data->drawstate.cliprect_dirty = SDL_TRUE;
>                  }
> +                SetDrawState(data, cmd, SHADER_SOLID);
>                  break;
>              }
> 
> It seems to me the text is displayed.

Your solution also works, in my case.
Comment 12 Alex Szpakowski 2019-08-27 11:12:59 UTC
I think we should get clarification about whether the clip rect is meant to be relative to the viewport when drawing no matter the order of operations, before making any more changes.
Comment 13 Sylvain 2019-08-27 11:21:14 UTC
I agree this needs clarification.

(Btw, it was also question of removing the viewport, bug 2535)
Comment 14 Ryan C. Gordon 2019-09-20 20:47:34 UTC
We're changing how we do SDL release versions; now releases will be even numbers (2.0.10, 2.0.12, etc), and as soon as we tag a release, we'll move the internal version number to an odd number (2.0.12 ships, we tag the latest in revision control as 2.0.13 immediately, which will become 2.0.14 on release, etc).

As such, I'm moving the bugs tagged with target-2.0.11 to target 2.0.12. Sorry if you get a lot of email from this change!

Thanks,
--ryan.
Comment 15 Ryan C. Gordon 2019-09-20 20:48:43 UTC
We're changing how we do SDL release versions; now releases will be even numbers (2.0.10, 2.0.12, etc), and as soon as we tag a release, we'll move the internal version number to an odd number (2.0.12 ships, we tag the latest in revision control as 2.0.13 immediately, which will become 2.0.14 on release, etc).

As such, I'm moving the bugs tagged with target-2.0.11 to target 2.0.12. Sorry if you get a lot of email from this change!

Thanks,
--ryan.
Comment 16 Richard Russell 2020-01-12 12:20:51 UTC
(In reply to Alex Szpakowski from comment #5)
> I think this fixes the RenderClear issues:
> https://hg.libsdl.org/SDL/rev/d465f3a64af6
> Let me know if it works!

My app was also affected by the SDL_RenderClear issue, although I only noticed it on Android (without finding this report I wouldn't have had a clue what was happening!).  I can confirm that your fix works for me.
Comment 17 Sam Lantinga 2020-02-12 07:30:42 UTC
(In reply to Alex Szpakowski from comment #12)
> I think we should get clarification about whether the clip rect is meant to
> be relative to the viewport when drawing no matter the order of operations,
> before making any more changes.

Whatever the answer is, it should be consistent with the underlying graphics APIs, so people using them are familiar with the SDL behavior. Based on my experience it seems like the clip rect is independent of viewport.

Ryan, what's your thought?
Comment 18 Sam Lantinga 2020-03-09 03:05:53 UTC
It looks like the intent (and previous behavior) is to have the clip rect relative to the viewport. I'll make sure that's the case for all the renderers.
Comment 19 Sam Lantinga 2020-03-09 04:12:57 UTC
I verified that all renderers set the clip rectangle relative to the current viewport and updated the header documentation to reflect that.

Thanks everyone!