martin-marek / hdr-plus-swift

📸Night mode on any camera. Based on HDR+.
https://burst.photo
GNU General Public License v3.0
198 stars 9 forks source link

Fix off-by-one issue introduced in #38 #50

Closed Alex-Vasile closed 2 months ago

Alex-Vasile commented 2 months ago

Part of #38 combined the conversion from a .RGBA texture (MxNx4) to a .R texture (2Mx2N) with the subsequent cropping step. The RGBA texture is used since it conveniently stores all 4 sub-pixels of a Bayer CFA at one spatial coordinate using 4-vector while also providing performance benefits. The conversion back to the .R texture is done since the final image is a .R texture (a flattened Bayered image).

This had a subtle bug I missed the first time around in the following lines:

command_encoder.setBytes([Int32(pad_left/2)], length: MemoryLayout<Int32>.stride, index: 0)
command_encoder.setBytes([Int32(pad_top/2)], length: MemoryLayout<Int32>.stride, index: 1)

Nothing ensure that pad_left and pad_top inside convert_to_bayer is a multiple of 2. Originally, as used by crop_texture this was not a problem since pad_left and pad_top were used directly to index into the .R texture so the division by 2 was not needed. The division by 2 is needed since the RGBA texture indexes by Bayer CFA repeat units, rather than individual sub-pixels (i.e. RGBG instead of R, G, B, or G).

Certain images I've found end up requiring pad_left and/or pad_top to be odd, meaning that the indexing needing is no longer aligned and couldn't be performed in the .RGBA image.

Profiling the change, the difference in speed between doing the steps sequentially (as previously done) versus together did not yield a measurable difference in performance. So rather than fiddle with the padding math, I undid the portion of the PR that changed how convert_to_bayer worked.