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 2687

Summary: SDL_BlitScaled does not handle clipping correctly
Product: SDL Reporter: Benoit Pierre <benoit.pierre>
Component: videoAssignee: Sam Lantinga <slouken>
Status: RESOLVED FIXED QA Contact: Sam Lantinga <slouken>
Severity: normal    
Priority: P2 CC: philipp.wiesemann, sylvain.becker
Version: 2.0.3   
Hardware: x86_64   
OS: Linux   
Attachments: patch to fix SDL_BlitScaled clipping
Python test script
add SDL_round
patch to fix SDL_BlitScaled clipping
drop SDL_round....

Description Benoit Pierre 2014-08-15 15:00:14 UTC 
Created attachment 1820 [details]
patch to fix SDL_BlitScaled clipping

1. the destination clipping rectangle is ignored
2. when clipping the source rectangle, the destination rectangle is not updated
3. when clipping the destination rectangle, the source rectangle is not updated

2 and 3 result in the scaling ratios not being kept.

The attached Python script demonstrate the issue (pysdl2 is needed: http://pysdl2.readthedocs.org/).

Usage:

- the source surface is shown on the left, the destination surface on the right
- the source/destination rectangles are shown in blue
- the destination clipping rectangle is shown in red
- the final destination rectangle (after clipping) is shown in green
- clicking with the left/right/middle button select the source/destination/clip rectangle, additional clicks center the rectangle on the mouse cursor
- using the mouse wheel resize the selected rectangle

I can convert it to C if Python is an issue, and there is interest in adding it to the existing tests.

The attached patch fix SDL_UpperBlitScaled implementation:

- honor destination clipping rectangle
- update both destination and source rectangles when clipping source rectangle to source surface and destination rectangle to destination clip rectangle
- don't change scaling factors when clipping

N.B.:

- when no scaling is involved (source and destination width/height are the same), SDL_UpperBlit is used (so SDL_BlitScaled behaves like SDL_BlitSurface)
- the final destination rectangle after all clipping is performed is saved back to dstrect (like for SDL_UpperBlit)
- with this patch, I now get the same output with the software and the OpengGL accelerated implementations when using the renderer API (calls to SDL_RenderSetClipRect/SDL_RenderCopy with scaling involved)
Comment 1 Benoit Pierre 2014-08-15 15:00:49 UTC 
Created attachment 1823 [details]
Python test script
Comment 2 Philipp Wiesemann 2014-08-15 19:02:36 UTC 
(In reply to Benoit Pierre from comment #0)
> Created attachment 1820 [details]
> patch to fix SDL_BlitScaled clipping

The patch uses round() from math.h but SDL does not link with the C standard library on every supported platform. Therefore round() should either be introduced as SDL_round() in SDL_stdinc.h or be replaced with other functions already available there.
Comment 3 Benoit Pierre 2014-08-15 21:06:00 UTC 
(In reply to Philipp Wiesemann from comment #2)
> (In reply to Benoit Pierre from comment #0)
> > Created attachment 1820 [details]
> > patch to fix SDL_BlitScaled clipping
> 
> The patch uses round() from math.h but SDL does not link with the C standard
> library on every supported platform. Therefore round() should either be
> introduced as SDL_round() in SDL_stdinc.h or be replaced with other
> functions already available there.

OK, so here is a patch that add SDL_round, lifting the actual implementation from uClibc, and an updated SDL_BlitScaled patch that depends on it.

It tested both the Autotools and the CMake builds on Linux, with and without libmath round (by manually undefining HAVE_ROUND in SDL_config.h). I updated the various SDL_config_PLATFORM.h headers, defining HAVE_ROUND when HAVE_MATH was already.

(I noticed the SDL_config_pandora.h has HAVE_MATH defined but not HAVE_POW, normal?)
Comment 4 Benoit Pierre 2014-08-15 21:06:29 UTC 
Created attachment 1824 [details]
add SDL_round
Comment 5 Benoit Pierre 2014-08-15 21:07:03 UTC 
Created attachment 1825 [details]
patch to fix SDL_BlitScaled clipping
Comment 7 Benoit Pierre 2014-08-17 09:21:16 UTC 
Great, but I don't see src/libm/s_round.c, did you forget to add it? :P

Also, how would one go about updating the documentation on https://wiki.libsdl.org, just request an account and edit away? I'd like to update SDL_BlitScaled documentation to mention the fact that dstrect will be updated.
Comment 8 Philipp Wiesemann 2014-08-17 12:23:56 UTC 
Unlike the other files in src/libm the s_round.c from the patch was released under the GNU LGPL license. This may cause a license problem for the platforms were SDL would be using it (because it is compiled into SDL and not just dynamically linked).
Comment 9 Benoit Pierre 2014-08-17 12:56:39 UTC 
Created attachment 1827 [details]
drop SDL_round....
Comment 10 Benoit Pierre 2014-08-17 12:57:11 UTC 
Damn, sorry about that... The attached patch drop SDL_round and just manually round with SDL_trunc(x + 0.5).
Comment 11 Sam Lantinga 2014-08-17 17:08:17 UTC 
Do you have an implementation of SDL_trunc()? :)
Comment 12 Benoit Pierre 2014-08-17 17:54:33 UTC 
Wow, this is embarrassing... :( I thought SDL already provided SDL_trunc...  SDL_floor can be used instead.

But the real question, is why does the CMake build not compiles everything with a least -Wall? I don't get any warning at all! It builds, it links... No complaints! Of course the resulting library is missing SDL_trunc:

> objdump -t libSDL2-2.0.so | grep trunc
0000000000000000         *UND*  0000000000000000              SDL_trunc
Comment 13 Sam Lantinga 2014-08-17 20:17:57 UTC 
Updated, using SDL_floor()
https://hg.libsdl.org/SDL/rev/2e4e71ec140f
Comment 14 Sylvain 2021-01-02 08:44:46 UTC 
for the record, 
https://hg.libsdl.org/SDL/rev/7101c5a0d561

- better fix to clip after scaling computation is done
- add back back SDL_round (added in bug #5404)