Hi there!
After upgrading from 2.0.12 to 2.0.22 I got a strange glitch.
After some investigation, I wrote simple code to reproduce the error.
float scale = float(2048) / float(1920); //window size.x = 2048, logical size.x =1920
SDL_RenderSetScale(renderer, scale, scale);
SDL_Rect clip = { 0, 0, 1920, 1080 };
SDL_RenderSetClipRect(renderer, &clip);
SDL_Rect new_clip;
SDL_RenderGetClipRect(renderer, &new_clip);
printf("before: %d %d %d %d\n", clip.x, clip.y, clip.w, clip.h);
printf("after : %d %d %d %d\n", new_clip.x, new_clip.y, new_clip.w, new_clip.h);
prints
before: 0 0 1920 1080
after : 0 0 1919 1080
Problem occurs because SDL_RenderGetClipRect uses SDL_floor for calculating dimensions (w/h).
Is it a bug or expected behavior?
@rmg.nik
Could you test the fix thatās been merged ?
also this is not a regression: this was already present in 2.0.12 and before. So Not sur if this fix the glitch
I tested and the problem is still reproducing.
I extended the code to reproduce the problem. It is āsimple draw loopā. I donāt remember why I added logic to backuping the scale and restoring the cliprect for the screen clearing, but if remove the scale backuping-restoring the bug disappears.
//init scale and cliprect
float scale = float(2048) / float(1920); //window size.x = 2048, logical size.x =1920
SDL_RenderSetScale(renderer, scale, scale);
SDL_Rect clip = { 0, 0, 1920, 1080 };
SDL_RenderSetClipRect(renderer, &clip);
for (int i = 0; i < 1000; ++i)
{
//----------------------------------------
//clear the entire screen logic
SDL_Rect cliprect;
float sx, sy; //remove this line to fix the problem
SDL_RenderGetScale(renderer, &sx, &sy); //remove this line to fix the problem
SDL_RenderSetScale(renderer, 1.0f, 1.0f); //remove this line to fix the problem
SDL_RenderGetClipRect(renderer, &cliprect);
SDL_RenderSetClipRect(renderer, nullptr);
//SDL_RenderClear(renderer);
SDL_RenderSetClipRect(renderer, &cliprect);
SDL_RenderSetScale(renderer, sx, sy); //remove this line to fix the problem
//----------------------------------------
//backup cliprect
SDL_Rect bac_cliprect;
SDL_RenderGetClipRect(renderer, &bac_cliprect);
//----------------------------------------
//set smaller cliprect
//this step is unnecessary to reproduce the problem
SDL_Rect sub_cliprect;
sub_cliprect.x = 0;
sub_cliprect.y = 0;
sub_cliprect.w = 100;
sub_cliprect.h = 100;
SDL_RenderSetClipRect(renderer, &sub_cliprect);
//draw something logic
//----------------------------------------
//restore cliprect
SDL_RenderSetClipRect(renderer, &bac_cliprect);
//----------------------------------------
//print current cliprect
SDL_RenderGetClipRect(renderer, &cliprect);
printf("cliprect: %d %d %d %d\n", cliprect.x, cliprect.y, cliprect.w, cliprect.h);
}
And no - in 2.0.12 the logic was slightly different and bug was missing.
So, there is no problem in float precision. Here or a problem in my logic, or it is necessary to use SDL_roundf in SDL_RenderGetClipRect.
Can you re-try exactly your first test-code ? Iāve doubled-checked and
before: 0 0 1920 1080
after : 0 0 1919 1080
was the output for every previous SDL version I have. And now in head :
before: 0 0 1920 1080
after : 0 0 1920 1080
With your second code, this is different.
before the output was constant to:
cliprect: 0 0 1919 1080
( which was also incorrect, because you expected 1920, where you get 1919).
and that has also changed with 2.0.16. and also later.
When you get and re-set the cliprect, you in fact apply the rounding method.
diff --git a/src/render/SDL_render.c b/src/render/SDL_render.c
index 58cda3d60..e4f50e9da 100644
--- a/src/render/SDL_render.c
+++ b/src/render/SDL_render.c
@@ -2464,10 +2464,10 @@ SDL_RenderGetViewport(SDL_Renderer * renderer, SDL_Rect * rect)
CHECK_RENDERER_MAGIC(renderer, );
if (rect) {
- rect->x = (int)SDL_floor(renderer->viewport.x / renderer->scale.x);
- rect->y = (int)SDL_floor(renderer->viewport.y / renderer->scale.y);
- rect->w = (int)SDL_floor(renderer->viewport.w / renderer->scale.x);
- rect->h = (int)SDL_floor(renderer->viewport.h / renderer->scale.y);
+ rect->x = (int)SDL_round(renderer->viewport.x / renderer->scale.x);
+ rect->y = (int)SDL_round(renderer->viewport.y / renderer->scale.y);
+ rect->w = (int)SDL_round(renderer->viewport.w / renderer->scale.x);
+ rect->h = (int)SDL_round(renderer->viewport.h / renderer->scale.y);
}
}
@@ -2507,10 +2507,10 @@ SDL_RenderGetClipRect(SDL_Renderer * renderer, SDL_Rect * rect)
CHECK_RENDERER_MAGIC(renderer, )
if (rect) {
- rect->x = (int)SDL_floor(renderer->clip_rect.x / renderer->scale.x);
- rect->y = (int)SDL_floor(renderer->clip_rect.y / renderer->scale.y);
- rect->w = (int)SDL_floor(renderer->clip_rect.w / renderer->scale.x);
- rect->h = (int)SDL_floor(renderer->clip_rect.h / renderer->scale.y);
+ rect->x = (int)SDL_round(renderer->clip_rect.x / renderer->scale.x);
+ rect->y = (int)SDL_round(renderer->clip_rect.y / renderer->scale.y);
+ rect->w = (int)SDL_round(renderer->clip_rect.w / renderer->scale.x);
+ rect->h = (int)SDL_round(renderer->clip_rect.h / renderer->scale.y);
}
}
Changing the rounding method would fit what you expect.
(Since, cliprect and viewport use the same calculation. we need to update GetViewport,
and same thing with w/h vs x/y)
But I think this doesnāt reflect the real viewport / clip that end up being used inside the backend.
Get/Set cliprect are not usual setter / getter methods : but more like a āget the cliprect at some scaleā.
Maybe we could also have the float versions that would be better for you use-case.
But even with the functions below, the second test case still produces a ā1919 1080ā output.
Because it single precision vs double precision.
diff --git a/src/render/SDL_render.c b/src/render/SDL_render.c
index 58cda3d60..f7899e746 100644
--- a/src/render/SDL_render.c
+++ b/src/render/SDL_render.c
@@ -2514,6 +2514,42 @@ SDL_RenderGetClipRect(SDL_Renderer * renderer, SDL_Rect * rect)
}
}
+int
+SDL_RenderSetClipRectF(SDL_Renderer * renderer, const SDL_FRect * frect)
+{
+ int retval;
+ CHECK_RENDERER_MAGIC(renderer, -1)
+
+ if (frect) {
+ renderer->clipping_enabled = SDL_TRUE;
+ renderer->clip_rect.x = (double)frect->x * renderer->scale.x;
+ renderer->clip_rect.y = (double)frect->y * renderer->scale.y;
+ renderer->clip_rect.w = (double)frect->w * renderer->scale.x;
+ renderer->clip_rect.h = (double)frect->h * renderer->scale.y;
+ } else {
+ renderer->clipping_enabled = SDL_FALSE;
+ SDL_zero(renderer->clip_rect);
+ }
+
+ retval = QueueCmdSetClipRect(renderer);
+ return retval < 0 ? retval : FlushRenderCommandsIfNotBatching(renderer);
+}
+
+void
+SDL_RenderGetClipRectF(SDL_Renderer * renderer, SDL_FRect * frect)
+{
+ CHECK_RENDERER_MAGIC(renderer, )
+
+ if (frect) {
+ frect->x = (float)(renderer->clip_rect.x / renderer->scale.x);
+ frect->y = (float)(renderer->clip_rect.y / renderer->scale.y);
+ frect->w = (float)(renderer->clip_rect.w / renderer->scale.x);
+ frect->h = (float)(renderer->clip_rect.h / renderer->scale.y);
+ }
+}
+
Yes my first code works correctly with ādoubleā. Sorry I have not published a full problemā¦
I donāt think that extending SDL API is a good idea. I always thought that SDL should be simple).
Replacing SDL_floor with SDL_round fixed the problem.
Replacing by āroundā indeed fixed the test-case, but the ClipRect / Viewporit you get isnāt whatās actually applied.
Thinking again about it. I think you should do the save/restore like this :
- Backup the ClipRect,
- Backup the Scale.
ā¦ do other rendering.
- Restore the Scale
- Restore the ClipRect
Maybe, but for now I just need that after SDL_RenderSetClipRect SDL_RenderGetClipRect returns the same value. This we achieve by replacing SDL_floor with SDL_round.
Thank you!
Youāre talking about modifying your own sdl version to work around temporarly ?
Yes, I modified the SDL source code in our project.