Skip to content

Conversation

@AHolliday
Copy link
Contributor

@AHolliday AHolliday commented Apr 19, 2019

resolves #2102

This pullrequest changes

During one of the stages of selective search, the image is rotated 45 degrees and gradients are computed along the X and Y directions in the rotated image. The gradient image is then rotated back to the original orientation and cropped to remove surrounding empty space, leaving it with the same shape as the original image. However, the implementation of the crop will specify an ROI with negative x or y values if the image's native aspect ratio is too high or too low (basically, if the image is far from square).

This is because the bounding box of the rotated image is strictly bigger than the original image if the original is sufficiently close to square; but if the original is too "skinny", the bounding box of its rotation will be less tall or wide than the original.

The fix is to threshold the x and y computed for the ROI at 0.

Note: while fixing this bug, I also discovered that parts of "narrow" gradient images were being clipped at their edges due to improper cropping, again due to the apparent assumption that the bounding box of the rotated image was always strictly bigger in both axes than the original image. This PR includes a fix for that as well.

@alalek
Copy link
Member

alalek commented Apr 20, 2019

@AHolliday Thank you for investigating of this problem!

@the-glu Could you take a look on this patch?

@AHolliday
Copy link
Contributor Author

AHolliday commented May 29, 2019

@alalek @the-glu Anyone had a chance to take a look at this?

@alalek
Copy link
Member

alalek commented May 30, 2019

@AHolliday Looks like there is no objections about the fix (also there are no automatic tests for this functionality too), so lets apply proposed fix 👍

As a "bugfix" this patch should go into 3.4 branch first. We will merge changes from 3.4 into master regularly (weekly/bi-weekly).

So, please:

  • change "base" branch of this PR: master => 3.4 (use "Edit" button near PR title)
  • rebase your commits from master onto 3.4 branch. For example:
    git rebase -i --onto upstream/3.4 upstream/master
    (check list of your commits, save and quit (Esc + "wq" + Enter)
    where upstream is configured by following this GitHub guide and fetched (git fetch upstream).
  • push rebased commits into source branch of your fork (with --force option)

Note: no needs to re-open PR, apply changes "inplace".

@AHolliday AHolliday changed the base branch from master to 3.4 May 30, 2019 23:54
@AHolliday AHolliday closed this May 31, 2019
@alalek
Copy link
Member

alalek commented May 31, 2019

Something goes wrong here with git, so GitHub closes PR (and it can't be reopened for now at least from our/maintainers side).

Please revert changes back, something like:

git checkout -B fix_2102 54bac53
git push origin fix_2102 -f

(some extra commands to recover source tree: git stash, git rebase --abort)

Then try re-open this PR.


If anything fails, then open new PR (from 3.4 branch) with cross-link on this PR.
Backup of your patch is here.

@AHolliday
Copy link
Contributor Author

AHolliday commented Jun 1, 2019 via email

@AHolliday
Copy link
Contributor Author

AHolliday commented Jun 1, 2019 via email

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants