foobaz / lossypng

Shrink PNG files by applying a lossy filter
138 stars 18 forks source link

Deep copy #3

Open kasimi opened 8 years ago

kasimi commented 8 years ago

Thanks for your work, I'm following this project with great interest.

I'm completely new to Go so I could be wrong about this. In these two lines

the intention of the loop is to copy the array values from row 0 to rows 1..n, however, what happens is that merely the references from row 0 are copied,

If I do colorError[0][0][0] = 999 after the loop, colorError[1][0][0] equals 999 too.

Also, you can simplify the code a tad by looping until i < errorRowCount - 1 and removing the % errorRowCount.

kasimi commented 8 years ago

Please disregard my comment about the deep copy, I was mistaken. I now understand that the loops I linked to are supposed to shift the rows, not copy their values. However, I do think there's a bug because the loops effectively assign colorError[0] to colorError[1] and colorError[2] resulting in the issue with the 999.

Moreover, I think the last row needs to be zero'd before continuing with the calculations as it hasn't been visited by the algorithm yet.

Here is what I think should happen (Java):

// Remember first row
int[][] firstRow = colorError[0];

// Move rows one level up
for (int y = 1; y < colorError.length; y++) {
    colorError[y - 1] = colorError[y];
}

// Set new last row
colorError[colorError.length - 1] = firstRow;

// Zero last row
for (int x = 0; x < firstRow.length; x++) {
    Arrays.fill(firstRow[x], 0);
}