Bug 377197 - Customize grouping behaviour [patch]
Summary: Customize grouping behaviour [patch]
Status: RESOLVED FIXED
Alias: None
Product: digikam
Classification: Applications
Component: Albums-ItemGroup (show other bugs)
Version: 5.5.0
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Digikam Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-03-04 13:12 UTC by Simon
Modified: 2017-05-12 23:04 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.6.0
Sentry Crash Report:


Attachments
Patch to customize grouping behaviour (87.89 KB, patch)
2017-03-04 13:12 UTC, Simon
Details
Patch to customize grouping behaviour - 2 (109.01 KB, patch)
2017-03-10 10:56 UTC, Simon
Details
Deduplicate code between icon and table view (46.12 KB, patch)
2017-03-18 18:22 UTC, Simon
Details
Customize grouping behavior patch version 3 (108.88 KB, patch)
2017-03-18 19:43 UTC, caulier.gilles
Details
Deduplicate code between icon and table view (V2) (47.20 KB, patch)
2017-03-18 23:43 UTC, Simon
Details
Customize grouping behavior patch version 4 (110.06 KB, patch)
2017-03-18 23:44 UTC, Simon
Details
Customize grouping behavior patch version 5 (109.98 KB, patch)
2017-03-19 08:14 UTC, caulier.gilles
Details
Deduplicate code between icon and table view (V3) (47.22 KB, patch)
2017-03-19 17:13 UTC, Simon
Details
Deduplicate code between icon and table view (V4) (47.21 KB, patch)
2017-03-21 23:31 UTC, Simon
Details
Deduplicate code between icon and table view (V5) (47.21 KB, patch)
2017-03-24 09:19 UTC, Simon
Details
Customize grouping behavior patch version 6 (110.10 KB, patch)
2017-03-24 09:22 UTC, Simon
Details
Customize grouping behavior patch version 7 (110.22 KB, patch)
2017-03-31 09:57 UTC, Simon
Details
Deduplicate code between icon and table view (V6) (47.21 KB, patch)
2017-03-31 09:59 UTC, Simon
Details
Customize grouping behavior patch version 8 (110.24 KB, patch)
2017-04-13 22:26 UTC, Simon
Details
Customize grouping behavior patch version 9 (132.75 KB, patch)
2017-05-03 20:45 UTC, Simon
Details
Customize grouping behavior patch version 10 (132.77 KB, patch)
2017-05-08 12:36 UTC, Simon
Details
grouping.png (81.20 KB, image/png)
2017-05-08 16:32 UTC, Maik Qualmann
Details
Customize grouping behavior patch version 11 (124.43 KB, patch)
2017-05-11 16:46 UTC, caulier.gilles
Details
Customize grouping behavior patch version 12 (131.15 KB, patch)
2017-05-11 20:36 UTC, caulier.gilles
Details
Customize grouping behavior patch version 12 (139.12 KB, patch)
2017-05-11 20:37 UTC, Simon
Details
Customize grouping behavior patch version 13 (151.59 KB, patch)
2017-05-12 19:10 UTC, Simon
Details
Adjust kipi-plugins for grouping (4.55 KB, patch)
2017-05-12 19:11 UTC, Simon
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Simon 2017-03-04 13:12:10 UTC
Created attachment 104362 [details]
Patch to customize grouping behaviour

This extends the changes made in https://bugs.kde.org/show_bug.cgi?id=372027
As requested in the user mailing list, neither the old behaviour (disregard grouping) nor the behaviour introduces in the issue linked above (always operate on all group items) is ideal for all circumstances/users.

This patch makes it customizable for operations where it makes sense/is possible. The behaviour is specifiable in the configuration in the "Miscellaneous" part. However this does not need to be specified manually by the user at all. By default these are set to "Ask", meaning a dialogue will pop up when an operation is run. There one can choose whether the operation should happen on all items in the group or not and a checkbox to remember the choice is provided.

Also I found some dead/duplicate code related to icon and table view, which I centralized. In the end the whole patch got quite big. I can push it to a branch with the refactoring and grouping related code separated if you want.
Comment 1 Maik Qualmann 2017-03-08 19:21:45 UTC
Simon,

I think we do not need that much. A simple "use first image of grouped or use all images" for all actions should be enough. Let's see what Gilles says when he has time. I have also found some errors on the right mouse button context menu:

- Assign color label to a group, the dialog asks for each image
- Assign or remove tags no longer works
- Assign rating no longer works
- Assign flags no longer works

Maik
Comment 2 caulier.gilles 2017-03-08 21:23:42 UTC
Simon,

Curiously, i never seen this entry until Maik comment it.

So, i will take a look when time permit, i hope while this week end...

Gilles Caulier
Comment 3 Simon 2017-03-09 20:17:58 UTC
Yes, context menu is obviously broken. I tested all kinds of things but
for unknown reasons never once did a right click. This code is also
heavily duplicated between table and icon view, I am thus refactoring
while fixing.

On 08/03/17 20:21, Maik Qualmann wrote:
> Simon,
>
> I think we do not need that much. A simple "use first image of grouped or use
> all images" for all actions should be enough. Let's see what Gilles says when
> he has time. I have also found some errors on the right mouse button context
> menu:
>
> - Assign color label to a group, the dialog asks for each image
> - Assign or remove tags no longer works
> - Assign rating no longer works
> - Assign flags no longer works
>
> Maik
>
Comment 4 Simon 2017-03-10 10:56:49 UTC
Created attachment 104490 [details]
Patch to customize grouping behaviour - 2

This is an actualized patch squashing some more duplicated code and fixing the issues mentioned by Maik.
Comment 5 Simon 2017-03-18 18:22:52 UTC
Created attachment 104632 [details]
Deduplicate code between icon and table view

This is the part of the patch that is not actually directly related to grouping. I just needed this to get consistent behaviour between the two views and to get rid of duplicate code. In my opinion it is cleaner and much less error prone. It probably makes sense if you first look at this and when this is committed we can look at the grouping stuff.
Comment 6 caulier.gilles 2017-03-18 18:59:28 UTC
Yes good improvement in source code.

Can you reproduce the icon/widget overlays on icon view, when you enable all options from setup and when you change the icon size from status bar ?

Also turn on Hdpi support for icons view (thumb size > 256)...

Gilles
Comment 7 caulier.gilles 2017-03-18 19:03:25 UTC
Note : it's voluntary to not include "Deduplicate" patch in "Customize" patch ?

Do you want to apply "Deduplicate" patch quickly in git/master ?

Gilles
Comment 8 caulier.gilles 2017-03-18 19:05:29 UTC
[gilles@localhost core]$ patch -p1 < customizeGrouping-2.patch 
patching file app/items/digikamimageview.cpp
patching file app/items/digikamimageview.h
patching file app/items/imagecategorizedview.cpp
patching file app/items/imagecategorizedview.h
patching file app/main/digikamapp.cpp
patching file app/utils/contextmenuhelper.cpp
patching file app/views/digikamview.cpp
patching file app/views/digikamview.h
patching file app/views/tableview/tableview.cpp
patching file app/views/tableview/tableview.h
patching file app/views/tableview/tableview_model.cpp
patching file app/views/tableview/tableview_model.h
patching file libs/models/imagefiltermodel.cpp
patching file libs/settings/applicationsettings.cpp
patching file libs/settings/applicationsettings.h
Hunk #2 succeeded at 463 (offset 3 lines).
patching file libs/settings/applicationsettings_miscs.cpp
Hunk #2 succeeded at 288 (offset 11 lines).
patching file libs/settings/applicationsettings_p.cpp
Hunk #3 succeeded at 236 (offset 1 line).
Hunk #4 succeeded at 366 (offset 2 lines).
patching file libs/settings/applicationsettings_p.h
Hunk #3 succeeded at 285 (offset 1 line).
patching file utilities/setup/setupmisc.cpp
patching file utilities/setup/setupmisc.h

[gilles@localhost core]$ patch -p1 < viewRefactor.patch 
patching file app/items/digikamimageview.cpp
Reversed (or previously applied) patch detected!  Assume -R? [n] 

Why ?

Gilles
Comment 9 Maik Qualmann 2017-03-18 19:07:35 UTC
Simon,

Short test here, cut, copy, paste, delete to the trash no longer works.

Maik
Comment 10 Maik Qualmann 2017-03-18 19:08:40 UTC
I have patch 104632 tested.

Maik
Comment 11 caulier.gilles 2017-03-18 19:43:02 UTC
Created attachment 104633 [details]
Customize grouping behavior patch version 3

Updated patch to compile with git/master.
Comment 12 caulier.gilles 2017-03-18 19:47:47 UTC
Simon, 

With Customize patch applied, i can see this on the console :

digikam.general: Cancel Main Thread
QObject::connect: No such slot Digikam::DigikamView::deleteSelected() in /home/gilles/Devel/5.x/core/app/views/digikamview.cpp:2594
QObject::connect: No such signal Digikam::ContextMenuHelper::signalslotCreateGroupByTime() in /home/gilles/Devel/5.x/core/app/views/digikamview.cpp:2655
QObject::connect: No such signal Digikam::ContextMenuHelper::signalslotCreateGroupByFilename() in /home/gilles/Devel/5.x/core/app/views/digikamview.cpp:2658
QObject::connect: No such slot Digikam::DigikamView::deleteSelected() in /home/gilles/Devel/5.x/core/app/views/digikamview.cpp:2594
QObject::connect: No such signal Digikam::ContextMenuHelper::signalslotCreateGroupByTime() in /home/gilles/Devel/5.x/core/app/views/digikamview.cpp:2655
QObject::connect: No such signal Digikam::ContextMenuHelper::signalslotCreateGroupByFilename() in /home/gilles/Devel/5.x/core/app/views/digikamview.cpp:2658
digikam.geoiface: ----

The grouping behavior do not generate frame around thumb anymore...

Gilles
Comment 13 Simon 2017-03-18 23:43:18 UTC
Created attachment 104635 [details]
Deduplicate code between icon and table view (V2)

So the idea of the two patches is just to split this into smaller changes that are easier to review. If the refactor has problems, the rest won't work either.

There were some typos and C&P errors in the context menu, the issues you (Maik & Gilles) reported should be fixed now.
Comment 14 Simon 2017-03-18 23:44:33 UTC
Created attachment 104636 [details]
Customize grouping behavior patch version 4
Comment 15 caulier.gilles 2017-03-19 08:04:49 UTC
"Customize" patch version 4 do not compile. I has fixed this problem in version 3 :

/home/gilles/Devel/5.x/core/app/views/tableview/tableview.cpp: In member function ‘QList<QAction*> Digikam::TableView::getExtraGroupingActions()’:
/home/gilles/Devel/5.x/core/app/views/tableview/tableview.cpp:301:78: error: no matching function for call to ‘QAction::QAction(QString)’
     QAction* const actionHideGrouped = new QAction(i18n("Hide grouped items"));
                                                                              ^
In file included from /usr/lib64/qt5/include/QtWidgets/qmenu.h:40:0,
                 from /usr/lib64/qt5/include/QtWidgets/QMenu:1,
                 from /home/gilles/Devel/5.x/core/app/views/tableview/tableview.cpp:31:
/usr/lib64/qt5/include/QtWidgets/qaction.h:174:5: note: candidate: QAction::QAction(QActionPrivate&, QObject*)
     QAction(QActionPrivate &dd, QObject *parent);
     ^
/usr/lib64/qt5/include/QtWidgets/qaction.h:174:5: note:   candidate expects 2 arguments, 1 provided
/usr/lib64/qt5/include/QtWidgets/qaction.h:89:5: note: candidate: QAction::QAction(const QIcon&, const QString&, QObject*)
     QAction(const QIcon &icon, const QString &text, QObject* parent);
     ^
/usr/lib64/qt5/include/QtWidgets/qaction.h:89:5: note:   candidate expects 3 arguments, 1 provided
/usr/lib64/qt5/include/QtWidgets/qaction.h:88:5: note: candidate: QAction::QAction(const QString&, QObject*)
     QAction(const QString &text, QObject* parent);
     ^
/usr/lib64/qt5/include/QtWidgets/qaction.h:88:5: note:   candidate expects 2 arguments, 1 provided
/usr/lib64/qt5/include/QtWidgets/qaction.h:87:14: note: candidate: QAction::QAction(QObject*)
     explicit QAction(QObject* parent);
              ^
/usr/lib64/qt5/include/QtWidgets/qaction.h:87:14: note:   no known conversion for argument 1 from ‘QString’ to ‘QObject*’
/home/gilles/Devel/5.x/core/app/views/tableview/tableview.cpp:311:78: error: no matching function for call to ‘QAction::QAction(QString)’
     QAction* const actionIgnoreGrouping = new QAction(i18n("Ignore grouping"));
                                                                              ^
In file included from /usr/lib64/qt5/include/QtWidgets/qmenu.h:40:0,
                 from /usr/lib64/qt5/include/QtWidgets/QMenu:1,
                 from /home/gilles/Devel/5.x/core/app/views/tableview/tableview.cpp:31:
/usr/lib64/qt5/include/QtWidgets/qaction.h:174:5: note: candidate: QAction::QAction(QActionPrivate&, QObject*)
     QAction(QActionPrivate &dd, QObject *parent);
     ^
/usr/lib64/qt5/include/QtWidgets/qaction.h:174:5: note:   candidate expects 2 arguments, 1 provided
/usr/lib64/qt5/include/QtWidgets/qaction.h:89:5: note: candidate: QAction::QAction(const QIcon&, const QString&, QObject*)
     QAction(const QIcon &icon, const QString &text, QObject* parent);
     ^
/usr/lib64/qt5/include/QtWidgets/qaction.h:89:5: note:   candidate expects 3 arguments, 1 provided
/usr/lib64/qt5/include/QtWidgets/qaction.h:88:5: note: candidate: QAction::QAction(const QString&, QObject*)
     QAction(const QString &text, QObject* parent);
     ^
/usr/lib64/qt5/include/QtWidgets/qaction.h:88:5: note:   candidate expects 2 arguments, 1 provided
/usr/lib64/qt5/include/QtWidgets/qaction.h:87:14: note: candidate: QAction::QAction(QObject*)
     explicit QAction(QObject* parent);
              ^
/usr/lib64/qt5/include/QtWidgets/qaction.h:87:14: note:   no known conversion for argument 1 from ‘QString’ to ‘QObject*’
/home/gilles/Devel/5.x/core/app/views/tableview/tableview.cpp:321:82: error: no matching function for call to ‘QAction::QAction(QString)’
     QAction* const actionShowSubItems = new QAction(i18n("Show grouping in tree"));
                                                                                  ^
In file included from /usr/lib64/qt5/include/QtWidgets/qmenu.h:40:0,
                 from /usr/lib64/qt5/include/QtWidgets/QMenu:1,
                 from /home/gilles/Devel/5.x/core/app/views/tableview/tableview.cpp:31:
/usr/lib64/qt5/include/QtWidgets/qaction.h:174:5: note: candidate: QAction::QAction(QActionPrivate&, QObject*)
     QAction(QActionPrivate &dd, QObject *parent);
     ^
/usr/lib64/qt5/include/QtWidgets/qaction.h:174:5: note:   candidate expects 2 arguments, 1 provided
/usr/lib64/qt5/include/QtWidgets/qaction.h:89:5: note: candidate: QAction::QAction(const QIcon&, const QString&, QObject*)
     QAction(const QIcon &icon, const QString &text, QObject* parent);
     ^
/usr/lib64/qt5/include/QtWidgets/qaction.h:89:5: note:   candidate expects 3 arguments, 1 provided
/usr/lib64/qt5/include/QtWidgets/qaction.h:88:5: note: candidate: QAction::QAction(const QString&, QObject*)
     QAction(const QString &text, QObject* parent);
     ^
/usr/lib64/qt5/include/QtWidgets/qaction.h:88:5: note:   candidate expects 2 arguments, 1 provided
/usr/lib64/qt5/include/QtWidgets/qaction.h:87:14: note: candidate: QAction::QAction(QObject*)
     explicit QAction(QObject* parent);
              ^
/usr/lib64/qt5/include/QtWidgets/qaction.h:87:14: note:   no known conversion for argument 1 from ‘QString’ to ‘QObject*’
core/app/CMakeFiles/digikamgui_src.dir/build.make:1177: recipe for target 'core/app/CMakeFiles/digikamgui_src.dir/views/tableview/tableview.cpp.o' failed
make[2]: *** [core/app/CMakeFiles/digikamgui_src.dir/views/tableview/tableview.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs....

QAction need a parent in constructor (qt 5.6.2 under Mageia 6)

Gilles
Comment 16 caulier.gilles 2017-03-19 08:07:26 UTC
This warning need also to be corrected :

[ 90%] Building CXX object core/app/CMakeFiles/digikamgui_src.dir/digikamgui_src_automoc.cpp.o
/home/gilles/Devel/5.x/core/app/views/digikamview.cpp: In member function ‘bool Digikam::DigikamView::needGroupResolving(Digikam::ApplicationSettings::OperationType, bool) const’:
/home/gilles/Devel/5.x/core/app/views/digikamview.cpp:2469:12: warning: enumeration value ‘WelcomePageMode’ not handled in switch [-Wswitch]
     switch (viewMode())
            ^
/home/gilles/Devel/5.x/core/app/views/digikamview.cpp:2469:12: warning: enumeration value ‘TrashViewMode’ not handled in switch [-Wswitch]
Comment 17 caulier.gilles 2017-03-19 08:14:37 UTC
Created attachment 104641 [details]
Customize grouping behavior patch version 5

This version of patch fix 2 last problems that i report here...
Comment 18 Simon 2017-03-19 13:24:49 UTC
I intentionally removed "this" from these kind of calls:

     QAction* const actionIgnoreGrouping = new QAction(i18n("Ignore grouping"), this);

The reason is that they will be children of DigikamView not of TableView. According to QT5 specifications (http://doc.qt.io/qt-5/qaction.html#QAction) a missing parent object should be acceptable. For me it compiles just fine, I don't understand why it doesn't for you.
Comment 19 caulier.gilles 2017-03-19 14:10:11 UTC
Qt 5.6 LTS need certainly an explicit parent, or null. You must pass a parent to QAction constructor.

Gilles
Comment 20 Simon 2017-03-19 17:13:10 UTC
Created attachment 104648 [details]
Deduplicate code between icon and table view (V3)

Ok, thanks for fixing. I wasn't aware of a QT LTS version we should be compatible with.

Also about the split patch: I would indeed like to get the refactoring part merged independently, as I assume the grouping part will need longer discussion (?) and another issue I was looking at recently would be easier to fix with this in place already.
Comment 21 Vincent Tassy 2017-03-21 16:02:27 UTC
Will this patch also fix the current behaviour of the Slideshow (that shows all images in the groups instead of the visible one) ?

Or does something need to be changed in the Slideshow or Selection code ?
Comment 22 Maik Qualmann 2017-03-21 19:47:44 UTC
Simon,

patch 104648 does not compile...

/home/maik/Devel/digikam-software-compilation/core/app/views/tableview/tableview.cpp: In member function ‘QList<QAction*> Digikam::TableView::getExtraGroupingActions() const’:
/home/maik/Devel/digikam-software-compilation/core/app/views/tableview/tableview.cpp:297:84: error: invalid conversion from ‘const QObject*’ to ‘QObject*’ [-fpermissive]
     QAction* const actionHideGrouped = new QAction(i18n("Hide grouped items"), this);
                                                                                    ^
In file included from /usr/include/qt5/QtWidgets/qmenu.h:46:0,
                 from /usr/include/qt5/QtWidgets/QMenu:1,
                 from /home/maik/Devel/digikam-software-compilation/core/app/views/tableview/tableview.cpp:31:
/usr/include/qt5/QtWidgets/qaction.h:94:14: note:   initializing argument 2 of ‘QAction::QAction(const QString&, QObject*)’
     explicit QAction(const QString &text, QObject *parent = nullptr);
              ^~~~~~~
/home/maik/Devel/digikam-software-compilation/core/app/views/tableview/tableview.cpp:307:84: error: invalid conversion from ‘const QObject*’ to ‘QObject*’ [-fpermissive]
     QAction* const actionIgnoreGrouping = new QAction(i18n("Ignore grouping"), this);
                                                                                    ^
In file included from /usr/include/qt5/QtWidgets/qmenu.h:46:0,
                 from /usr/include/qt5/QtWidgets/QMenu:1,
                 from /home/maik/Devel/digikam-software-compilation/core/app/views/tableview/tableview.cpp:31:
/usr/include/qt5/QtWidgets/qaction.h:94:14: note:   initializing argument 2 of ‘QAction::QAction(const QString&, QObject*)’
     explicit QAction(const QString &text, QObject *parent = nullptr);
              ^~~~~~~
/home/maik/Devel/digikam-software-compilation/core/app/views/tableview/tableview.cpp:317:88: error: invalid conversion from ‘const QObject*’ to ‘QObject*’ [-fpermissive]
     QAction* const actionShowSubItems = new QAction(i18n("Show grouping in tree"), this);
                                                                                        ^
In file included from /usr/include/qt5/QtWidgets/qmenu.h:46:0,
                 from /usr/include/qt5/QtWidgets/QMenu:1,
                 from /home/maik/Devel/digikam-software-compilation/core/app/views/tableview/tableview.cpp:31:
/usr/include/qt5/QtWidgets/qaction.h:94:14: note:   initializing argument 2 of ‘QAction::QAction(const QString&, QObject*)’
     explicit QAction(const QString &text, QObject *parent = nullptr);
              ^~~~~~~
make[2]: *** [core/app/CMakeFiles/digikamgui_src.dir/build.make:1178: core/app/CMakeFiles/digikamgui_src.dir/views/tableview/tableview.cpp.o] Fehler 1
Comment 23 Simon 2017-03-21 23:31:57 UTC
Created attachment 104675 [details]
Deduplicate code between icon and table view (V4)

@Maik Sorry for the inconvenience, even C&P should be tested...

@Vincent Yes, the current state also addresses the behaviour of groups regarding slideshow. You will be able to choose whether to include all or only the first (visible) image.
Comment 24 Maik Qualmann 2017-03-23 20:47:23 UTC
Simon,

"Move To Album..." does not work.

Maik
Comment 25 Simon 2017-03-24 09:19:03 UTC
Created attachment 104714 [details]
Deduplicate code between icon and table view (V5)

Fixed moving to album - thanks Maik!
Comment 26 Simon 2017-03-24 09:22:30 UTC
Created attachment 104716 [details]
Customize grouping behavior patch version 6
Comment 27 Simon 2017-03-31 09:57:23 UTC
Created attachment 104827 [details]
Customize grouping behavior patch version 7

Update to master.
Comment 28 Simon 2017-03-31 09:59:06 UTC
Created attachment 104828 [details]
Deduplicate code between icon and table view (V6)
Comment 29 Simon 2017-04-13 22:26:16 UTC
Created attachment 105016 [details]
Customize grouping behavior patch version 8

I would like to get this done, as there were some reports from users who had problems with the "new" grouping behaviour. Also some other things I wanted to fix depend partially on this.

Can I commit the refactor part (@Maik)?

The grouping part probably needs more attention.
Comment 30 Maik Qualmann 2017-04-14 14:51:43 UTC
I have not found any problems with the V8 patch. But I think we do not need to distinguish between BQM, light table, renaming, etc. An option for all cases (yes, no, ask) should be enough. Gilles, Mario what is your point of view?

Maik
Comment 31 Mario Frank 2017-04-14 14:54:34 UTC
(In reply to Maik Qualmann from comment #30)
> I have not found any problems with the V8 patch. But I think we do not need
> to distinguish between BQM, light table, renaming, etc. An option for all
> cases (yes, no, ask) should be enough. Gilles, Mario what is your point of
> view?
> 
> Maik

I agree. Keep it simple if there is no really good reason. And I currently see none.
Comment 32 Simon 2017-04-14 15:06:29 UTC
Originally the request originated around tagging. After the change users started complaining mostly about slideshow and lighttable. And for my personal workflow it absolutely makes sense to do operations like moving and tagging images together. However with slideshow and lighttable, it doesn't really. For bqm it depends on what you want to do.
What I took out of all the responses is, that there is a multitude of different workflows and I will never be able to think of each and every one. That's why I tried to make it as customizable as possible.
Comment 33 Simon 2017-05-03 20:45:22 UTC
Created attachment 105330 [details]
Customize grouping behavior patch version 9

I updated this with information about the operation in the dialog, streamlined the setup page creation and added some documentation.
Comment 34 Simon 2017-05-08 12:36:05 UTC
Created attachment 105389 [details]
Customize grouping behavior patch version 10

Version 10 fixes compiler warnings.

There was no more problems reported, the refactoring has been met positively and I need it to do further improvements - so unless there is opposition, I will push the non-grouping related changes (104828) to master soon.

Regarding the grouping and whether to have separate options for different operations or just one option: Can you reply to my earlier statement explaining why I think it is worth having separate options please.
Even if the answer is, that you still want the simpler version, that is better than just letting this hang around. I will be disappointed (and run the full version locally, as I already got used to it), but one option to control all is still better than no option.
Comment 35 Maik Qualmann 2017-05-08 16:32:26 UTC
Created attachment 105393 [details]
grouping.png

As I said, I think it would not be necessary if Gilles did not mind, I would also agree. But see screenshot, the first line has no text. Everything stands on ask, but the kipi-Plugins take only the first image of the group, no dialog.

Maik
Comment 36 Maik Qualmann 2017-05-08 16:36:57 UTC
Ignore the thing with the empty text line, I have not seen right. But the other thing does not work.

Maik
Comment 37 caulier.gilles 2017-05-08 17:46:12 UTC
Maik,

No objection from me. I just follow the file only and let's you to test Simon patches.

Note : kipi interface do not support yet grouped items. If i remember, there is a generic file in bugzilla about this topic.

Gilles
Comment 38 Simon 2017-05-08 17:54:31 UTC
I just had a look and couldn't even figure out how the selection makes it to the kipi plugins. Apparently I just added the Kipi member to the enum as I considered it relevant and then never followed through actually implementing it. If you can give me a pointer for how this currently works, I might be able to work around this limitation, otherwise I will remove kipi from this patch.
Comment 39 Maik Qualmann 2017-05-08 17:58:49 UTC
Ok, I also see no code for the kipi-Plugins in the patch, except for the configuration. Simon, I would suggest setting the default for Kipi plugins to NO and disable for selection this line. Or hide the line. And apply the patch.

Maik
Comment 40 caulier.gilles 2017-05-08 18:01:07 UTC
Simon,

The items selcted from icon view is passed to a plugin through the KIPI interface. All is located in core/utilities/kdesupport/kipi/kipiimagecollection.cpp. Look in function images(), and especially line 188 which pass the selected items to the interface as a list of URL. You nee to code something to take a care about the grouped images in the current selection from icon view.

So the patch relevant must be located in DigikamView class.

Gilles
Comment 41 Simon 2017-05-08 22:19:16 UTC
That was too good to be true. The only change needed should be in line 190 of kipiimagecollection.cpp:

    return DigikamApp::instance()->view()->selectedUrls();

to

    return DigikamApp::instance()->view()->selectedUrls(ApplicationSettings::Kipi);

However the problem is, that the kipi plugins (at least dropbox and facebook) call this function several times, so the dialog pop ups multiple times. That's a very bad user experience. I had a quick look at it and it seems to construct the KipiImageCollection several times. I don't know that part at all (and don't use kipi), so I am not overly motivated to read all this code to debug this.
Comment 42 caulier.gilles 2017-05-09 22:22:13 UTC
>The only change needed should be in line 190 of kipiimagecollection.cpp:
>    return DigikamApp::instance()->view()->selectedUrls();
>to
>    return DigikamApp::instance()->view()->selectedUrls(ApplicationSettings::Kipi);

==> no. the only change is to patch DigikamView::selectedUrls() method to return all grouped item depending of the application settings.

If you don't want to test with KIPI, i write a new application interface helper to port batch tools from KIPI to digiKam core (yes, at least in the future only export to web service tools will be hosted to KIPI. That all).

This new interface template is located in core/utilities/assistants/common/dinfointerface.cpp. This is nothing to do here are implementation is done with empty methods. The real implementation for digiKam database is in core/libs/database/utils/dbinfoiface.cpp.

This implementation is mostly some code backport from kipi interface. I plan to factorize this one with the Database info interface, later when implementation will be finalized.

Currently HTMLGallery tool use this interface. When you start this tool, it can use the image selection from icon view (look the option in intro page). If you select some grouped items, only the first one of each groups is used instead all items grouped (depending of grouped icon view option of course).

If you look on dbinfoiface.cpp::line 207, it call DigikamView::selectedUrls(), which must return the right selection if grouped items are selected and depending of icon view grouped options.

So, the patch to support grouped items from icon view must be done somewhere in DigikamView::selectedUrls(). That all. all the rest (kipi included) will work as expected.

Gilles
Comment 43 caulier.gilles 2017-05-09 22:23:13 UTC
Ah i forget to said that Digikamview::allUrls() must be patch to return also the grouped items list depending of digiKam settings.

Gilles
Comment 44 caulier.gilles 2017-05-09 22:25:45 UTC
Simon, 

Look this report :

https://bugs.kde.org/show_bug.cgi?id=286609

Panorama tool do not support grouped items behavior. Look how it's called from DigikamApp::line 2621. It use also DigikamView::selectedUrls()...

Gilles
Comment 45 Simon 2017-05-09 22:36:35 UTC
Gilles,

This is all done already. DigikamView::selectedUrls() accepts an (optional) argument (either bool or ApplicationSettings::OperationType) to include grouped items or not.

The change I proposed is working. The problem is just that it is called several times when opening a kipi plugin, so the dialog asking the user whether to include grouped items or not appears several times. This is a problem of kipi plugins calling KIPI::Interface::currentSelection() several times on opening, each time triggering DigikamView::selectedUrls(Applicationsettings::Kipi) and opening a dialog about grouping.

I will look at HTMLGallery and panorama tools to support grouping for them as well.
Comment 46 caulier.gilles 2017-05-11 16:25:00 UTC
I do not see why it's called more than one time with KIPI plugins.

I see your screenshot of setup panel. I like the new settings. It's clear.

Just a remark : "kipi plugins" => "plugins" : since there is no more image editor plugins, only KIPI provide digiKam plugins. So no need to name KIPI..., just plugins is enough.

I try your patch now...

Gilles
Comment 47 caulier.gilles 2017-05-11 16:26:21 UTC
Simon, patch 9 must be obsolete no ?

What's about Deduplicate code patch ? It's not integrated in patch 10 ?

Gilles
Comment 48 Simon 2017-05-11 16:31:04 UTC
Yes, the deduplicate patch is integrated in the grouping patch. I just thought that splitting the change into meaningful pieces makes it more readable (but that's not really the case with patches, more with commits...) and maybe it could be merged faster.

I will rename the kipi setting then.

About the panorama tool: That is already working with grouping in the current patch.
I have a working solution for the html tool (it had the same problem as the kipi tool with multiple dialogs), but I want to extend it. And I may be able to fix this problem for the kipi plugins as well. So I will post an updated patch, hopefully soon.
Comment 49 caulier.gilles 2017-05-11 16:40:41 UTC
The patch V10 cannot not be applied it against git/master. New version is ready on my computer. I will post it in few minutes...

Gilles
Comment 50 caulier.gilles 2017-05-11 16:46:06 UTC
Created attachment 105446 [details]
Customize grouping behavior patch version 11

New version of patch compilable against git/master
Comment 51 caulier.gilles 2017-05-11 17:21:35 UTC
I tested with patch v11 and HTMLExport and i do not see any difference while processing items. No time latency or something like that.

My grouped album to test :

https://www.flickr.com/photos/digikam/34596906595/in/dateposted-public/

Gilles
Comment 52 Maik Qualmann 2017-05-11 18:22:31 UTC
I have a compiler error here, but will check again, since Gilles has already tested it already.

Maik

/home/maik/Devel/digikam-software-compilation/core/app/items/digikamimageview.cpp: In member function ‘void Digikam::DigikamImageView::deleteSelected(Digikam::ImageViewUtilities::DeleteMode)’:
/home/maik/Devel/digikam-software-compilation/core/app/items/digikamimageview.cpp:384:58: error: no matching function for call to ‘Digikam::DigikamImageView::selectedImageInfos(bool)’
     ImageInfoList imageInfoList = selectedImageInfos(true);
                                                          ^
In file included from /home/maik/Devel/digikam-software-compilation/core/app/items/digikamimageview.h:30:0,
                 from /home/maik/Devel/digikam-software-compilation/core/app/items/digikamimageview_p.h:36,
                 from /home/maik/Devel/digikam-software-compilation/core/app/items/digikamimageview.cpp:28:
/home/maik/Devel/digikam-software-compilation/core/app/items/imagecategorizedview.h:79:28: note: candidate: Digikam::ImageInfoList Digikam::ImageCategorizedView::selectedImageInfos() const
     ImageInfoList          selectedImageInfos()             const;
                            ^~~~~~~~~~~~~~~~~~
/home/maik/Devel/digikam-software-compilation/core/app/items/imagecategorizedview.h:79:28: note:   candidate expects 0 arguments, 1 provided
/home/maik/Devel/digikam-software-compilation/core/app/items/digikamimageview.cpp: In member function ‘void Digikam::DigikamImageView::deleteSelectedDirectly(Digikam::ImageViewUtilities::DeleteMode)’:
/home/maik/Devel/digikam-software-compilation/core/app/items/digikamimageview.cpp:394:58: error: no matching function for call to ‘Digikam::DigikamImageView::selectedImageInfos(bool)’
     ImageInfoList imageInfoList = selectedImageInfos(true);
                                                          ^
In file included from /home/maik/Devel/digikam-software-compilation/core/app/items/digikamimageview.h:30:0,
                 from /home/maik/Devel/digikam-software-compilation/core/app/items/digikamimageview_p.h:36,
                 from /home/maik/Devel/digikam-software-compilation/core/app/items/digikamimageview.cpp:28:
/home/maik/Devel/digikam-software-compilation/core/app/items/imagecategorizedview.h:79:28: note: candidate: Digikam::ImageInfoList Digikam::ImageCategorizedView::selectedImageInfos() const
     ImageInfoList          selectedImageInfos()             const;
                            ^~~~~~~~~~~~~~~~~~
/home/maik/Devel/digikam-software-compilation/core/app/items/imagecategorizedview.h:79:28: note:   candidate expects 0 arguments, 1 provided
/home/maik/Devel/digikam-software-compilation/core/app/items/digikamimageview.cpp: In member function ‘void Digikam::DigikamImageView::rename()’:
/home/maik/Devel/digikam-software-compilation/core/app/items/digikamimageview.cpp:431:67: error: ‘needGroupResolving’ was not declared in this scope
     bool grouping = needGroupResolving(ApplicationSettings::Rename);
                                                                   ^
/home/maik/Devel/digikam-software-compilation/core/app/items/digikamimageview.cpp:432:46: error: no matching function for call to ‘Digikam::DigikamImageView::selectedUrls(bool&)’
     QList<QUrl>  urls = selectedUrls(grouping);
                                              ^
In file included from /home/maik/Devel/digikam-software-compilation/core/app/items/digikamimageview.h:30:0,
                 from /home/maik/Devel/digikam-software-compilation/core/app/items/digikamimageview_p.h:36,
                 from /home/maik/Devel/digikam-software-compilation/core/app/items/digikamimageview.cpp:28:
/home/maik/Devel/digikam-software-compilation/core/app/items/imagecategorizedview.h:81:28: note: candidate: QList<QUrl> Digikam::ImageCategorizedView::selectedUrls() const
     QList<QUrl>            selectedUrls()                   const;
                            ^~~~~~~~~~~~
/home/maik/Devel/digikam-software-compilation/core/app/items/imagecategorizedview.h:81:28: note:   candidate expects 0 arguments, 1 provided
/home/maik/Devel/digikam-software-compilation/core/app/items/digikamimageview.cpp:444:63: error: no matching function for call to ‘Digikam::DigikamImageView::selectedImageInfos(bool&)’
         QUrl nextUrl = nextInOrder(selectedImageInfos(grouping).last(),1).fileUrl();
                                                               ^
In file included from /home/maik/Devel/digikam-software-compilation/core/app/items/digikamimageview.h:30:0,
                 from /home/maik/Devel/digikam-software-compilation/core/app/items/digikamimageview_p.h:36,
                 from /home/maik/Devel/digikam-software-compilation/core/app/items/digikamimageview.cpp:28:
/home/maik/Devel/digikam-software-compilation/core/app/items/imagecategorizedview.h:79:28: note: candidate: Digikam::ImageInfoList Digikam::ImageCategorizedView::selectedImageInfos() const
     ImageInfoList          selectedImageInfos()             const;
                            ^~~~~~~~~~~~~~~~~~
/home/maik/Devel/digikam-software-compilation/core/app/items/imagecategorizedview.h:79:28: note:   candidate expects 0 arguments, 1 provided

Maik
Comment 53 caulier.gilles 2017-05-11 19:45:39 UTC
This error can be generated by a different GC version than one, and of course by different compilation options passed by KDE::ECM modules. Here we don't manage anything.

The best way to hack is to check compilation trace with "export VERBOSE=1"

Mine :

[gilles@localhost core]$ gcc --version
gcc (GCC) 5.4.0

[ 85%] Building CXX object core/app/CMakeFiles/digikamgui_src.dir/items/imagecategorizedview.cpp.o
cd /home/gilles/Devel/5.x/build/core/app && /bin/c++   -DLIBPGF_DISABLE_OPENMP -DQT_NO_CAST_FROM_ASCII -DQT_NO_CAST_FROM_BYTEARRAY -DQT_NO_CAST_TO_ASCII -DQT_NO_SIGNALS_SLOTS_KEYWORDS -DQT_NO_URL_CAST_FROM_STRING -DQT_USE_FAST_OPERATOR_PLUS -DQT_USE_QSTRINGBUILDER -DTRANSLATION_DOMAIN=\"digikam\" -D_GNU_SOURCE -D_LARGEFILE64_SOURCE -I/home/gilles/Devel/5.x/build/core/app -I/home/gilles/Devel/5.x/core/app -I/home/gilles/Devel/5.x/core/app/date -I/home/gilles/Devel/5.x/core/app/dragdrop -I/home/gilles/Devel/5.x/core/app/filters -I/home/gilles/Devel/5.x/core/app/items -I/home/gilles/Devel/5.x/core/app/items/overlays -I/home/gilles/Devel/5.x/core/app/main -I/home/gilles/Devel/5.x/core/app/utils -I/home/gilles/Devel/5.x/core/app/views -I/home/gilles/Devel/5.x/core/app/views/tableview -I/home/gilles/Devel/5.x/core/libs/album -I/home/gilles/Devel/5.x/core/libs/database/collection -I/home/gilles/Devel/5.x/core/libs/database/coredb -I/home/gilles/Devel/5.x/core/libs/database/dbjobs -I/home/gilles/Devel/5.x/core/libs/database/engine -I/home/gilles/Devel/5.x/core/libs/database/haar -I/home/gilles/Devel/5.x/core/libs/database/imagehistory -I/home/gilles/Devel/5.x/core/libs/database/imgqsort -I/home/gilles/Devel/5.x/core/libs/database/item -I/home/gilles/Devel/5.x/core/libs/database/server -I/home/gilles/Devel/5.x/core/libs/database/tags -I/home/gilles/Devel/5.x/core/libs/database/thumbsdb -I/home/gilles/Devel/5.x/core/libs/database/utils -I/home/gilles/Devel/5.x/core/libs/dialogs -I/home/gilles/Devel/5.x/core/libs/dimg -I/home/gilles/Devel/5.x/core/libs/dimg/dklcms -I/home/gilles/Devel/5.x/core/libs/dimg/filters/auto -I/home/gilles/Devel/5.x/core/libs/dimg/filters/bcg -I/home/gilles/Devel/5.x/core/libs/dimg/filters/bw -I/home/gilles/Devel/5.x/core/libs/dimg/filters/cb -I/home/gilles/Devel/5.x/core/libs/dimg/filters/curves -I/home/gilles/Devel/5.x/core/libs/dimg/filters/decorate -I/home/gilles/Devel/5.x/core/libs/dimg/filters -I/home/gilles/Devel/5.x/core/libs/dimg/filters/film -I/home/gilles/Devel/5.x/core/libs/dimg/filters/fx -I/home/gilles/Devel/5.x/core/libs/dimg/filters/greycstoration/cimg -I/home/gilles/Devel/5.x/core/libs/dimg/filters/greycstoration -I/home/gilles/Devel/5.x/core/libs/dimg/filters/hsl -I/home/gilles/Devel/5.x/core/libs/dimg/filters/icc -I/home/gilles/Devel/5.x/core/libs/dimg/filters/lc -I/home/gilles/Devel/5.x/core/libs/dimg/filters/lens -I/home/gilles/Devel/5.x/core/libs/dimg/filters/levels -I/home/gilles/Devel/5.x/core/libs/dimg/filters/nr -I/home/gilles/Devel/5.x/core/libs/dimg/filters/redeye -I/home/gilles/Devel/5.x/core/libs/dimg/filters/sharp -I/home/gilles/Devel/5.x/core/libs/dimg/filters/transform -I/home/gilles/Devel/5.x/core/libs/dimg/filters/wb -I/home/gilles/Devel/5.x/core/libs/dimg/imagehistory -I/home/gilles/Devel/5.x/core/libs/dimg/loaders -I/home/gilles/Devel/5.x/core/libs/dmetadata -I/home/gilles/Devel/5.x/core/libs/dngwriter -I/home/gilles/Devel/5.x/core/libs/dngwriter/extra/dng_sdk -I/home/gilles/Devel/5.x/core/libs/dngwriter/extra/md5 -I/home/gilles/Devel/5.x/core/libs/dngwriter/extra/xmp_sdk/include -I/home/gilles/Devel/5.x/core/libs/dtrash -I/home/gilles/Devel/5.x/core/libs/facesengine/alignment-congealing -I/home/gilles/Devel/5.x/core/libs/facesengine/alignment-flandmark -I/home/gilles/Devel/5.x/core/libs/facesengine -I/home/gilles/Devel/5.x/core/libs/facesengine/detection -I/home/gilles/Devel/5.x/core/libs/facesengine/facedb -I/home/gilles/Devel/5.x/core/libs/facesengine/preprocessing-tantriggs -I/home/gilles/Devel/5.x/core/libs/facesengine/recognition-opencv-lbph -I/home/gilles/Devel/5.x/core/libs/facesengine/shape-predictor -I/home/gilles/Devel/5.x/core/libs/fileactionmanager -I/home/gilles/Devel/5.x/core/libs/filters -I/home/gilles/Devel/5.x/core/libs/imageproperties -I/home/gilles/Devel/5.x/core/libs/iojobs -I/home/gilles/Devel/5.x/core/libs/jpegutils -I/home/gilles/Devel/5.x/core/libs/kmemoryinfo -I/home/gilles/Devel/5.x/core/libs/models -I/home/gilles/Devel/5.x/core/libs/notificationmanager -I/home/gilles/Devel/5.x/core/libs/pgfutils -I/home/gilles/Devel/5.x/core/libs/progressmanager -I/home/gilles/Devel/5.x/core/libs/rawengine -I/home/gilles/Devel/5.x/core/libs/rawengine/libraw/internal -I/home/gilles/Devel/5.x/core/libs/rawengine/libraw/libraw -I/home/gilles/Devel/5.x/core/libs/settings -I/home/gilles/Devel/5.x/core/libs/tags -I/home/gilles/Devel/5.x/core/libs/tags/tagsmanager/models -I/home/gilles/Devel/5.x/core/libs/tags/tagsmanager -I/home/gilles/Devel/5.x/core/libs/template -I/home/gilles/Devel/5.x/core/libs/threadimageio -I/home/gilles/Devel/5.x/core/libs/threads -I/home/gilles/Devel/5.x/core/libs/versionmanager -I/home/gilles/Devel/5.x/core/libs/widgets/colors -I/home/gilles/Devel/5.x/core/libs/widgets/combo -I/home/gilles/Devel/5.x/core/libs/widgets/common -I/home/gilles/Devel/5.x/core/libs/widgets/files -I/home/gilles/Devel/5.x/core/libs/widgets/fonts -I/home/gilles/Devel/5.x/core/libs/widgets/graphicsview -I/home/gilles/Devel/5.x/core/libs/widgets/iccprofiles -I/home/gilles/Devel/5.x/core/libs/widgets/imagehistory -I/home/gilles/Devel/5.x/core/libs/widgets/itemview -I/home/gilles/Devel/5.x/core/libs/widgets/mainview -I/home/gilles/Devel/5.x/core/libs/widgets/metadata -I/home/gilles/Devel/5.x/core/showfoto/main -I/home/gilles/Devel/5.x/core/showfoto/setup -I/home/gilles/Devel/5.x/core/showfoto/thumbbar -I/home/gilles/Devel/5.x/core/tests/advancedrename -I/home/gilles/Devel/5.x/core/tests/albummodel -I/home/gilles/Devel/5.x/core/tests/database -I/home/gilles/Devel/5.x/core/tests/dimg -I/home/gilles/Devel/5.x/core/tests/dmetadata -I/home/gilles/Devel/5.x/core/tests/facesengine -I/home/gilles/Devel/5.x/core/tests/facesengine/demo -I/home/gilles/Devel/5.x/core/tests/fileio -I/home/gilles/Devel/5.x/core/tests/geolocation/editor -I/home/gilles/Devel/5.x/core/tests/geolocation/geoiface/calibrator -I/home/gilles/Devel/5.x/core/tests/geolocation/geoiface/demo -I/home/gilles/Devel/5.x/core/tests/geolocation/geoiface -I/home/gilles/Devel/5.x/core/tests/iojobs -I/home/gilles/Devel/5.x/core/tests/modeltest -I/home/gilles/Devel/5.x/core/tests/multithreading -I/home/gilles/Devel/5.x/core/tests/timestampupdate -I/home/gilles/Devel/5.x/core/tests/widgets -I/home/gilles/Devel/5.x/core/utilities/advancedrename -I/home/gilles/Devel/5.x/core/utilities/advancedrename/common -I/home/gilles/Devel/5.x/core/utilities/advancedrename/parser -I/home/gilles/Devel/5.x/core/utilities/advancedrename/parser/modifiers -I/home/gilles/Devel/5.x/core/utilities/advancedrename/parser/options -I/home/gilles/Devel/5.x/core/utilities/advancedrename/parser/options/database -I/home/gilles/Devel/5.x/core/utilities/advancedrename/parser/options/database/keys -I/home/gilles/Devel/5.x/core/utilities/assistants/calendar/print -I/home/gilles/Devel/5.x/core/utilities/assistants/calendar/wizard -I/home/gilles/Devel/5.x/core/utilities/assistants/common -I/home/gilles/Devel/5.x/core/utilities/assistants/expoblending/blendingdlg -I/home/gilles/Devel/5.x/core/utilities/assistants/expoblending/manager -I/home/gilles/Devel/5.x/core/utilities/assistants/expoblending/wizard -I/home/gilles/Devel/5.x/core/utilities/assistants/firstrun -I/home/gilles/Devel/5.x/core/utilities/assistants/htmlgallery/generator -I/home/gilles/Devel/5.x/core/utilities/assistants/htmlgallery/parameters -I/home/gilles/Devel/5.x/core/utilities/assistants/htmlgallery/wizard -I/home/gilles/Devel/5.x/core/utilities/assistants/panorama/manager -I/home/gilles/Devel/5.x/core/utilities/assistants/panorama/ptoparser -I/home/gilles/Devel/5.x/core/utilities/assistants/panorama/ptotype -I/home/gilles/Devel/5.x/core/utilities/assistants/panorama/tasks -I/home/gilles/Devel/5.x/core/utilities/assistants/panorama/wizard -I/home/gilles/Devel/5.x/core/utilities/facemanagement -I/home/gilles/Devel/5.x/core/utilities/fuzzysearch -I/home/gilles/Devel/5.x/core/utilities/geolocation/editor/backends -I/home/gilles/Devel/5.x/core/utilities/geolocation/editor/bookmark -I/home/gilles/Devel/5.x/core/utilities/geolocation/editor/correlator -I/home/gilles/Devel/5.x/core/utilities/geolocation/editor/dialog -I/home/gilles/Devel/5.x/core/utilities/geolocation/editor/dragdrop -I/home/gilles/Devel/5.x/core/utilities/geolocation/editor/items -I/home/gilles/Devel/5.x/core/utilities/geolocation/editor/reversegeocoding -I/home/gilles/Devel/5.x/core/utilities/geolocation/editor/searches -I/home/gilles/Devel/5.x/core/utilities/geolocation/geoiface -I/home/gilles/Devel/5.x/core/utilities/geolocation/geoiface/backends -I/home/gilles/Devel/5.x/core/utilities/geolocation/geomapwrapper -I/home/gilles/Devel/5.x/core/utilities/geolocation/mapsearches -I/home/gilles/Devel/5.x/core/utilities/imageeditor/core -I/home/gilles/Devel/5.x/core/utilities/imageeditor/dialogs -I/home/gilles/Devel/5.x/core/utilities/imageeditor/editor -I/home/gilles/Devel/5.x/core/utilities/imageeditor/main -I/home/gilles/Devel/5.x/core/utilities/imageeditor/printiface -I/home/gilles/Devel/5.x/core/utilities/imageeditor/rawimport -I/home/gilles/Devel/5.x/core/utilities/imageeditor/tools/colors -I/home/gilles/Devel/5.x/core/utilities/imageeditor/tools/decorate -I/home/gilles/Devel/5.x/core/utilities/imageeditor/tools/enhance -I/home/gilles/Devel/5.x/core/utilities/imageeditor/tools/enhance/hotpixels -I/home/gilles/Devel/5.x/core/utilities/imageeditor/tools/filters -I/home/gilles/Devel/5.x/core/utilities/imageeditor/tools/transform -I/home/gilles/Devel/5.x/core/utilities/imageeditor/widgets -I/home/gilles/Devel/5.x/core/utilities/importui/backend -I/home/gilles/Devel/5.x/core/utilities/importui/dialogs -I/home/gilles/Devel/5.x/core/utilities/importui/items -I/home/gilles/Devel/5.x/core/utilities/importui/main -I/home/gilles/Devel/5.x/core/utilities/importui/models -I/home/gilles/Devel/5.x/core/utilities/importui/views -I/home/gilles/Devel/5.x/core/utilities/importui/widgets -I/home/gilles/Devel/5.x/core/utilities/kdesupport/akonadi -I/home/gilles/Devel/5.x/core/utilities/kdesupport/kfilemetadata -I/home/gilles/Devel/5.x/core/utilities/kdesupport/kipi -I/home/gilles/Devel/5.x/core/utilities/kdesupport/ksane -I/home/gilles/Devel/5.x/core/utilities/lighttable -I/home/gilles/Devel/5.x/core/utilities/maintenance -I/home/gilles/Devel/5.x/core/utilities/metadataedit/dialog -I/home/gilles/Devel/5.x/core/utilities/metadataedit/exif -I/home/gilles/Devel/5.x/core/utilities/metadataedit/iptc -I/home/gilles/Devel/5.x/core/utilities/metadataedit/xmp -I/home/gilles/Devel/5.x/core/utilities/presentation/audio -I/home/gilles/Devel/5.x/core/utilities/presentation/common -I/home/gilles/Devel/5.x/core/utilities/presentation/dialogs -I/home/gilles/Devel/5.x/core/utilities/presentation/opengl -I/home/gilles/Devel/5.x/core/utilities/presentation -I/home/gilles/Devel/5.x/core/utilities/presentation/widgets -I/home/gilles/Devel/5.x/core/utilities/queuemanager/main -I/home/gilles/Devel/5.x/core/utilities/queuemanager/manager -I/home/gilles/Devel/5.x/core/utilities/queuemanager/tools/color -I/home/gilles/Devel/5.x/core/utilities/queuemanager/tools/convert -I/home/gilles/Devel/5.x/core/utilities/queuemanager/tools/custom -I/home/gilles/Devel/5.x/core/utilities/queuemanager/tools/decorate -I/home/gilles/Devel/5.x/core/utilities/queuemanager/tools/enhance -I/home/gilles/Devel/5.x/core/utilities/queuemanager/tools/filters -I/home/gilles/Devel/5.x/core/utilities/queuemanager/tools/metadata -I/home/gilles/Devel/5.x/core/utilities/queuemanager/tools/transform -I/home/gilles/Devel/5.x/core/utilities/queuemanager/views -I/home/gilles/Devel/5.x/core/utilities/searchwindow -I/home/gilles/Devel/5.x/core/utilities/setup/album -I/home/gilles/Devel/5.x/core/utilities/setup/camera -I/home/gilles/Devel/5.x/core/utilities/setup/collections -I/home/gilles/Devel/5.x/core/utilities/setup/editor -I/home/gilles/Devel/5.x/core/utilities/setup/metadata -I/home/gilles/Devel/5.x/core/utilities/setup -I/home/gilles/Devel/5.x/core/utilities/slideshow -I/home/gilles/Devel/5.x/core/libs/jpegutils/libjpeg-84 -I/home/gilles/Devel/5.x/build/core/app/utils -I/home/gilles/Devel/5.x/build/extra/libkipi/src -I"/lib64/qt5/include/QtAV /lib64/qt5/include/QtAVWidgets" -I/usr/include/opencv -I/usr/lib64/qt5/include -I/usr/lib64/qt5/include/QtWebKitWidgets -I/usr/lib64/qt5/include/QtWebKit -I/usr/lib64/qt5/include/QtGui -I/usr/lib64/qt5/include/QtCore -I/usr/lib64/qt5/./mkspecs/linux-g++ -I/usr/lib64/qt5/include/QtNetwork -I/usr/lib64/qt5/include/QtWidgets -I/usr/lib64/qt5/include/QtXml -I/usr/lib64/qt5/include/QtSql -I/usr/include/KF5/KI18n -I/usr/include/KF5 -I/usr/include/KF5/Solid -I/usr/include/KF5/KCoreAddons -I/usr/include/KF5/KWindowSystem -I/usr/include/KF5/KXmlGui -I/usr/lib64/qt5/include/QtDBus -I/usr/include/KF5/KConfigCore -I/usr/include/KF5/KConfigWidgets -I/usr/include/KF5/KCodecs -I/usr/include/KF5/KWidgetsAddons -I/usr/include/KF5/KConfigGui -I/usr/include/KF5/KAuth -I/usr/include/KF5/KService -I/usr/include/KF5/KIOWidgets -I/usr/include/KF5/KIOCore -I/usr/include/KF5/KJobWidgets -I/usr/include/KF5/KCompletion -I/usr/lib64/qt5/include/QtOpenGL -I/usr/include/gphoto2  -std=c++0x -fno-operator-names -Wall -Wextra -Wcast-align -Wchar-subscripts -Wformat-security -Wno-long-long -Wpointer-arith -Wundef -Wnon-virtual-dtor -Woverloaded-virtual -Werror=return-type   -std=c++0x -fno-operator-names -Wall -Wextra -Wcast-align -Wchar-subscripts -Wformat-security -Wno-long-long -Wpointer-arith -Wundef -Wnon-virtual-dtor -Woverloaded-virtual -Werror=return-type   -std=c++0x -fno-operator-names -Wall -Wextra -Wcast-align -Wchar-subscripts -Wformat-security -Wno-long-long -Wpointer-arith -Wundef -Wnon-virtual-dtor -Woverloaded-virtual -Werror=return-type -fexceptions -g -fPIC -fvisibility=hidden -fvisibility-inlines-hidden   -o CMakeFiles/digikamgui_src.dir/items/imagecategorizedview.cpp.o -c /home/gilles/Devel/5.x/core/app/items/imagecategorizedview.cpp

Gilles
Comment 54 Maik Qualmann 2017-05-11 20:17:50 UTC
Gilles,

The patch can not compile, the imagecategorizedview.h is not patched.

maik@linux-tpgn:~ gcc --version
gcc (SUSE Linux) 6.3.1 20170202 [gcc-6-branch revision 245119]

Maik
Comment 55 caulier.gilles 2017-05-11 20:36:44 UTC
Created attachment 105453 [details]
Customize grouping behavior patch version 12

This one must compile fine...
Comment 56 Simon 2017-05-11 20:37:27 UTC
Created attachment 105454 [details]
Customize grouping behavior patch version 12

These changes somehow went missing patch 11, 12 should compile again (doesn't contain anything new of notice yet).
Comment 57 Simon 2017-05-12 19:10:39 UTC
Created attachment 105488 [details]
Customize grouping behavior patch version 13

So I did not follow through with my earlier statement and did patch kipi-plugins. I reduced the number of ImageCollection instances created and removed redundant calls to loadImagesFromCurrentSelection(). Before these were merely inefficient, with this new grouping dialog these calls were disruptive. Now everything runs smoothly (I hope I caught all plugins, I must admit I didn't actually try all of them).

The html gallery does work with grouping as well now (part of "Tools"). I also added the grouping functionality when the album selector is used for consistency.
Comment 58 Simon 2017-05-12 19:11:25 UTC
Created attachment 105489 [details]
Adjust kipi-plugins for grouping
Comment 59 caulier.gilles 2017-05-12 19:38:44 UTC
Simon,

I tested too on my computer. No side effect and all run as expected as i can see...

Good work. I think all is fine to commit now.

Gilles
Comment 60 Simon 2017-05-12 23:04:53 UTC
Git commit 4ed5f88f787329a29273bbb2fb2fe80714bc735d by Simon Frei.
Committed on 12/05/2017 at 22:47.
Pushed by sfrei into branch 'master'.

Handle grouped images based on what operation is to be done
Related: bug 286609, bug 378552, bug 377632
FIXED-IN: 5.6.0

M  +4    -1    NEWS
M  +8    -233  app/items/digikamimageview.cpp
M  +7    -18   app/items/digikamimageview.h
M  +68   -11   app/items/imagecategorizedview.cpp
M  +12   -5    app/items/imagecategorizedview.h
M  +6    -6    app/main/digikamapp.cpp
M  +2    -2    app/utils/contextmenuhelper.cpp
M  +341  -80   app/views/digikamview.cpp
M  +63   -13   app/views/digikamview.h
M  +93   -235  app/views/tableview/tableview.cpp
M  +31   -30   app/views/tableview/tableview.h
M  +14   -18   app/views/tableview/tableview_model.cpp
M  +2    -0    app/views/tableview/tableview_model.h
M  +5    -122  libs/database/coredb/coredb.cpp
M  +1    -12   libs/database/coredb/coredb.h
M  +106  -50   libs/database/utils/dbinfoiface.cpp
M  +9    -2    libs/database/utils/dbinfoiface.h
M  +39   -6    libs/database/utils/dio.cpp
M  +4    -0    libs/database/utils/dio_p.h
M  +9    -0    libs/dmetadata/metadatasettingscontainer.cpp
M  +3    -1    libs/dmetadata/metadatasettingscontainer.h
M  +1    -1    libs/models/imagefiltermodel.cpp
M  +26   -5    libs/settings/applicationsettings.cpp
M  +74   -2    libs/settings/applicationsettings.h
M  +0    -5    libs/settings/applicationsettings_mime.cpp
M  +122  -0    libs/settings/applicationsettings_miscs.cpp
M  +31   -2    libs/settings/applicationsettings_p.cpp
M  +208  -196  libs/settings/applicationsettings_p.h
M  +1    -1    utilities/assistants/common/dimageslist.cpp
M  +0    -5    utilities/assistants/common/dinfointerface.cpp
M  +2    -3    utilities/assistants/common/dinfointerface.h
M  +4    -1    utilities/assistants/htmlgallery/wizard/htmlselectionpage.cpp
M  +12   -106  utilities/kdesupport/kipi/kipiimagecollection.cpp
M  +1    -0    utilities/setup/CMakeLists.txt
M  +10   -10   utilities/setup/album/setupmime.cpp
M  +116  -40   utilities/setup/metadata/setupmetadata.cpp
M  +1    -0    utilities/setup/metadata/setupmetadata.h
M  +8    -6    utilities/setup/setupdatabase.cpp
M  +137  -39   utilities/setup/setupmisc.cpp
M  +13   -0    utilities/setup/setupmisc.h
A  +53   -0    utilities/setup/setuputils.cpp     [License: BSD]
A  +37   -0    utilities/setup/setuputils.h     [License: BSD]

https://commits.kde.org/digikam/4ed5f88f787329a29273bbb2fb2fe80714bc735d