Review Request 108708: fix possibly broken rect size calculation in Krita blur filters

[Boudewijn Rempt]

> On Feb. 2, 2013, 9:44 p.m., Boudewijn Rempt wrote:
> > Yeah... If msvc warns about it, but I do think it's silly :-). Anyway, I still need to test, but I'm sure it's fine, it doesn't alter what we meant with the code.
> 
> Friedrich W. H. Kossebau wrote:
>     I was surprised as well, but really operator-() on an unsigned int only result in the 2-complement.
>     --- 8< ---
>     #include <iostream>
>     
>     int main()
>     {
>         int a = 5;
>         unsigned int b = 5;
>     
>         std::cout <<"int: "<< (-a)<<" uint: "<< (-b) << std::endl;
>     }
>     --- 8< ---
>     $ g++ test.cpp -o test
>     $ ./test 
>     int: -5 uint: 4294967291
>

I tested it and it still blurs, the same way as before, so I guess it's good :-)


- Boudewijn


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108708/#review26549
-----------------------------------------------------------


On Feb. 2, 2013, 12:49 a.m., Friedrich W. H. Kossebau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108708/
> -----------------------------------------------------------
> 
> (Updated Feb. 2, 2013, 12:49 a.m.)
> 
> 
> Review request for Calligra and Boudewijn Rempt.
> 
> 
> Description
> -------
> 
> MSVC hints in its warnings that using operator-() on an unsigned int will still result in an unsigned int.
> Reading up the intertubes on that it seems to be usually the 2's complement of the value.
> So all the rect calculations in these blur filter must have been giving strange results for quite some time.
> 
> Is that true? Nobody saw that?
> 
> Attached patch fixes that by always turning the values used for calculations into signed integer, by the implicit conversion coming with the assignment.
> 
> Other possible fixes would be to simply switch the operator-() usage into a -1* operation. Not sure what I prefer, your choice :)
> 
> 
> Diffs
> -----
> 
>   krita/plugins/filters/blur/kis_blur_filter.cpp b81f0bd 
>   krita/plugins/filters/blur/kis_gaussian_blur_filter.cpp 0c68562 
>   krita/plugins/filters/blur/kis_motion_blur_filter.cpp 6a13ca3 
> 
> Diff: http://git.reviewboard.kde.org/r/108708/diff/
> 
> 
> Testing
> -------
> 
> None, as I have no clue of the blur filters and how to see the effect of the patch. Left for Krita insiders.
> 
> 
> Thanks,
> 
> Friedrich W. H. Kossebau
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20130203/ea8f15a2/attachment.htm>