search

scalableminds / webknossos-wrap

Efficient 3D volume storage format for webKnossos and analyses
MIT License
8 stars 3 forks source link

Fix writing incomplete block #42

Closed valentin-pinkau closed 5 years ago

valentin-pinkau commented 5 years ago

This PR implements a fix for the case that an incomplete row-major (c order) block is written. When writing an incomplete block, the current implementation writes the data correctly but fills the rest of the block with scrambled data. The scrambled data origins from the block which is overwritten if it was empty, no harm is done. The bug comes due to the fact that the existing data is first read in colum-major order into a buffer and the data to write (c order) is loaded in the very same buffer.

I added two tests to catch this bug and implemented a fix by filling the correct block_buf.

valentin-pinkau commented 5 years ago

@amotta filling the buffers with zeros does not solve the problem, so I don't think it is necessary to zero them.

The bug comes due to the fact that the existing data is first read in colum-major order into a buffer and the data to write (c order) is loaded in the very same buffer.

amotta commented 5 years ago

@valentin-pinkau Ahh, now I get :) Thank you.

I will try to review the pull request tomorrow.

valentin-pinkau commented 5 years ago

@philippotto @amotta Thank you for your reviews! Please have another look at the code. I refactored the File::write_mat method and moved the order handling into the copy_from_order_agnostic method. I like the code much better now. I added and refactored the tests based on your feedback.