Bug 110514

Summary: Enhanced selection, refactored histogram
Product: [Applications] digikam Reporter: gsasha
Component: ImageEditor-CanvasAssignee: Digikam Developers <digikam-bugs-null>
Status: RESOLVED INTENTIONAL    
Severity: wishlist CC: caulier.gilles, lure
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: Compiled Sources   
OS: Linux   
Latest Commit: Version Fixed In: 0.9.1
Attachments: Patch against the Aug 9 SVN version
New version of the patch
Now including the rect_controller

Description gsasha 2005-08-10 16:51:01 UTC
Version:            (using KDE KDE 3.4.2)
Installed from:    Compiled From Sources
OS:                Linux

I have done several enhancements to the Image Editor. Changes include:

1. Enhanced selection, with sane behavior of moving, resizing, etc. The implementation sits outside of the Canvas, and is reusable.
2. Refactored out the Histogram. Now the rendering sits in one class, and moving - in another (I will probably reuse Selector's code for this if resizing of the Histogram is required). As a result, the code is much simpler and more straightforward.
3. Removed the separate thread used for computing the Histogram. It doesn't look like it was improving the responsibility a lot, but made the whole thing more complicated. (receiving a CustomEvent as notification is way too ugly to my understanding).
4. Added a new Histogram mode, RGB (currently it's what is shown when Alpha is selected.
5. Changed the rendering of Histogram, no flicker now.

The [current version of] patch can be found at <a href="http://www.cs.technion.ac.il/~gsasha/selectorv4.diff">http://www.cs.technion.ac.il/~gsasha/selectorv4.diff</a>.
I will also provide the diff as an attachment here.
Comment 1 gsasha 2005-08-10 16:56:22 UTC
Created attachment 12167 [details]
Patch against the Aug 9 SVN version
Comment 2 Owen Hirst 2005-08-10 17:01:31 UTC
Hello, Your patch is still incomplete. It is missing the histogram_renderer.[cpp/h] files
Comment 3 gsasha 2005-08-10 19:18:00 UTC
Created attachment 12168 [details]
New version of the patch

Added a new attachment, this time including the missing files
Comment 4 Owen Hirst 2005-08-10 19:52:11 UTC
Now it seems to be missing rect_controller.cpp/h :)
Comment 5 gsasha 2005-08-10 20:08:26 UTC
Created attachment 12169 [details]
Now including the rect_controller

GRRR. Is there a way in SVN, like in CVS, to check for all unknown files?
Comment 6 caulier.gilles 2005-08-11 15:59:30 UTC
-Point 1 : good sound... Implementation not yet checked !

-Point 2 : Take a care to use common ImageHistogram implementation.

-Point 3 : Definitivly NO. You need to use a separate thread againsty GUI thread. If you want to compute an histogram from a very large image (8/10 MPix for example), image editor will be frozen. Always use Qthread in this case. You can use my ThreadedImageFilter implementation in libs/imagefilter for example. Note that Qt4 will simplify QThread management using signal and slots ! Qthread will be derived from QObject.

-Point 4 : Add this mode to the common implementation, and updated all other histogram rendering accordingly. digiKam must be stay homogenous! I recommend you to remove temporally this implementation.

-Point 5 : Good sound.

In General (implementation just checked, not compiled):

- In imageHistogram.cpp : no need to recreate histogram computation implementation with your ImageHistogramData class... You must use the current implementation like it's used by other digiKam/DigikamImagePlugins parts (try grep for more info).

Take a care : The current histogram implementation is common to digiKam. If you want add more methods, patch current ImageHistogram implementation.

- Don't forget to use Digikam namespace.

- Never use std::cerr, use Kdebug instead.

- In the future, digiKam will be support 16/bits/color/pixels image file format using a new embedded framework named DImage instead QImage/imlib2. It's important to use a common implementation about histogram computation. 

- Why not using my histogram widget to rendering histogram on image editor canvas ? It's a common object...

- Don't forget to try your implementation with ShowFoto.

Gilles
Comment 7 caulier.gilles 2005-08-11 16:00:07 UTC
-Point 1 : good sound... Implementation not yet checked !

-Point 2 : Take a care to use common ImageHistogram implementation.

-Point 3 : Definitivly NO. You need to use a separate thread againsty GUI thread. If you want to compute an histogram from a very large image (8/10 MPix for example), image editor will be frozen. Always use Qthread in this case. You can use my ThreadedImageFilter implementation in libs/imagefilter for example. Note that Qt4 will simplify QThread management using signal and slots ! Qthread will be derived from QObject.

-Point 4 : Add this mode to the common implementation, and updated all other histogram rendering accordingly. digiKam must be stay homogenous! I recommend you to remove temporally this implementation.

-Point 5 : Good sound.

In General (implementation just checked, not compiled):

- In imageHistogram.cpp : no need to recreate histogram computation implementation with your ImageHistogramData class... You must use the current implementation like it's used by other digiKam/DigikamImagePlugins parts (try grep for more info).

Take a care : The current histogram implementation is common to digiKam. If you want add more methods, patch current ImageHistogram implementation.

- Don't forget to use Digikam namespace.

- Never use std::cerr, use Kdebug instead.

- In the future, digiKam will be support 16/bits/color/pixels image file format using a new embedded framework named DImage instead QImage/imlib2. It's important to use a common implementation about histogram computation. 

- Why not using my histogram widget to rendering histogram on image editor canvas ? It's a common object...

- Don't forget to try your implementation with ShowFoto.

Gilles Caulier
Comment 8 alfons.hoogervorst 2005-08-13 10:41:42 UTC
interesting. I'll take a look at this too. 
Comment 9 Luka Renko 2007-01-28 11:03:37 UTC
SVN commit 627802 by lure:

Remove View->Histogram (blended histogram) from Digikam Editor and Showfoto
(histogram is now avalable in sidebar with more functionality)

Move Full screen and Slideshow in the same group in View menu to match
the same grouping used in toolbar.

CCBUG: 110514


 M  +2 -4      showfoto/showfotoui.rc  
 M  +1 -536    utilities/imageeditor/canvas/canvas.cpp  
 M  +0 -19     utilities/imageeditor/canvas/canvas.h  
 M  +2 -5      utilities/imageeditor/editor/digikamimagewindowui.rc  
 M  +0 -41     utilities/imageeditor/editor/editorwindow.cpp  
 M  +0 -1      utilities/imageeditor/editor/editorwindow.h  
 M  +0 -3      utilities/imageeditor/editor/editorwindowprivate.h  
Comment 10 Luka Renko 2007-01-28 11:04:58 UTC
View->Histogram code was removed as Digikam has now full featured histogram in the sidebar. This makes this improvement obsolete.