917445 - [Gallery] Automatic Image Enhancement

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Gallery, defect)

Description

[Tom Herold]

There should be an option to auto enhance images as featured in many other gallery/image applications.
One simple way of doing this is to stretch an image's histogram (sometimes called histogram normalization) by scaling the minimum and maximum color values of an image to the [0,255] range.

A simple implementation of this auto levels feature are outlined here:
http://www.ipol.im/pub/art/2011/llmps-scb/
http://scien.stanford.edu/pages/labsite/2010/psych221/projects/2010/JasonSu/simplestcb.html

Comment 1

[Tom Herold]

A first prototype can be found at http://hotzenklotz.github.io/color-balance/

Comment 2

[Tom Herold]

@rob: Right now, I have a first implementation that let's you auto enhance images with a one button solution. It works without breaking exposure correction, color filters, borders etc. However, if you decide to use auto enhance, it will always apply this image correction first and then applied whatever other effect the user choose. This works well, with cropping, color filters and borders. Exposure is a bit trickier, since the exposure is added on top of the image enhancement and maybe apply more of the effect than was called for. This is isn't really an issue, just something to keep in mind.

Yet, what I still need is a solution for the button just performing one action and not needing any parameters and hence the extra tab menu. Diego already suggested having a discussion for this as well. (in his case for image rotation)

Comment 3

[John Hu [:johnhu][:johu][:醬糊小弟]]

(In reply to Tom Herold from comment #1)
> A first prototype can be found at http://hotzenklotz.github.io/color-balance/

This feature is so cool. Nice job, Tom.

Comment 4

[Tom Herold]

Attachment: Github Pull Request

Comment 5

[David Flanagan [:djf]]

Comment on attachment 814616 [details]
Github Pull Request

Tom,

I've noted some issues on Github.  I think there are some issues with the min/max computation and that performance could be slightly improved by only computing the cumulative parts you need.

Also, I think sometimes your code will end up calling edit() before the latest histogram values have been returned by the worker.  

And I think you can compute the histogram when you first create the preview image rather than decoding the preview into a new canvas just for that step.

Comment 6

[Tom Herold]

@Rob: This is a friendly reminder, that I am still waiting for an icon for this. Also, can you please remind me again what we agreed on regarding the "parameter" tab that will not be used for this feature. Thank you.

Comment 7

[Diego Marcos [:dmarcos]]

I have some input on this. I realized that the magic wand icon we use now for the filters menu is usually used for the auto enhance feature (in android and ios). I think we should reuse the magic wand for auto enhance and create a new icon for the filters menu. What do you think?

Comment 8

[Tom Herold]

I agree with Diego here. That is one of the reasons I used in the first place. IRRC Hema also voted for a magic wand.

Comment 9

[Tom Herold]

Attachment: undo.gif

Adding a dedicated "undo" button makes sense. I found the clicking the "enhancement" button multiple times to toggle the effect on and off to be confusing. Additionally, this way there is some use for the "parameter" tab bar that was previously unused.

Comment 10

[David Flanagan [:djf]]

Comment on attachment 814616 [details]
Github Pull Request

The patch seems to work nicely. But your changes to the worker today have some problems.  See my github comments.

Also consider making the "auto enhance on" touch sensitive so the user can toggle by tapping the button or the message.

Comment 11

[Tom Herold]

Comment on attachment 814616 [details]
Github Pull Request

I implemented the UI according to the design spec but used my own home made icons for the moment. I filed a follow up bug to replace these: 933066

I changed the worker to include the single channel threshold condition that you proposed. I don't know if a threshold of 50 is the right amount but I find it a reasonable assumption.

Comment 12

[David Flanagan [:djf]]

Comment on attachment 814616 [details]
Github Pull Request

There is one more problem in the worker code. See github.

Also note that using "on" in the icon isn't localizable. Even though these are just placeholders, someone will probably file a bug about it.  So if you've got time, please consider changing that icon to remove the text.  Maybe the on icon could have the stars coming out of the wand and off would just be the wand without the trail of stars?

Please fix those two things and land it!

And congratulations on a great internship!

Comment 13

[David Flanagan [:djf]]

Also, the travis build is failing. It appears unrelated, but it would be nice to have a solid green before landing this.

Comment 14

[David Flanagan [:djf]]

Actually, since you taught me how to restart Travis builds, I've restarted it. Hopefully the failed test was transient.

Comment 15

[Tom Herold]

(In reply to David Flanagan [:djf] from comment #12)
> Comment on attachment 814616 [details]
> Github Pull Request
> 
> There is one more problem in the worker code. See github.
> 
> Also note that using "on" in the icon isn't localizable. Even though these
> are just placeholders, someone will probably file a bug about it.  So if
> you've got time, please consider changing that icon to remove the text. 
> Maybe the on icon could have the stars coming out of the wand and off would
> just be the wand without the trail of stars?
> 
> Please fix those two things and land it!
> 
> And congratulations on a great internship!

You make a good point here with the wand icon. I just followed the UI teams's specs. I will inform Rob about it as well.

Comment 16

[Tom Herold]

Attachment: screencast.gif

I added a screen cast to show the wand icon without the "on" text and use the sparks instead to represent the current status.

Comment 17

[Tom Herold]

Merged.
https://github.com/mozilla-b2g/gaia/commit/a6a0438df14ecf92a587793ed2dbf073e5131d15

Comment 18

[Marcia Knous [:marcia]]

https://moztrap.mozilla.org/manage/case/11267/ added to Moztrap to cover this test.