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