Skip to content

Don't use f32::round for filter 1D#2838

Merged
197g merged 1 commit intoimage-rs:mainfrom
RunDevelopment:no-f32--round-filter-1d
Mar 10, 2026
Merged

Don't use f32::round for filter 1D#2838
197g merged 1 commit intoimage-rs:mainfrom
RunDevelopment:no-f32--round-filter-1d

Conversation

@RunDevelopment
Copy link
Member

Same as #2837. I removed f32::round and replaced it with + 0.5 for better perf on x86..

The self.min(u16::MAX as f32) might be unnecessary too. I left it in for now, because it maps NaN to u16::MAX instead of 0 (by the as cast). If NaN isn't a concern for filter 1D, then it can be removed too. Just let me know.

@197g 197g merged commit b28909f into image-rs:main Mar 10, 2026
31 checks passed
@197g
Copy link
Member

197g commented Mar 10, 2026

I'm actually not sure myself..

@RunDevelopment RunDevelopment deleted the no-f32--round-filter-1d branch March 10, 2026 17:10
@awxkee
Copy link
Contributor

awxkee commented Mar 10, 2026

NaNs typically saturated to the lower bound if we assume ieee standard. However, NaNs appearing here is unlikely so I don’t think it’s important.

@197g
Copy link
Member

197g commented Mar 10, 2026

We might put it into compatibility notes but we don't provide a method that would filter float images but convert the result into a uint image. I don't think you could provoke a NaN-conversion from the image input except by the kernel; but then it applies to the whole image which does not seem so useful.

@awxkee
Copy link
Contributor

awxkee commented Mar 10, 2026

I think we've overlooked this a bit in those PRs, since I already put filter on not normal values. However, it's still possible to submit here NaN values. I don't see much point to mess with NaN inputs, if you see that different we could just return for any NaN input a default image filled with pre-defined value. exp is defined for all inputs except NaN so in practice except a system has malformed libm here should not be NaNs. As well we could scan kernel and if there is a NaN somewhere it's also possible to return a default image.
I could do PR with NaNs filter or return default image, or something else?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants