www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - Tried C++ to D. Wrong result.

reply Dmitry <dmitry indiedev.ru> writes:
I tried translate C++ programm to D, but result is different.

original:
https://github.com/urraka/alpha-bleeding/blob/master/src/alpha-bleeding.cpp
result (with removed alpha):
https://github.com/urraka/alpha-bleeding/blob/master/media/alpha-bleeding-opaque.png

my:
https://pastebin.com/GzZQ7WHt
result (with removed alpha):
https://www.dropbox.com/s/xbjphlievboslv2/original-2.png

What did I wrong?

P.S. Also on an one image it was crashed at line 63 (range 
violation)
https://pastebin.com/TenGusw0
so I added ((index + 3) < N) into condition.
Nov 27 2017
next sibling parent reply rikki cattermole <rikki cattermole.co.nz> writes:
Did you confirm that the image was loaded originally correctly?
Nov 27 2017
parent Dmitry <dmitry indiedev.ru> writes:
On Monday, 27 November 2017 at 14:14:12 UTC, rikki cattermole 
wrote:
 Did you confirm that the image was loaded originally correctly?
Yes. This image was used: https://github.com/urraka/alpha-bleeding/blob/master/media/original.png
Nov 27 2017
prev sibling next sibling parent reply Stefan Koch <uplink.coder googlemail.com> writes:
On Monday, 27 November 2017 at 14:08:27 UTC, Dmitry wrote:
 I tried translate C++ programm to D, but result is different.

 original:
 https://github.com/urraka/alpha-bleeding/blob/master/src/alpha-bleeding.cpp
 result (with removed alpha):
 https://github.com/urraka/alpha-bleeding/blob/master/media/alpha-bleeding-opaque.png

 my:
 https://pastebin.com/GzZQ7WHt
 result (with removed alpha):
 https://www.dropbox.com/s/xbjphlievboslv2/original-2.png

 What did I wrong?

 P.S. Also on an one image it was crashed at line 63 (range 
 violation)
 https://pastebin.com/TenGusw0
 so I added ((index + 3) < N) into condition.
First I'd make sure that what you get out of dlib load is the same as the c++ version gets. Just use standard debugging techniques.
Nov 27 2017
parent Dmitry <dmitry indiedev.ru> writes:
On Monday, 27 November 2017 at 14:35:39 UTC, Stefan Koch wrote:
 First I'd make sure that what you get out of dlib load is the 
 same as the c++ version gets.
 Just use standard debugging techniques.
Yes, it's same. As you can see, the top-middle area of the result is same. I wrote a video of process (D version, every 100-th frame) https://www.dropbox.com/s/hcw1x4cwjou69su/video.mpg C++ version: https://www.dropbox.com/s/i7xpa5mzddpz6nu/video_orig.mpg
Nov 27 2017
prev sibling parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Monday, 27 November 2017 at 14:08:27 UTC, Dmitry wrote:
 https://pastebin.com/GzZQ7WHt
The first thing that jumped out to me is this: size_t[] pending = new size_t[N]; size_t[] pendingNext = new size_t[N]; That's giving it N elements of zero, then you append to it later with pending ~= i; In the C++ version they are declared std::vector<size_t> pending; std::vector<size_t> pendingNext; which doesn't put empty elements at it. I suspect you will get better results by just making the D decls size_t[] pending; size_t[] pendingNext; and leave the rest of the code the same and see what happens.
Nov 27 2017
parent reply Dmitry <dmitry indiedev.ru> writes:
On Monday, 27 November 2017 at 17:01:29 UTC, Adam D. Ruppe wrote:
 In the C++ version they are declared

 std::vector<size_t> pending;
 std::vector<size_t> pendingNext;
Ah, indeed. I thought that pending.reserve(N); pendingNext.reserve(N); initializes them (last time I used C++ about 17 years ago...)
 I suspect you will get better results by just making the D decls
 and leave the rest of the code the same and see what happens.
It fixed a delay (you can see it on video I posted before), but result is same.
Nov 27 2017
parent reply Dmitry <dmitry indiedev.ru> writes:
On Monday, 27 November 2017 at 17:21:05 UTC, Dmitry wrote:
 It fixed a delay (you can see it on video I posted before), but 
 result is same.
It seems I found the problem. C++ version (line 93): if (image[index + 3] != 0) I changed to if (image[index] != 0) and it works. I don't understand why there was used "+ 3" (and why it works in C++ version). Thank you all for help.
Nov 27 2017
parent reply =?UTF-8?Q?Ali_=c3=87ehreli?= <acehreli yahoo.com> writes:
On 11/27/2017 10:25 AM, Dmitry wrote:
 On Monday, 27 November 2017 at 17:21:05 UTC, Dmitry wrote:
 It fixed a delay (you can see it on video I posted before), but result
 is same.
It seems I found the problem. C++ version (line 93): if (image[index + 3] != 0) I changed to if (image[index] != 0) and it works. I don't understand why there was used "+ 3" (and why it works in C++ version).
So, it looks like the original code was accessing out of bounds and probably that's why you inserted the ((index + 3) < N) check in the D version because D was catching that error at runtime. // C++ if (image[index + 3] != 0) // D if (((index + 3) < N) && (data[index + 3] != 0)) Which of course would skip the body of the if block, causing a difference from the original result. Ali
Nov 27 2017
parent reply Dmitry <dmitry indiedev.ru> writes:
On Monday, 27 November 2017 at 18:40:41 UTC, Ali Çehreli wrote:

 So, it looks like the original code was accessing out of bounds 
 and probably that's why you inserted the ((index + 3) < N) 
 check in the D version because D was catching that error at 
 runtime.
Yes, it is.
 Which of course would skip the body of the if block, causing a 
 difference from the original result.
Yes. I fixed it in C++ version also and now both versions works same.
Nov 27 2017
parent reply =?UTF-8?Q?Ali_=c3=87ehreli?= <acehreli yahoo.com> writes:
On 11/27/2017 10:47 AM, Dmitry wrote:
 On Monday, 27 November 2017 at 18:40:41 UTC, Ali Çehreli wrote:

 So, it looks like the original code was accessing out of bounds and
 probably that's why you inserted the ((index + 3) < N) check in the D
 version because D was catching that error at runtime.
Yes, it is.
This is exactly the kind of bug Walter wanted to avoid when leaving C's arrays behind. (This includes C++'s std::vector because vector::operator[] is permissive. (To be safe, one needs to use .at() or check indexes explicitly.)) So, as advertised, port your programs to D and the results will likely be more correct. I like it! :) Ali P.S. I think you have an unnecessary 'ref' on the D version because a slice is already a reference to elements: // C++ void alpha_bleeding(unsigned char *image, int width, int height) // D private void alphaBleeding(ref ubyte[] data, int width, int height) You would need that 'ref' if you wanted to modify the original array itself by e.g. adding elements to it.
Nov 27 2017
parent reply Dmitry <dmitry indiedev.ru> writes:
On Monday, 27 November 2017 at 19:01:28 UTC, Ali Çehreli wrote:
 P.S. I think you have an unnecessary 'ref' on the D version 
 because a slice is already a reference to elements:
Fixed, thank you.
Nov 27 2017
parent reply Temtaime <temtaime gmail.com> writes:
On Tuesday, 28 November 2017 at 06:46:18 UTC, Dmitry wrote:
 On Monday, 27 November 2017 at 19:01:28 UTC, Ali Çehreli wrote:
 P.S. I think you have an unnecessary 'ref' on the D version 
 because a slice is already a reference to elements:
Fixed, thank you.
https://pastebin.com/xJXPBh0n Converted it and it works as expected.
Nov 28 2017
parent Dmitry <dmitry indiedev.ru> writes:
On Tuesday, 28 November 2017 at 09:01:47 UTC, Temtaime wrote:
 https://pastebin.com/xJXPBh0n
 Converted it and it works as expected.
What did you use for it? In future I'll be needed to convert some more C++ code. P.S. /And it works wrong, because uses unsafe pointer (ubyte *image). So, it takes wrong values (Blue of the next pixel instead of Alpha of the current pixel). Same with original code./ P.P.S. Anyway, I already found all things I did wrong. But also I found in your code that there is `swap` function, didn't know it. Thank you!
Nov 28 2017