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

Apply filter on picture taken #196

Merged
merged 1 commit into from
Jan 3, 2020
Merged

Conversation

rawbertp
Copy link
Contributor

@rawbertp rawbertp commented Nov 15, 2019

Screen Recording 2019-11-15 at 11 27 39 1

Instead of selecting a filter before taking the picture (without even knowing what the filter result is going to look like!), allow the users to apply the filter after the picture was taken. The selected filter will be applied immediately.

Fixes/addresses: #190 ( #96 )

Further improvements/next steps (but not necessarily as part of this PR):

  • Present a tiny preview ("on hover") showing what the filter result looks like
  • Use IMagick and implement "nicer" filters

@sualko @andi34 I have set this to WIP as there are some minor bits and pieces missing but you can already review it if you want to.

@rawbertp rawbertp force-pushed the rp-filter-pic branch 5 times, most recently from 2757978 to 96144af Compare November 17, 2019 17:47
@sualko sualko added the WIP label Nov 21, 2019
@sualko
Copy link
Collaborator

sualko commented Nov 21, 2019

I like the idea to choose the filter afterwards and I think there are just some ui things we have to revisit. The current filter list doesn't look that good as overlay, but I assume this was just a proof of concept. If you have time it would be great if you could finalize this pr.

Further improvements/next steps (but not necessarily as part of this PR)

Would love to see this in a second pr, so that we can merge your enhancement asap.

@andi34
Copy link
Collaborator

andi34 commented Dec 17, 2019

Thanks for the nice work!

I've already rebased it here which can be picked once #206 and maybe #205 was merged:
andi34@5d4c3da

Works fine for me (only had time to test in dev mode),
i've put your README formatting changes here already: #206 .

Changing the Filtermenu , adding preview on hover and reworking filters to use Imagemagick (not sure if a pi is the best device for imagemagick, was quite slow once i tested) could be done in seperate PR's.

@andi34
Copy link
Collaborator

andi34 commented Dec 23, 2019

@rawbertp could you please rebase on latest master branch? ( andi34@c7ae73f can be picked an pushed )

For now i'd say it works as it should. Improvements can be made later on another PR if needed.

@rawbertp
Copy link
Contributor Author

rawbertp commented Jan 3, 2020

Done @andi34 .

I agree with both of you - the overlay doesn't look especially great but it is the same as it used to be before. So yes - it should be changed at some stage but I wouldn't consider this as an essential part of this PR either.

@rawbertp rawbertp changed the title [WIP] Apply filter on picture taken Apply filter on picture taken Jan 3, 2020
@andi34 andi34 merged commit 6c2396c into andreknieriem:master Jan 3, 2020
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.

3 participants