Skip to content
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

Perspective warp #648

Merged
merged 23 commits into from
Jan 13, 2021
Merged

Perspective warp #648

merged 23 commits into from
Jan 13, 2021

Conversation

dschneiderch
Copy link
Collaborator

Describe your changes
Add function to fuse images that have different size/scale/proportions, e.g. from two (different) cameras in different locations . The function accepts 2 images and 4 pairs of points in order to compute a perspective transformation matrix so the images overlay nicely. I found this works much better than simple scale/rotate/translate https://cougphenomics.github.io/camera_overlays.html

Type of update
Is this a:

  • New feature or feature enhancement

Associated issues
discussed in various forms here #615 and #634

@codecov
Copy link

codecov bot commented Nov 18, 2020

Codecov Report

Merging #648 (3de53e0) into master (4b885cd) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #648   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          129       130    +1     
  Lines         6120      6176   +56     
=========================================
+ Hits          6120      6176   +56     
Flag Coverage Δ
unittests 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
plantcv/plantcv/transform/__init__.py 100.00% <100.00%> (ø)
plantcv/plantcv/transform/warp.py 100.00% <100.00%> (ø)
plantcv/__init__.py 100.00% <0.00%> (ø)

@dschneiderch
Copy link
Collaborator Author

@nfahlgren @HaleySchuhl The tests are failing on colorspaces() function which I didn't touch !?! It looks like it isn't importing the rgb2gray_* functions properly but i have no idea why.

@nfahlgren
Copy link
Member

@dschneiderch I think I got it, it was a tricky one. The transform submodule is imported before the visualize submodule because visualize.colorspaces uses transform.resize_factor. The new transform.warp function now uses visulize.overlay_two_imgs. That's actually fine, but visualize.colorspaces also uses rgb2gray_hsv and rgb2gray_lab, which are imported after transform. So since transform imports from visualize I just had to import rgb2gray functions before transform. Clear as mud right? :)

@dschneiderch
Copy link
Collaborator Author

wow, well thanks. I would not have guessed that import order matters. that means there could be circular dependencies(!)

@nfahlgren
Copy link
Member

@dschneiderch this is great! I just tested it on one of our old use cases, fitting a mask derived from an RGB image onto an image from our NIR cabinet. One little issue I ran into that I need to think about more is that our NIR images are 16-bit and the mask is 8-bit, so when they get overlayed in the debug image it looks like weird noise. I'm not sure that it needs to be fixed, I just converted the NIR image to 8-bit to get around it.

In thinking about this function and our existing crop_position_mask I'm wondering if we should consider deprecating crop_position_mask in favor of warp? I think this new approach is easier to use and is more likely to produce a good fit because it takes into account more than just scaling and translation. I can imagine a scenario where it's not possible to pick landmark points and crop_position_mask could still be used, but not sure that is going to be a common situation. What do @dschneiderch and @HaleySchuhl think?

@dschneiderch
Copy link
Collaborator Author

Thanks!
My first thought is to internally convert refimg to 8bit for the display but that might be problematic if you have really low values - for example from a bioluminescence image. Maybe we should convert img to the type of refimg? It'd be a little more user friendly.
From a theoretical standpoint warp is just a more advanced geometric transformation than scale and translate used in crop_position_translate (which I think could be vastly simplified by switching to opencv functions). What scenario were you thiking about not picking landmark points? You should be able to get the same results with warp as with crop_position_mask. if you know your cropping parameters x and y than you can use them to warp too. simplest case, the refimg coords are simply the image limits. I'd suggest labeling crop_position_mask as soft deprecated and point people towards warp. Hard for me to imagine a case when crop_position_mask is more useful than warp - maybe just easier to wrap your brain around what's happening.

@nfahlgren
Copy link
Member

Yeah, just imagining a case where there's nothing in the background to use as anchor points. With crop_position_mask you could guess and check until you got the overlap the way you want but I think that would be harder with landmark points. But like I said, not sure that's a likely scenario and I do think warp is not only a more complete computational approach, but also easier if you have easy to identify landmark points (i.e. no guess and check necessary).

@dschneiderch
Copy link
Collaborator Author

dschneiderch commented Dec 4, 2020 via email

@nfahlgren
Copy link
Member

True, that makes sense. So maybe we can mark crop_position_mask with a deprecation warning for the remainder of v3 and remove it in v4, if folks agree with that plan.

@HaleySchuhl
Copy link
Contributor

That sounds like a good plan to me!

@nfahlgren nfahlgren self-assigned this Jan 12, 2021
@nfahlgren nfahlgren added the new feature New feature ideas and solutions label Jan 12, 2021
@nfahlgren nfahlgren added this to In progress in PlantCV3 via automation Jan 12, 2021
@nfahlgren nfahlgren added this to the PlantCV v3.11.0 milestone Jan 12, 2021
Copy link
Member

@nfahlgren nfahlgren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @dschneiderch, this is a nice function to have integrated in!

@nfahlgren nfahlgren merged commit 9420ffa into master Jan 13, 2021
PlantCV3 automation moved this from In progress to Done Jan 13, 2021
@nfahlgren nfahlgren deleted the perspective_warp branch January 13, 2021 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature ideas and solutions
Projects
No open projects
PlantCV3
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants