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

Add attention_x and attention_y as output to attention based crop #3470

Merged
merged 5 commits into from
Jan 16, 2023

Conversation

ejoebstl
Copy link
Contributor

Hello,

This is in response to #3084 and builds upon the following PR in libvips.

The code change exposes the coordinates of the center of attention when using attention based smart cropping.

I'd suggest to merge this PR after my changes are included in libvips.

@ejoebstl
Copy link
Contributor Author

ejoebstl commented Dec 1, 2022

The relevant changes will be released in version 8.14 of libvips.

Copy link
Owner

@lovell lovell left a comment

Choose a reason for hiding this comment

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

Hello, thanks for the PR, I've left a couple of questions/comments inline that I think will need addressing.

src/pipeline.cc Outdated Show resolved Hide resolved
src/pipeline.cc Outdated Show resolved Hide resolved
@lovell lovell added this to the v0.32.0 milestone Dec 27, 2022
@ejoebstl
Copy link
Contributor Author

@lovell - Thank you, feedback adressed.

The newest libvips release also includes the attentionX and attentionY fields.

Is there an approximate timeline for the next release?

@ejoebstl ejoebstl changed the title [WIP] Add attention_x and attention_y as output to attention based crop Add attention_x and attention_y as output to attention based crop Jan 13, 2023
@lovell lovell changed the base branch from main to flow January 13, 2023 14:50
@lovell
Copy link
Owner

lovell commented Jan 13, 2023

Thanks for the updates, please can you rebase against the flow branch (I've changed the target branch of this PR to match) as that has the latest libvips with support for this feature.

@ejoebstl
Copy link
Contributor Author

Done 🙌

@lovell
Copy link
Owner

lovell commented Jan 15, 2023

Thanks for the updates, LGTM. The Windows failures are due to what appears to be a possible bug in that latest node-addon-api, which I've pinned to the last-known good release via commit a9bd0e7

@lovell lovell merged commit 6d404f4 into lovell:flow Jan 16, 2023
lovell added a commit that referenced this pull request Jan 16, 2023
@lovell
Copy link
Owner

lovell commented Jan 16, 2023

Danke schön Emanuel!

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.

None yet

2 participants