New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fast Marching Method for Image Inpainting #663
Conversation
skimage/inpaint/_inpaint.pyx
Outdated
@@ -0,0 +1,315 @@ | |||
#cython: cdivision=True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be a separate module. Probably should go under filter
?
skimage/filter/_inpaint_fmm.pyx
Outdated
if flag[i1, j1] == KNOWN: | ||
if flag[i2, j2] == KNOWN: | ||
r = sqrt(2 - (u1 - u2) ** 2) | ||
s = (u1 + u2 - r) * 0.5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function as a whole is pretty difficult to understand and the variable names aren't helpful. That said, the papers I've looked at aren't terribly enlightening. Telea's 2002 paper might be a better reference here and maybe Sethian's 1996 paper (specifically section 4) would be useful, but I can't honestly say that I really follow it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, it is difficult. And I had read the two papers during literature survey but there always seems to be a gap in explaining how they reach the code implementation from the equation as stated using forward and backward differences. However, we know what we really need to compute.
Given the distance/time values of 2 adjacent neighbours, how do we compute the distance/time it takes to reach the pixel in question ? (Distance and time are interchangeable since the speed is taken to be 1 in a direction normal to the curve at all points.) Let's break it down a bit:
In _fast_marching_method
I always pass the adjacent neighbours, in the sense:
0 A 0
B ? D
0 C 0
I'll pass A-B or A-D or B-C or C-D not A-C or B-D. So that's where the 2
comes into the picture in r
computation. Essentially, since the distance between them is sqrt(2)
, that is the max difference between there distance/time values.
So, in what case will it be lesser ? I can imagine a curve inclined at -45 deg hitting B and C. In this case, intuitively, the values should be the same. Assume the values for both is 1
. So my r
would simply be sqrt(2)
. Now, since my s
value according to L210 would be less than 1
. I go to L214. And s = (u1 + u2 + r) / 2
. This would be essentially be 1 / sqrt(2)
times greater than u1
or u2
i.e. 1
.
Now if we imagine a unit isosceles right angle triangle as:
B -- ?
\ |
\ |
C
And the curve parallel to side BC, then yes the distance value given as 1 + 1/sqrt(2)
for ?
given 1
for B
and C
, does make sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now, I like to believe that for curve inclined at various different angles, the essence of r
would be to compute the perpendicular distance to this curve using the difference in u
values (which depends on the direction of the curve), since the curve always travels normal to itself and with unit speed, the perpendicular distance from the curve to ?
should give me the answer. I just add an offset in time to keep the u
values always progressing ahead in time, which is what s
does here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This type of explanation should be summarized succinctly in the docstring
so that future maintainers can follow along.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright. Can I give a link to the above comment ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it would be better to summarise it neatly and include it into the code.
Also, just read over the code a few times and think whether there's a way
to better express it. If there isn't, that's okay, but then we put in the
due consideration.
I'll give my +1 for merging, but I'd like for another dev to look over it before actually merging into master. The code is still a bit difficult to understand in parts, but that may just be a symptom of the algorithm. |
If the code is hard to read to a seasoned developer, then we need to work |
else: | ||
u_out = 1 + u1 | ||
elif flag[i2, j2] == KNOWN: | ||
u_out = 1 + u2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now, using the same train of thought, imagine a horizontal curve hitting B
. Then the time/distance taken to reach ?
will be 1 + time till B
.
skimage/filter/_inpaint_fmm.pyx
Outdated
|
||
# Inpainted value considering the effect of gradient of intensity value | ||
# More accurate pixel value calculation refer to the OpenCV | ||
# implementation: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be useful to explicitly mention that this diverges from Telea's implementation, which is what's referenced.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. /ping @chintak
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted.
Other than a couple of minor comments, I'm fine with merging. But as before, I'd like someone else to have a read through, since I've looked at the code so many times that I'm probably too familiar with the implementation to be unbiased. |
skimage/filter/_inpaint_fmm.pyx
Outdated
Notes | ||
----- | ||
The steps of the algorithm are as follows: | ||
- Extract the pixel with the smallest ``u`` value in the BAND pixels |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No explanation of the meaning of "u" (this is the Eikonal function? Err, no, I guess not. Gradient?)
Changes Unknown when pulling e3625e5 on chintak:inpaint into * on scikit-image:master*. |
Closed until further notice. Feel free to reopen if you would like to continue working on this; please check also #1053. |
Tested for gray scale images. Cython implementation in
_inpaint.pyx
andinpaint
wrapper function andinitialise
function implemented in python ininpaint.py
.demo_inpaint.py
provides an interactive way for image inpainting. Unit tests are provided intest_inpaint.py
.TODO: