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

Fresh image processing #258

Closed
wants to merge 24 commits into from
Closed

Conversation

zamazan4ik
Copy link
Collaborator

Hello @manisandro .

I have createdfresh branch for image processing stuff. Can you check it please?

@@ -65,6 +65,7 @@ public:

public slots:
void setAngle(double angle);
void setScaledImage(const QImage& image, double scale = 1.0);
Copy link
Owner

Choose a reason for hiding this comment

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

scaled image is just the image used in the UI, not the one used when recognizing. See Displayer::getImage.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's just for testing. As far as i remember from our previous discuss about it, i should extend interface of Displayer.

This is actually non-trivial. The Displayer always queries the images directly from the source (i.e. an image file or pdf or djvu document) with the appropriate resolution, either for displaying in the gui, or for passing to tesseract for OCR. The getImage image method is only there for selection tools etc which need to operate on the currently displayed image. Conversely, a setImage method would make little sense because you would only be setting the image used for the UI at the current zoom level etc.
I see two options:
Parametrize the processing steps, so that they are applied to each rendered image, as is done for brightness, contrast etc, see
https://github.com/manisandro/gImageReader/blob/master/qt/src/DisplayRenderer.cc#L31
But I fear this might be too slow.
Otherwise you'd need to have the processing algorithms create a new temporary image, which is then passed to the displayer as a source. To make it elegant, you could actually extent the Source structure
https://github.com/manisandro/gImageReader/blob/master/qt/src/SourceManager.hh#L32
to allow specifying an alternative path to use instead of the path actually specified by the source, and if that path is not empty, then that image is used instead. This has the benefit that you don't need to pollute the source list in the UI with extra entries for the processing output. This alternative path entry would probably have to be a
QMap<int, QString>
since for multi-page documents you will need a temporary image for each page. For multi-page there is the additional challenge when to generate these temproary images, since generating them all at once will probably be to slow. Probably better to just generate them on demand.
I hope this makes sense to you.

Copy link
Owner

Choose a reason for hiding this comment

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

You'll probably want to do something similar to DisplayRenderer::adjustImage.

Copy link
Owner

Choose a reason for hiding this comment

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

Actually it depends on what operation you are performing, if they are operations which take a long time and should not be repeated everytime the image is redrawn, then you'll probably want to create a new temporary image with the operations applied, and then use that as source.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hah, it's quite difficult to choose, because different image operations have different duration: from milliseconds to seconds. E.g. denoising is very expensive operation.

then you'll probably want to create a new temporary image with the operations applied, and then use that as source.

Create on disk and then reload from disk?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You can look into image_processing sources: i use for it Doxygen.

Copy link
Owner

Choose a reason for hiding this comment

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

I'm familiar with doxygen, but again, I'm not convinced it helps much here besides blowing up the size of the source files.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

About SourceManager::addSource method: but in this case as fasr as i understand my temporary item will be added to QListWidgetItem. Is it ok?

Copy link
Owner

Choose a reason for hiding this comment

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

Another idea could be to extend the Source struct by adding a QMap<int,QString> preprocessedImages, where you store, for each page, the preprocessed image. If such a page exists, the Displayer will render that image instead of the one of the original source. You can then give the user the option to revert to the original image, in which case the entry in preprocessedImages is simply discarded. This approach would also make it easy to apply the algorithms to all pages of a multipage document, without polluting the sources list with tons of entries.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I like this idea.

@zamazan4ik
Copy link
Collaborator Author

Also please check UI. Now it's simple set of buttons for performing different algorithms.

@manisandro
Copy link
Owner

How do you envison the UI in the releasable version? If they remain simple button, I suppose a combobox with the respective entries in the advanced image controls toolbar would work just as well.

@manisandro
Copy link
Owner

Also, how likely is that an algorithm will "wreck" an image? If that can happen, you'll either need a preview or a undo/redo. Preview is easier I suppose ;)

@zamazan4ik
Copy link
Collaborator Author

Also, how likely is that an algorithm will "wreck" an image? If that can happen, you'll either need a preview or a undo/redo. Preview is easier I suppose ;)

Of course, always there is some chance to make error in image processing algorithm :-)

This question is very important and should be discussed. I see several ways to deal with it:

  1. Every button apply algorithm on preview. And after some steps user can press "Apply" button and image from preview will replace source image.
  2. You way with Ctrl+Z/Ctrl+Y. I think we also MUST implement it. It's very convinient way for program. I as user want to have this feature in gIR.
  3. After every image processing algorithm ask to user about "Apply result from preview to source image?". I don't like this way :-)

@zamazan4ik
Copy link
Collaborator Author

How do you envison the UI in the releasable version? If they remain simple button, I suppose a combobox with the respective entries in the advanced image controls toolbar would work just as well.

I want to clone UI from Abbyy FineReader to gIR. And in FineReader they have separate dock widget for image processing.

Image proccesing UI will not be easy buttons. We will extend it with some customizable features: e.g. user must to be able to regulate denoise power, because we can't do it automatically.

@manisandro
Copy link
Owner

For a start I'd just add a notification bar asking the user whether he wants to apply the change.

@zamazan4ik
Copy link
Collaborator Author

Do you have any other warnings/suggestions?

@manisandro
Copy link
Owner

Nope, with correct source logic and polished UI should be good!

@zamazan4ik
Copy link
Collaborator Author

Ok, nice. Thank you for the review :-)

@manisandro
Copy link
Owner

Hi @zamazan4ik, I've finally ended up releasing 3.3.0, so we can start again with feature work, if you are still interested in pursuing this.

@manisandro
Copy link
Owner

Closing since this appears to be abbandoned.

@manisandro manisandro closed this Jul 24, 2019
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