extragear/graphics/digikam

[Marcel Wiesweg]

SVN commit 498926 by mwiesweg:

digikam from trunk: threaded image loading

- threaded image loading
   - add SharedLoadSaveThread class
   - split declarations to different header files
   - add LoadingCache class
   - sharing of loading processes: if an image is currently being loaded in another
     SharedLoadSaveThread, a second thread will wait for the first to finish
     instead of loading it twice
   - caching of images: loaded images are cached and available to subsequent requests
   This should eliminate any unnecessary work.
- Histogram tab
   - adapt to shared loading
   - remove some repaints
   - painting progress message only after timeout value (100 ms), much snappier now
   - fixed display of loading failure
- Image Editor and Showfoto
   - adapt DImgInterface to threaded image loading
   - add appropriate signals and slots to all three layers (DImgInterface, Canvas, ImageWindow and Showfoto)
   - split loading and saving functions in two parts in all three layers
   - for saving, some information needs to be stored across asynchronous calls which
     was previously stored in local variables.
     Added appropriate members respectively SavingContext classes (in ImageWindow, Showfoto).
   - put in hooks for progress info display
   - added preliminary code to wait on asynchronous saving, needed for promptUserSave()
       - added a modal progress dialog
       - needs to be tested and fixed, see TODO below
   - fix creation of ImageWindow singleton upon exiting
     (checking for imageWindow() would create the instance if not yet done)

TODO:
   - add progress display to status bar
   - disable actions when saving (all actions?) and loading (not all actions,
     e.g. selecting another image must be possible and abort loading)
   - test, think about and fix waiting on save() in promptUserSave()
   - more thorough testing
   - fix a bug when fast switching through list in IE. First add disabling of actions.
   - still some debug statements active

CCMAIL: [hidden email]



 M  +2 -2      digikam/digikamapp.cpp  
 M  +18 -16    libs/imageproperties/imagepropertiescolorstab.cpp  
 M  +1 -1      libs/imageproperties/imagepropertiescolorstab.h  
 M  +1 -1      libs/threadimageio/Makefile.am  
 A             libs/threadimageio/loadingcache.cpp   [License: GPL]
 A             libs/threadimageio/loadingcache.h   [License: GPL]
 A             libs/threadimageio/loadingdescription.h   [License: GPL]
 M  +336 -36   libs/threadimageio/loadsavethread.cpp  
 M  +32 -79    libs/threadimageio/loadsavethread.h  
 A             libs/threadimageio/managedloadsavethread.h   [License: GPL]
 A             libs/threadimageio/sharedloadsavethread.h   [License: GPL]
 M  +10 -3     libs/widgets/histogramwidget.cpp  
 M  +2 -1      libs/widgets/histogramwidget.h  
 M  +202 -91   showfoto/showfoto.cpp  
 M  +11 -0     showfoto/showfoto.h  
 M  +1 -0      utilities/imageeditor/canvas/Makefile.am  
 M  +46 -25    utilities/imageeditor/canvas/canvas.cpp  
 M  +12 -4     utilities/imageeditor/canvas/canvas.h  
 M  +94 -42    utilities/imageeditor/canvas/dimginterface.cpp  
 M  +14 -4     utilities/imageeditor/canvas/dimginterface.h  
 M  +1 -0      utilities/imageeditor/editor/Makefile.am  
 M  +262 -122  utilities/imageeditor/editor/imagewindow.cpp  
 M  +11 -0     utilities/imageeditor/editor/imagewindow.h  


_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Marcel Wiesweg]

Hi,

I just put this commit into SVN, see the message on the mailing list for
complete changelog.
I think I have to elaborate on some points.


> - Image Editor and Showfoto
>    - adapt DImgInterface to threaded image loading
>    - add appropriate signals and slots to all three layers (DImgInterface,
> Canvas, ImageWindow and Showfoto) - split loading and saving functions in
> two parts in all three layers - for saving, some information needs to be
> stored across asynchronous calls which was previously stored in local
> variables.
>      Added appropriate members respectively SavingContext classes (in
> ImageWindow, Showfoto).

I just split the function in two parts and made the local variables which are
used in the second part member variables. In ImageWindow and Showfoto, where
there are more than one such variable, I introduced a class which bundles
them to keep things clear. Neither the naming of these variables nor the
functions as such have been changed.

>    - put in hooks for progress info display
>    - added preliminary code to wait on asynchronous saving, needed for
> promptUserSave() - added a modal progress dialog
>        - needs to be tested and fixed, see TODO below

This is a problem I had not thought of. In promptUserSave(), a return value is
needed from the save() functions. As far as I see, at least when
promptUserSave is called from closeEvent(), this cannot be avoided.
So we need to wait in this function till slotSavingFinished is called,
essentially this means waiting for event delivery in the event queue.

My Qt experience ends here, I am lacking a deeper understanding for things
like enter_loop, qt_enter_modal, modal dialogs and such. I just put in a
modal dialog which is closed in finishSaving in the hope that this works, but
this needs testing.

It is probably better to use the standard, still-to-be-written progress
display and use some other means for synchronizing, something like Digikam
SyncJob?


>    - fix creation of ImageWindow singleton upon exiting
>      (checking for imageWindow() would create the instance if not yet done)
>
> TODO:
>    - add progress display to status bar
>    - disable actions when saving (all actions?) and loading (not all
> actions, e.g. selecting another image must be possible and abort loading) -
> test, think about and fix waiting on save() in promptUserSave() - more
> thorough testing

There are TODOs in the relevant places (imagewindow.cpp, showfoto.cpp)
When saving, no other actions should be possible IMO.
Loading can safely be aborted, so next/previous buttons and the thumb bar
should be usable. On the other nything that changes image data, starting
image plugins etc. should not be possible.

>    - fix a bug when fast switching through list in IE. First add disabling
> of actions.

I have found a bug there probably, still unsure where but I don't think it is
in the infrastructure. Will fix this when the disabling of actions is done
and there is always a clean state, and the bug still appears.

> - still some debug statements active
>

Marcel
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier]

Le Lundi 16 Janvier 2006 19:07, vous avez écrit :

> Hi,
>
> I just put this commit into SVN, see the message on the mailing list for
> complete changelog.
> I think I have to elaborate on some points.
>
> > - Image Editor and Showfoto
> >    - adapt DImgInterface to threaded image loading
> >    - add appropriate signals and slots to all three layers
> > (DImgInterface, Canvas, ImageWindow and Showfoto) - split loading and
> > saving functions in two parts in all three layers - for saving, some
> > information needs to be stored across asynchronous calls which was
> > previously stored in local variables.
> >      Added appropriate members respectively SavingContext classes (in
> > ImageWindow, Showfoto).

ImageWindowSavingContext will be common to showfoto and IE. To do a clean
implementation, I recommend you to put ImageWindowSavingContext class
implementation to a separate header file, like iofilesettingscontainer class.

Maintening showfoto and IE GUI implementation is infernal. I don't know why
Renchi haven't created a common class for that. There are a lot of common
code in both.

We need talking about this point for the future... Joern, Tom, what's do you
think about ?

>
> I just split the function in two parts and made the local variables which
> are used in the second part member variables. In ImageWindow and Showfoto,
> where there are more than one such variable, I introduced a class which
> bundles them to keep things clear. Neither the naming of these variables
> nor the functions as such have been changed.
>
> >    - put in hooks for progress info display
> >    - added preliminary code to wait on asynchronous saving, needed for
> > promptUserSave() - added a modal progress dialog
> >        - needs to be tested and fixed, see TODO below

ok. I will test it indeep...

something is broken in current implementation about saving : the result file
still with the temporary file name, not the target filename.

>
> This is a problem I had not thought of. In promptUserSave(), a return value
> is needed from the save() functions. As far as I see, at least when
> promptUserSave is called from closeEvent(), this cannot be avoided.
> So we need to wait in this function till slotSavingFinished is called,
> essentially this means waiting for event delivery in the event queue.
>
> My Qt experience ends here, I am lacking a deeper understanding for things
> like enter_loop, qt_enter_modal, modal dialogs and such. I just put in a
> modal dialog which is closed in finishSaving in the hope that this works,
> but this needs testing.
>
> It is probably better to use the standard, still-to-be-written progress
> display and use some other means for synchronizing, something like Digikam
> SyncJob?

... I haven't yet study SyncJob implementation. I will take a look.

>
> >    - fix creation of ImageWindow singleton upon exiting
> >      (checking for imageWindow() would create the instance if not yet
> > done)
> >
> > TODO:
> >    - add progress display to status bar
> >    - disable actions when saving (all actions?) and loading (not all
> > actions, e.g. selecting another image must be possible and abort loading)
> > - test, think about and fix waiting on save() in promptUserSave() - more
> > thorough testing
>
> There are TODOs in the relevant places (imagewindow.cpp, showfoto.cpp)
> When saving, no other actions should be possible IMO.
> Loading can safely be aborted, so next/previous buttons and the thumb bar
> should be usable. On the other nything that changes image data, starting
> image plugins etc. should not be possible.

yes. There is already a method in showfoto.cpp to disable all menu actions
when there is no current image loaded. you need to update this method and
backport it in imagewindow.cpp

But i'm back about a common IE/showfoto GUI class implementation. I think this
is the right way instead to duplicate source code in IE and showfoto...

>
> >    - fix a bug when fast switching through list in IE. First add
> > disabling of actions.
>
> I have found a bug there probably, still unsure where but I don't think it
> is in the infrastructure. Will fix this when the disabling of actions is
> done and there is always a clean state, and the bug still appears.
>

ok
--
Gilles
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier]

Le Mardi 17 Janvier 2006 19:58, vous avez écrit :

> > ImageWindowSavingContext will be common to showfoto and IE. To do a clean
> > implementation, I recommend you to put ImageWindowSavingContext class
> > implementation to a separate header file, like iofilesettingscontainer
> > class.
> >
> > Maintening showfoto and IE GUI implementation is infernal. I don't know
> > why Renchi haven't created a common class for that. There are a lot of
> > common code in both.
> >
> > We need talking about this point for the future... Joern, Tom, what's do
> > you think about ?
>
> YES, please!
>
> :-)

No response. I will trying to contact Tom and Joern by IRC.

About IRC, have you trying #digikam channel :

http://www.digikam.org/?q=contact

>
> The common code probably increased a lot with the addition of right side
> bar and thumb bar. The current state is unmaintainable. Things happen like
> ImageWindow using KTempFile, while Showfoto creates its own file name. So
> - canvas
> - right side bar
> - thumb bar
> - loading and saving
> and a lot of other stuff can be done in a common class.
>
> Some other things like managing the list of URLs, the current picture are
> specific, but this seems to be a small subset. However, I am not familiar
> with any pitfalls in this code, both files are large, complex and not easy
> to get a complete view of.
>

I'm completly agree with you. I will take a look to create a comon GUI class
for IE and showfoto. Until this new class will be created, we need to
continue with the curren timplementation.

> > > I just split the function in two parts and made the local variables
> > > which are used in the second part member variables. In ImageWindow and
> > > Showfoto, where there are more than one such variable, I introduced a
> > > class which bundles them to keep things clear. Neither the naming of
> > > these variables nor the functions as such have been changed.
> > >
> > > >    - put in hooks for progress info display
> > > >    - added preliminary code to wait on asynchronous saving, needed
> > > > for promptUserSave() - added a modal progress dialog
> > > >        - needs to be tested and fixed, see TODO below
> >
> > ok. I will test it indeep...
> >
> > something is broken in current implementation about saving : the result
> > file still with the temporary file name, not the target filename.
> >
> > > This is a problem I had not thought of. In promptUserSave(), a return
> > > value is needed from the save() functions. As far as I see, at least
> > > when promptUserSave is called from closeEvent(), this cannot be
> > > avoided. So we need to wait in this function till slotSavingFinished is
> > > called, essentially this means waiting for event delivery in the event
> > > queue.
> > >
> > > My Qt experience ends here, I am lacking a deeper understanding for
> > > things like enter_loop, qt_enter_modal, modal dialogs and such. I just
> > > put in a modal dialog which is closed in finishSaving in the hope that
> > > this works, but this needs testing.
> > >
> > > It is probably better to use the standard, still-to-be-written progress
> > > display and use some other means for synchronizing, something like
> > > Digikam SyncJob?
> >
> > ... I haven't yet study SyncJob implementation. I will take a look.
>
> There is a dummy widget created and some magic is done with qt_enter_loop.
> As I said, I have no experience of this, and there is no documentation.
>

I need any time to study this implementation (:=)))
--
Gilles
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel