Bug 361047

Created attachment 98114 [details]
mockup to better recognize groups of images

This idea is EXCELENT. I search a lots a visual solution to show grouped images. The current icon was the only way to do it, and i agree that usability is not very good.

Gilles Caulier

Thank you! :-)

I'd be happy to create a border / backdrop for grouped images, I'll be happy to do it - just tell me what formats you need (probably SVG to zoom it, or are you going to draw it in software?).

IMHO: The colored background for expanded groups should be very slight and adhere to the theme that is currently set in KDE.

Jens,

The thumb-view rendering do not use SVG like way to render thumbnail borders. The GUI is hardcoded in source code using Qt5 API. It's not very complicated in fact. Dolphin and Gwenview do the same as i know.

If you want to take a look in source code, the frame around thumb image for all icon view (Album GUI, LT, Import tool) is coded here :

https://quickgit.kde.org/?p=digikam.git&a=blob&h=63042b56f064c47195074fe2b66692091a87ff27&hb=16dc3a9b055931dbb84a2d29f3a0a901bebbc812&f=libs%2Fwidgets%2Fitemview%2Fditemdelegate.cpp

Line 90, the item view delegate which render thumb in customized way , call a method named generateFuzzyRect(), in case of the border pixamp is not yet stored in cache mecanism, to prevent hang up while icon view rendering. This method is available here :

https://quickgit.kde.org/?p=digikam.git&a=blob&h=21d42a66a15e53d83056de0ded70108e747ba9b4&hb=16dc3a9b055931dbb84a2d29f3a0a901bebbc812&f=libs%2Fwidgets%2Fmainview%2Fthumbbardock.cpp

... at line 239. This is the hard coded implementation to draw fuzzy border around thumb image, for each icon view item.

A new method named generateFuzzyRectForGroup() must be written here to render a new border in case of grouped image. The switch between generateFuzzyRect() generateFuzzyRectForGroup() will be done on top level class if the properties hasGroup is turn on or off accordingly with DB properties.

Gilles

I have looked into the source but I'm not good enough in C++ to do the actual programming. Sorry ... What I did in GIMP was to copy the current thumbnail border and clone it rotated around the image center three times -10°, 5°, and 10°. Then combine the resulting images.

Of course, the perfect solution ;) would be to display the actual thumbnails of the "other" grouped images so their borders are a little visible (much brighter than default so the front image is still prominent) - but I don't think this is really necessary for a start.

Hi Mohamed and Omar,

I identify better where the code must be patched exactly :

https://quickgit.kde.org/?p=digikam.git&a=blob&h=35b917742c0a0eee08c453f0235190c87404b403&hb=f6e0fba7025fbbe5f9cd1d4fc8b16f88a5d0928e&f=libs%2Fwidgets%2Fitemview%2Fitemviewimagedelegate.cpp

In this implementation, the method ItemViewImageDelegate::drawThumbnail() from the icon-view model is used by view class to draw the border over the thumbnail.

The border pixmap is get through the call.

QPixmap borderPix = thumbnailBorderPixmap(actualPixmapRect.size());

This model class don't know if it's a grouped item or not. The view class know this property. For ex, to draw the grouped icon, we call from view class the method in the model  : ItemViewImageDelegate::drawGroupIndicator().

So i propose:

1/ to pass as argument a new bool value to ItemViewImageDelegate::drawThumbnail() to indicate the grouped properties. This properties will be get in view class (see how ItemViewImageDelegate::drawGroupIndicator() is called).

2/ To implement relevant code in ItemViewImageDelegate::drawThumbnail() to draw grouped frame over the current border pixmap. I'm not sure if additional rotated border as proposed in this entry will be the best visual solution. Experimentation is welcome.

Gilles Caulier

-

Hi Mr Gilles,


I'll look into it during this week, and will finish it by next Friday, Please let me know if you need it finished earlier.

Regards,
Omar

while next week, this will be fine

Gilles Caulier

Created attachment 98455 [details]
Group border modification patch

This patch is for grouped images border new background

Created attachment 98456 [details]
Screenshot of the new grouped images border

Thanks Omar for you patch.

1/ code cosmetic : 

* Always add a space after a coma, especially  with methods arguments.
* Try to always align code with multi-lines
* Wrap code with bracket after if/else statements, especially with multi-lines.

2/ new border : why the rotated frames are not centered with thumb center ? It's wanted ? They show more visible on the bottom side.

3/ optimization : these calls to generateFuzzyRect() are time consuming :

+QPixmap ThumbBarDock::generateFuzzyRectForGroup(const QSize& size, const QColor& color, int radius)
+{
+    // Create two normal borders
+    QPixmap border1 = generateFuzzyRect(size,color,radius);
+    QPixmap border2 = generateFuzzyRect(size,color,radius);

For some thumbnails it's not a problem. But imagine in an album with 100 grouped items ?
Why not yo simply make deep copy of first pixmap ?

4/ Go to Setup/Album-View, and turn on huge thumbnail support for HD screen (thumbs >256 pixels size). The grouped frame do not work.

Gilles Caulier

Thanks Mr Gilles for the feedback.

I'll make another patch taking into consideration the points you mentioned and get back to you.

Thanks for your time.

This looks great! Thank you for implementing!

One little request .. can you make the grouped border a little darker? I think it would be better visible. Or does it depend on the KDE theme?

Omar,

Did you work on a new patch ?

Gilles Caulier

Dear Mr Gilles,

I'm sorry for being late, I'll start today on it, and also tomorrow will be working on it the whole day, and will send a patch tomorrow max.

Mr Jens, yes for sure i can, I'll also try to do the grouped background color and get back to you :)

Regards,
Omar

Created attachment 98577 [details]
Thumbnail centred

Hello,

I've fixed the centring issue for the thumbnail and have attached a sample for 3 different groups, please let me know if you think they're still not symmetrical.

Regarding the large thumbnail support, I've tried to reproduce this issue on my PC but couldn't

I tried the following
From Settings->Configure-digiKam --> Album View :
I checked the option --> use large thumbnail size for high screen resolution
and restart digiKam for this option to take effect, but nothing happened

Also from Tools-> Maintenance :
I checked the rebuild thumbnails  and restarted digiKam but nothing happened.

Do I need to have an HD screen for this option to take effect?

I'll still search for the code responsible for handling this issue and get back to you with the patch once i finish it.

Thank you :)
Omar

HD thumbnails option permit to use largest thumbs (> 256 pixels)

Use silder on statusbar to change icon thumbs size. You can increase it over 256 until 512 .

Note : For me the screenshot look good.

Gilles Caulier

Hi, 

I tried to adjust the thumbnail size to 512 on my 32" screen, and I see no problem.

Mohamed Anwer

So i'm waiting new patch to review...

Gilles Caulier

Created attachment 98630 [details]
Group border modification patch v2

Contains code for centring thumbnail inside new group borders

Dear Mr Gilles,

I'm sorry for being late, attached the new patch containing the modifications needed for centring the thumbnail inside the group borders, If we have time I can work on the other needed modifications (grouped images different background colour and the darker group border) and get back to you as soon as I finish them.

Omar

The new patch do not work better in HD thumb size.

1/ the frame go outside the icon border.
2/ with black color theme, frame is invisible.
3/ The side-effects are less visible with small thumb size, but are always here (zoom on the screen near the border frame for ex)

Proposal : 

- draw the grouped frame inside the thumbnail, not outside.
- use right color element to dram the frame.

Gilles Caulier

Screenshot :
https://www.flickr.com/photos/digikam/26086592624/in/dateposted-public/
https://www.flickr.com/photos/digikam/26086592554/in/dateposted-public/

Gilles Caulier

Note : LightRoom groupped items is called "Image Stacked". It's explained here :

http://valeriegoettsch.com/lightroom-grid-view-icons/

Excepted the icon+number of items in the stack, no specify frames is draw.

Gilles

(In reply to caulier.gilles from comment #25)
> Note : LightRoom groupped items is called "Image Stacked". It's explained
> here :
> 
> http://valeriegoettsch.com/lightroom-grid-view-icons/
> 
> Excepted the icon+number of items in the stack, no specify frames is draw.
> 
> Gilles

Thanks for the feedback.

to start working i just want to make sure that i understood what's required.

In the following image
http://valeriegoettsch.com/wp-content/uploads/badges-diagram.jpg

I'll imitate the following features:
- Stack (stating 1 of 2 -- where 2 is the group images count)
- the virtual copy (do i need to draw the turned corner to indicate visually that this is a group ?)

in the following image 
https://bugs.kde.org/attachment.cgi?id=98577&action=edit
What about the icon in the bottom right on which the number of grouped images is written ?
should it be removed?

Thank you,
Omar

Hmmm. I still like iPhoto's visualization of an image "stack" better:
https://www.apple.com/de/mac/iphoto/ (scroll down to the screenshot labeled "Share.")

It looks less technical, it is easy to spot among many images, and there is nothing to translate.

Of course a number in the corner indicating the number of images in addition (!) is also good. :-)

No. The LR screenshot is just to indicate how pro application do the job. It just informative.

LR do no make better than us... That all.

The information in DK about grouped items is already here and at the right place. Do not change it.

Just try to make the grouped border frame better, to render it inside the thumbnails, not outside. The free space outside is too limited and make mistake visually with large thumbs.

Gilles Caulier

Also take a care of color theme used with DK. Look in settings menu for details.

If you render the border frame inside the thumbs, take a care about thumbs color content.

I think that we must to render a smallest thumb with a largest border in case of grouped items. This will be the right way to prevent to overload the thumbs content.

Gilles Caulier

Created attachment 98715 [details]
stack proposal

Dear Mr Gilles and Mr Jens,

I made this simple stack just for confirming if that's what we need or not?, i made it on a separate project to make the compilation quicker, that's why not all digKam image info is shown nor the borders, it's just for confirming the view and if that's what's required or not.

I'll still make the turned corner, or did i misunderstood the requirements?

I didn't fully understand what you mean by:

- "I think that we must to render a smallest thumb with a largest border in case of grouped items"
do you mean that the border will be of different size for different images?
and
"Just try to make the grouped border frame better, to render it inside the thumbnails, not outside"
how can the rotated frames fit inside the thumbnail?

if you can clarify more that would be perfect.

sorry for inconvenience.

Regards,
Omar

if we need to use the rotated borders, we can adjust the border for each image at different thumbnail sizes differently that will make us able to fit the rotated borders at different thumbnail sizes entirely inside the image rectangle, the only withdraw for this approach would be it'll show less number of images per row in the album grid.

In your stack proposal has the stack icon over the thumb. This one is already outside the thumb because it's clickable to open/close the stack. If you move it over the thumb, as the thumb is also clickable, this will not work. So the icon must still outside the thumb.

The large border in gray delimit a the stack in a more visible way, and it's more suitable. But a simple large gray border is not enough. We need rotated border pixmap inside this gray area.

Note about your last patch V2 : 

1/
static const int IMAGE_BORDER_RADIUS = 3; 
==
const int radius = IMAGE_BORDER_RADIUS;

...is not necessary.

2/ use more CR in you cod. no need to attach all lines together.

3/ No need a cast here : 

const_cast<DItemDelegate*>(this)->d->thumbnailBorderCache.insert(cacheKey, new QPixmap(pix));

d->thumbnailBorderCache.insert(cacheKey, new QPixmap(pix)); must be enough. Do not try to make JAVA coding style please.

Gilles Caulier

Now i understand the requirements, i'll finish it ASAP and get back to you.

Thanks you
Omar

Any progress here ?

Gilles Caulier

Created attachment 98937 [details]
Black background theme, group bordered is filled

Created attachment 98938 [details]
progress in ensure the image border doesn't go outside the thumbnail border, temp solution

Created attachment 98939 [details]
Group images highlighted when group is expanded

Dear Mr Gilles,

I've attached three images for the three problems i'm currently addressing:
1- Group border was transparent so when the theme is changed to black background, the border is visible, but i think it can be better, please let me know your opinion.

2- The Image to be totally included inside the thumbnail, i'm still solving this, what i'm trying to do is to limit the image thumbnail size (the space in which the image and its border to be drawn within) to make sure that the image and its border doesn't go outside the thumbnail also, they doesn't overlay the group indicator

3- the grouped images background color while expanded as requested by Mr Jens.

What i'm trying to do:
1- to add to each theme two more appropriate colors one for the grouped images background color while expanded, the other to choose the border color depending on the theme, the current setting is suitable when the background is white, but it doesn't look good when using any other background color.

2- resize the image thumbnail to fit within a max width and max height that are a function of the slider value in the buttom right so as to the grouped border fit within the image thumbnail as requested, but i didn't manage to do it yet.


If there're other requests please let me know what are they, and i'll finish within the next couple of days.

I'm expecting to finish these suggestions max by tomorrow(Friday) or by Saturday.

I'm sorry for the long delay, it won't happen again

Thank you,
Omar

Some comments from my side since I opened the original bug:

I like the progress and I am really thankful that there is so much effort invested in this feature!
I think the size of the additional border should scale with the size of the thumbnail and not be a fixed amount of pixels. In the screenshots mentioned above the size of the group border is (IMHO) too small and not really well visible. Look at iPhoto's borders (e.g. https://www.apple.com/de/mac/iphoto/, scroll down to the screenshot titled "Share.") for example.
Also, the background for an expanded group should have a little less contrast, the current dark yellow (is this fixed, configurable or depending on the theme?) is a little too drastic for my taste.

But apart from that, very good work! Thank you to all involved!

Omar,

If you update you patch, Jens will be able to test last version of your code (if he work with current implementation from git/master)...

Gilles Caulier

Created attachment 98977 [details]
Not final patch, just a trial for feedback

Dear Mr Gilles, Mr Jens,

I've attached a draft patch for testing and getting your feedback, what is supposed to be fixed in this patch:
1- The images are enclosed in the thumbnails border, not outside (i did so through the increasing the margin, i'm still trying to adapt the margin with the thumbnail size)
2- the grouped images while are in expanded mode are colored with different colors than the background and selected images (still not in all themes, if you can suggest the appropriate colors for each theme that would be great)
3- the group border is now visible nomatter the background color is.

What is still in progress:
1- Coloring the grouped images with less contrast colors depending on the theme (the main issue for this would be the color needed for each theme, your suggestions would be so much appreciated).
2- Adapting the margin with thumbnail size (currently it's 14, it needs to decrease with thumbnail size)
3- adjusting the look and feel for the grouped images borders.
4- adjusting the rating rectangle color transparency as it in some cases overlay the bottom group border.


Your feedback is so much appreciated, i'm still working on the patch and will get back to you tomorrow with my updates.

Thank you

Created attachment 99001 [details]
patch3 grouped images, fitting grouped borders

Comment on attachment 99001 [details]
patch3 grouped images, fitting grouped borders

>diff --git a/app/items/imagedelegate.cpp b/app/items/imagedelegate.cpp
>index 25f36c8..716afc4 100644
>--- a/app/items/imagedelegate.cpp
>+++ b/app/items/imagedelegate.cpp
>@@ -253,16 +253,22 @@ void ImageDelegate::paint(QPainter* p, const QStyleOptionViewItem& option, const
>     // Thumbnail
>     QPixmap pix;
> 
>+    bool isGroupExpanded = (info.hasGroupedImages() && index.data(ImageFilterModel::GroupIsOpenRole).toBool())
>+            || (info.isGrouped());
>     if (isSelected)
>     {
>         pix = d->selPixmap;
>     }
>+    else if(isGroupExpanded)
>+    {
>+        pix = d->grpPixmap;
>+    }
>     else
>     {
>         pix = d->regPixmap;
>     }
> 
>-    QRect actualPixmapRect = drawThumbnail(p, d->pixmapRect, pix, thumbnailPixmap(index));
>+    QRect actualPixmapRect = drawThumbnail(p, d->pixmapRect, pix, thumbnailPixmap(index), info.hasGroupedImages());
> 
>     if (!actualPixmapRect.isNull())
>     {
>@@ -271,7 +277,16 @@ void ImageDelegate::paint(QPainter* p, const QStyleOptionViewItem& option, const
> 
>     if (!d->ratingRect.isNull())
>     {
>-        drawRating(p, index, d->ratingRect, info.rating(), isSelected);
>+        int backgroundtype = 0;
>+        if(isSelected)
>+        {
>+            backgroundtype = 1;
>+        }
>+        else if(isGroupExpanded)
>+        {
>+            backgroundtype = 2;
>+        }
>+        drawRating(p, index, d->ratingRect, info.rating(), backgroundtype);
>     }
> 
>     // Draw Color Label rectangle
>diff --git a/libs/widgets/itemview/ditemdelegate.cpp b/libs/widgets/itemview/ditemdelegate.cpp
>index b92fd43..9fa3824 100644
>--- a/libs/widgets/itemview/ditemdelegate.cpp
>+++ b/libs/widgets/itemview/ditemdelegate.cpp
>@@ -78,19 +78,35 @@ void DItemDelegate::clearCaches()
>     d->squeezedTextCache.clear();
> }
> 
>-QPixmap DItemDelegate::thumbnailBorderPixmap(const QSize& pixSize) const
>+QPixmap DItemDelegate::thumbnailBorderPixmap(const QSize& pixSize, bool isGrouped) const
> {
>     const QColor borderColor = QColor(0, 0, 0, 128);
>-    QString cacheKey         = QString::number(pixSize.width()) + QLatin1Char('-') + QString::number(pixSize.height());
>+    QString cacheKey         = QString::number(pixSize.width())  + QLatin1Char('-')
>+                             + QString::number(pixSize.height()) + QLatin1Char('-')
>+                             + QString::number(isGrouped);
>     QPixmap* const cachePix  = d->thumbnailBorderCache.object(cacheKey);
> 
>     if (!cachePix)
>     {
>         const int radius = 3;
>-        QPixmap pix      = ThumbBarDock::generateFuzzyRect(QSize(pixSize.width()  + 2*radius,
>-                           pixSize.height() + 2*radius),
>-                           borderColor, radius);
>-        const_cast<DItemDelegate*>(this)->d->thumbnailBorderCache.insert(cacheKey, new QPixmap(pix));
>+        QPixmap pix;
>+        const int width  = pixSize.width()  + 2*radius;
>+        const int height = pixSize.height() + 2*radius;
>+
>+        if(!isGrouped)
>+        {
>+            pix = ThumbBarDock::generateFuzzyRect(QSize(width, height),
>+                                                  borderColor,
>+                                                  radius);
>+        }
>+        else
>+        {
>+            pix = ThumbBarDock::generateFuzzyRectForGroup(QSize(width, height),
>+                                                          borderColor,
>+                                                          radius);
>+        }
>+
>+        d->thumbnailBorderCache.insert(cacheKey, new QPixmap(pix));
>         return pix;
>     }
> 
>diff --git a/libs/widgets/itemview/ditemdelegate.h b/libs/widgets/itemview/ditemdelegate.h
>index 87f4614..7caa5c0 100644
>--- a/libs/widgets/itemview/ditemdelegate.h
>+++ b/libs/widgets/itemview/ditemdelegate.h
>@@ -85,7 +85,7 @@ protected:
>     virtual void clearCaches();
> 
>     QString squeezedTextCached(QPainter* const p, int width, const QString& text) const;
>-    QPixmap thumbnailBorderPixmap(const QSize& pixSize) const;
>+    QPixmap thumbnailBorderPixmap(const QSize& pixSize, bool isGrouped = false) const;
> 
> private:
> 
>diff --git a/libs/widgets/itemview/itemviewimagedelegate.cpp b/libs/widgets/itemview/itemviewimagedelegate.cpp
>index 35b9177..cba6afc 100644
>--- a/libs/widgets/itemview/itemviewimagedelegate.cpp
>+++ b/libs/widgets/itemview/itemviewimagedelegate.cpp
>@@ -62,11 +62,13 @@ ItemViewImageDelegatePrivate::ItemViewImageDelegatePrivate()
> 
>     // painting constants
>     radius    = 3;
>-    margin    = 5;
>+    margin    = 14;
> 
>     makeStarPolygon();
> 
>-    ratingPixmaps = QVector<QPixmap>(10);
>+    setGroupBackgroundColors();
>+
>+    ratingPixmaps = QVector<QPixmap>(15);
> }
> 
> void ItemViewImageDelegatePrivate::init(ItemViewImageDelegate* const _q)
>@@ -77,6 +79,28 @@ void ItemViewImageDelegatePrivate::init(ItemViewImageDelegate* const _q)
>                q, SLOT(slotThemeChanged()));
> }
> 
>+void ItemViewImageDelegatePrivate::setGroupBackgroundColors()
>+{
>+    grpBackgroundColors["Breeze"]                = QColor(200, 200, 200);
>+    grpBackgroundColors["Breeze Dark"]           = QColor(200, 200, 200);
>+    grpBackgroundColors["Breeze High Contrast"]  = QColor(200, 200, 200);
>+    grpBackgroundColors["ColorContrast"]         = QColor(200, 200, 200);
>+    grpBackgroundColors["DarkRoom"]              = QColor(200, 200, 200);
>+    grpBackgroundColors["GrayCard"]              = QColor(200, 200, 200);
>+    grpBackgroundColors["Honeycomb"]             = QColor(200, 200, 200);
>+    grpBackgroundColors["LowKey"]                = QColor(200, 200, 200);
>+    grpBackgroundColors["Norway"]                = QColor(200, 200, 200);
>+    grpBackgroundColors["Obsidian Coast"]        = QColor(200, 200, 200);
>+    grpBackgroundColors["Oxygen"]                = QColor(200, 200, 200);
>+    grpBackgroundColors["Oxygen Cold"]           = QColor(200, 200, 200);
>+    grpBackgroundColors["Steel"]                 = QColor(200, 200, 200);
>+    grpBackgroundColors["SunsetColor"]           = QColor(200, 200, 200);
>+    grpBackgroundColors["Wonton Soup"]           = QColor(200, 200, 200);
>+    grpBackgroundColors["Zion"]                  = QColor(200, 200, 200);
>+    grpBackgroundColors["Zion (Reversed)"]       = QColor(200, 200, 200);
>+    grpBackgroundColors["Default"]               = grpBackgroundColors["Breeze"] ;
>+}
>+
> void ItemViewImageDelegatePrivate::clearRects()
> {
>     gridSize   = QSize(0, 0);
>@@ -267,7 +291,7 @@ void ItemViewImageDelegate::invalidatePaintingCache()
> }
> 
> QRect ItemViewImageDelegate::drawThumbnail(QPainter* p, const QRect& thumbRect, const QPixmap& background,
>-                                           const QPixmap& thumbnail) const
>+                                           const QPixmap& thumbnail, bool isGrouped) const
> {
>     p->drawPixmap(0, 0, background);
> 
>@@ -293,24 +317,38 @@ QRect ItemViewImageDelegate::drawThumbnail(QPainter* p, const QRect& thumbRect,
> 
>     p->drawPixmap(0, 0, background);
> */
>-    QPixmap borderPix = thumbnailBorderPixmap(actualPixmapRect.size());
>-    p->drawPixmap(actualPixmapRect.x()-3, actualPixmapRect.y()-3, borderPix);
>+    QPixmap borderPix = thumbnailBorderPixmap(actualPixmapRect.size(), isGrouped);
>+
>+    if(isGrouped)
>+    {
>+        const int xPadding = (borderPix.width()-actualPixmapRect.width())/2;
>+        const int yPadding = (borderPix.height()-actualPixmapRect.height())/2;
>+
>+        p->drawPixmap(actualPixmapRect.x()-xPadding,
>+                      actualPixmapRect.y()-yPadding, borderPix);
>+    }
>+    else
>+    {
>+        p->drawPixmap(actualPixmapRect.x()-IMAGE_BORDER_RADIUS,
>+                      actualPixmapRect.y()-IMAGE_BORDER_RADIUS, borderPix);
>+    }
> 
>     p->drawPixmap(r.x() + (r.width()-thumbnail.width())/2,
>                   r.y() + (r.height()-thumbnail.height())/2,
>                   thumbnail);
>+
>     //p->restore();
>     return actualPixmapRect;
> }
> 
> void ItemViewImageDelegate::drawRating(QPainter* p, const QModelIndex& index, const QRect& ratingRect,
>-                                       int rating, bool isSelected) const
>+                                       int rating, int bgType) const
> {
>     Q_D(const ItemViewImageDelegate);
> 
>     if (d->editingRating != index)
>     {
>-        p->drawPixmap(ratingRect, ratingPixmap(rating, isSelected));
>+        p->drawPixmap(ratingRect, ratingPixmap(rating, bgType));
>     }
> }
> 
>@@ -622,6 +660,7 @@ void ItemViewImageDelegate::prepareBackground()
>     {
>         d->regPixmap = QPixmap();
>         d->selPixmap = QPixmap();
>+        d->grpPixmap = QPixmap();
>     }
>     else
>     {
>@@ -636,6 +675,15 @@ void ItemViewImageDelegate::prepareBackground()
>         QPainter p2(&d->selPixmap);
>         p2.setPen(qApp->palette().color(QPalette::Midlight));
>         p2.drawRect(0, 0, d->rect.width()-1, d->rect.height()-1);
>+
>+        // getting grouped background color
>+        QColor groupedImagesBGColor = d->grpBackgroundColors[ThemeManager::instance()->currentThemeName()];
>+
>+        d->grpPixmap = QPixmap(d->rect.width(), d->rect.height());
>+        d->grpPixmap.fill(groupedImagesBGColor);
>+        QPainter p3(&d->grpPixmap);
>+        p3.setPen(qApp->palette().color(QPalette::Midlight));
>+        p3.drawRect(0, 0, d->rect.width()-1, d->rect.height()-1);
>     }
> }
> 
>@@ -653,33 +701,34 @@ void ItemViewImageDelegate::prepareRatingPixmaps(bool composeOverBackground)
>     // We use antialiasing and want to pre-render the pixmaps.
>     // So we need the background at the time of painting,
>     // and the background may be a gradient, and will be different for selected items.
>-    // This makes 5*2 (small) pixmaps.
>+    // This makes 5*3 (small) pixmaps.
> 
>-    for (int sel=0; sel<2; ++sel)
>+    for (int sel=0; sel<3; ++sel)
>     {
>         QPixmap basePix;
> 
>-        if (composeOverBackground)
>-        {
>-            // do this once for regular, once for selected backgrounds
>-            if (sel)
>-            {
>-                basePix = d->selPixmap.copy(d->ratingRect);
>-            }
>-            else
>-            {
>-                basePix = d->regPixmap.copy(d->ratingRect);
>-            }
>-        }
>-        else
>-        {
>-            basePix = QPixmap(d->ratingRect.size());
>-            basePix.fill(Qt::transparent);
>-        }
>+//        if (composeOverBackground)
>+//        {
>+//            // do this once for regular, once for selected backgrounds, and once for grouped backgrounds
>+//            if(sel == 2)
>+//            {
>+//                basePix = d->grpPixmap.copy(d->ratingRect);
>+//            }
>+//            else if(sel == 1)
>+//            {
>+//                basePix = d->selPixmap.copy(d->ratingRect);
>+//            }
>+//            else
>+//            {
>+//                basePix = d->regPixmap.copy(d->ratingRect);
>+//            }
>+//        }
>+        basePix = QPixmap(d->ratingRect.size());
>+        basePix.fill(Qt::transparent);
> 
>         for (int rating=1; rating<=5; ++rating)
>         {
>-            // we store first the 5 regular, then the 5 selected pixmaps, for simplicity
>+            // we store first the 5 regular, then the 5 selected ,then the 5 grouped pixmaps
>             int index = (sel * 5 + rating) - 1;
> 
>             // copy background
>@@ -707,7 +756,7 @@ void ItemViewImageDelegate::prepareRatingPixmaps(bool composeOverBackground)
>     }
> }
> 
>-QPixmap ItemViewImageDelegate::ratingPixmap(int rating, bool selected) const
>+QPixmap ItemViewImageDelegate::ratingPixmap(int rating, int backgroundType) const
> {
>     Q_D(const ItemViewImageDelegate);
> 
>@@ -718,7 +767,11 @@ QPixmap ItemViewImageDelegate::ratingPixmap(int rating, bool selected) const
> 
>     --rating;
> 
>-    if (selected)
>+    if(backgroundType == 2)
>+    {
>+        return d->ratingPixmaps.at(10 + rating);
>+    }
>+    else if (backgroundType == 1)
>     {
>         return d->ratingPixmaps.at(5 + rating);
>     }
>diff --git a/libs/widgets/itemview/itemviewimagedelegate.h b/libs/widgets/itemview/itemviewimagedelegate.h
>index 16e1f0a..bfcabd9 100644
>--- a/libs/widgets/itemview/itemviewimagedelegate.h
>+++ b/libs/widgets/itemview/itemviewimagedelegate.h
>@@ -100,8 +100,9 @@ Q_SIGNALS:
> protected:
> 
>     /// Use the tool methods for painting in subclasses
>-    QRect drawThumbnail(QPainter* p, const QRect& thumbRect, const QPixmap& background, const QPixmap& thumbnail) const;
>-    void drawRating(QPainter* p, const QModelIndex& index, const QRect& ratingRect, int rating, bool isSelected)  const;
>+    QRect drawThumbnail(QPainter* p, const QRect& thumbRect,
>+                        const QPixmap& background, const QPixmap& thumbnail, bool isGrouped) const;
>+    void drawRating(QPainter* p, const QModelIndex& index, const QRect& ratingRect, int rating, int bgType)       const;
>     void drawName(QPainter* p,const QRect& nameRect, const QString& name)                                         const;
>     void drawTitle(QPainter *p, const QRect& titleRect, const QString& title)                                     const;
>     void drawComments(QPainter* p, const QRect& commentsRect, const QString& comments)                            const;
>@@ -126,7 +127,7 @@ protected:
> 
>     /** Returns the relevant pixmap from the cached rating pixmaps.
>      */
>-    QPixmap ratingPixmap(int rating, bool selected) const;
>+    QPixmap ratingPixmap(int rating, int backgroundType) const;
> 
>     virtual QAbstractItemDelegate* asDelegate();
> 
>diff --git a/libs/widgets/itemview/itemviewimagedelegatepriv.h b/libs/widgets/itemview/itemviewimagedelegatepriv.h
>index 5c38998..a5159e2 100644
>--- a/libs/widgets/itemview/itemviewimagedelegatepriv.h
>+++ b/libs/widgets/itemview/itemviewimagedelegatepriv.h
>@@ -33,6 +33,7 @@
> #include <QFont>
> #include <QPainter>
> #include <QPolygon>
>+#include <QMap>
> 
> // Local includes
> 
>@@ -57,6 +58,8 @@ public:
> 
>     void makeStarPolygon();
> 
>+    void setGroupBackgroundColors();
>+
>     /// Resets cached rects. Remember to reimplement in subclass for added rects.
>     virtual void clearRects();
> 
>@@ -70,6 +73,7 @@ public:
> 
>     QPixmap                   regPixmap;
>     QPixmap                   selPixmap;
>+    QPixmap                   grpPixmap;
>     QVector<QPixmap>          ratingPixmaps;
> 
>     QFont                     font;
>@@ -90,6 +94,8 @@ public:
>     QRect                     oneRowComRect;
>     QRect                     oneRowXtraRect;
> 
>+    QMap<QString,QColor>      grpBackgroundColors;
>+
>     // constant values for drawing
>     int                       radius;
>     int                       margin;
>diff --git a/libs/widgets/mainview/thumbbardock.cpp b/libs/widgets/mainview/thumbbardock.cpp
>index 21d42a6..2910339 100644
>--- a/libs/widgets/mainview/thumbbardock.cpp
>+++ b/libs/widgets/mainview/thumbbardock.cpp
>@@ -239,7 +239,7 @@ void ThumbBarDock::showThumbBar(bool status)
> QPixmap ThumbBarDock::generateFuzzyRect(const QSize& size, const QColor& color, int radius)
> {
>     QPixmap pix(size);
>-    pix.fill(Qt::transparent);
>+    pix.fill(Qt::white);
> 
>     QPainter painter(&pix);
>     painter.setRenderHint(QPainter::Antialiasing, true);
>@@ -304,4 +304,37 @@ QPixmap ThumbBarDock::generateFuzzyRect(const QSize& size, const QColor& color,
>     return pix;
> }
> 
>+
>+QPixmap ThumbBarDock::generateFuzzyRectForGroup(const QSize& size, const QColor& color, int radius)
>+{
>+    // Create two normal borders
>+    QPixmap border1 = generateFuzzyRect(size, color,radius);
>+    QPixmap border2 = border1.copy();
>+
>+    QTransform rm;
>+    // Rotate first border right by 3 degrees
>+    rm.rotate(3);
>+    border1 = border1.transformed(rm, Qt::SmoothTransformation);
>+
>+    // Rotate second border left by 3 degrees
>+    rm.rotate(-6);
>+    border2 = border2.transformed(rm, Qt::SmoothTransformation);
>+
>+    // Combine both borders
>+    int width  = std::max(border1.size().width()  , border2.size().width());
>+    int height = std::max(border1.size().height() , border2.size().height());
>+
>+    QPixmap result(QSize(width, height));
>+    result.fill(Qt::transparent); // force alpha channel
>+    {
>+        QPainter painter(&result);
>+        painter.setRenderHints(QPainter::Antialiasing, true);
>+        painter.drawPixmap(0, 0, border1);
>+        painter.drawPixmap(0, 0, border2);
>+    }
>+
>+    return result;
>+}
>+
>+
> } // namespace Digikam
>diff --git a/libs/widgets/mainview/thumbbardock.h b/libs/widgets/mainview/thumbbardock.h
>index 655bf11..2b7f136 100644
>--- a/libs/widgets/mainview/thumbbardock.h
>+++ b/libs/widgets/mainview/thumbbardock.h
>@@ -123,7 +123,8 @@ public:
>     void setShouldBeVisible(bool);
>     void restoreVisibility();
> 
>-    static QPixmap generateFuzzyRect(const QSize& size, const QColor& color, int radius);    
>+    static QPixmap generateFuzzyRect(const QSize& size, const QColor& color, int radius);
>+    static QPixmap generateFuzzyRectForGroup(const QSize& size, const QColor& color, int radius);
> 
> public Q_SLOTS:
>

Dear Mr Gilles, 

I'm sorry for the last comment, it seems that i don't have the previlledge to delete this comment.

The updates:
1- Now each theme has a customizable color for the background of grouped images when expanded, i'll tomorrow change these colors to be user friendly, now it's the same gray level for all themes, but it's customizable, we can change it for each theme independently.

2- rating rectangle now is not overlaying group borders.

In progress:
- updating margin with thumbnail size
- improving look and feel for the group border background images.

Created attachment 99002 [details]
group border fitting, different background colors for grouped images.

Jens, 

Did you test the last Omar patch against current implementation from git/master ? Any feedback ?

Gilles Caulier

Right now, I do not have the chance to compile KDE or Digikam, but I will happily review and comment any screenshots. Sorry ... maybe in two weeks time.

Sure.
I'll max tomorrow send you a list of screenshots with the colors for feedback, I've found a side effect from my patch and i'm working on it and will get back to you soon.

Omar

Omar,

the thumbnails in the icon view are now much larger with a wider frame. The frame around the thumbnails looks visually now different. The thumbnails should not change the appearance. The rotating stack must not be larger than the thumbnail image. The thumbnail image must be drawn smaller over the rotated stack. And the background color here is black with a dark theme and gray with a bright theme. I also believe that colorize the background color is not a good idea in different themes. I use a self-created theme. Maybe change only the frame color from the thumbnail image...

I have also a compiler error:

itemviewimagedelegate.cpp:296
Q_D(ItemViewImageDelegate); ==> Q_D(const ItemViewImageDelegate);

Maik

I just updated to Digikam 5.3 and this patch does not seem to be applied to it yet. Is there something I need to configure to have this functionality?

The patch is not yet integrated because Its not visually suitable and do not work yet in all conditions. The basis is here but GUI code need to be rewrites.

Gilles caulier

OK, no hurry. .)
Anything I can do to help (non-coder)? I have some more time now.

Any news here? Anything I can do to help? Thanks!

Jens,

Nothing for the moment. It still in my TODO list.

If you want to take a look in the patch, in fact the main implementation is already done, but the visual rendering need to be rewritten.

In fact i need a good idea. A mockup will be welcome. Perhaps also a search about solutions from other proprietary application can be a good inspiration

Gilles Caulier

I think the mockup Jens provided in the first place is quite good, but just the color thing. Of course the rotated frames are a nice gimmick but the colors will do, maybe with just a thicker frame for the group leader; better than fumbling endless with the rotation.
And if it really has to be a frame which indicates something "behind" the gruop leader and the rotation is the problem: why not just something like the icon under the thumbnail, stacked in a diagonal way?

Wolfgang

I think modifying the thumbnail is essential here, otherwise it's not obvious that this is actually a group. An additional icon or *only* the background will not do.

But if the thumbnail size cannot be changed or causes other headaches, how about a folded-corner effect like this one here?: http://www.freepik.com/free-vector/folded-corner_835380.htm

The corner should not cover half the thumbnail of course, but it must be clearly visible and obvious in all thumbnail sizes (and the corner might even contain the number of grouped images). I would be willing to trade my original "rotated image frame stack" idea for this one. and it might be easier to implement because the thumbnail size doesn't change.

Bonus points if the folded page corner actually displays the lower right bit of the *second* grouped image, maybe brightened up a little to differentiate it from the main thumbnail. :-)

What do you think?

Quite nice, that one!
What I was talking about is not an additional icon but a stacked frame *like* the icon. Might be easier than a rotated frame.

My originally suggested "rotated" frame can well be precomputed and just added around the image as a border. I wasn't implying the frame should be recomputed on every image (although this was the level of polish that iPhoto received from Apple ... ;) )

Created attachment 104161 [details]
patch version 4 updated with current implementation from git/master

Jens,

I updated the patch to be compilable with current implementation from git/Master. I don't introduce the SVG idea.

The result is not too bad. There are some problems with icons overlays, as geolocation and grouped indicator.

I like the isdea to apply a background color of all grouped items when this one is expanded. This color case of use need to be tested when more than one group is present in same album and when there are expanded.


The grouped thumb frame work as expected, even if large thumbnails size is selected in setup dialog.

Activating also all thumbnails info, as rating, tags, comments, file name, date, etc, sound like not broken, as with previous version 1 or 2 of the patch.

I'm not happy with the patch where some hardcoded enum are used as well. The code also depends of color theme used for icon view, and it's hardcoded.

I don't yet tested the CPU use when an album has a large amount of grouped items. Typically, the grouped thumb frame must be cached in memory. This must be fast, but i want to be sure that non time latency is not introduced by the code.

Ti resume : the patch need to be adjust, polished and tested again and again.

Gilles Caulier

Gilles,
that sounds great!
Can you create an appimage? I'll help you test it.
Jens

Hi Jens,

Not for the moment. I'm not at home without all my VM. Please weait next week.

We need also to fix some problem with icon overlays before. As well it's not suitbale if some thumbnails options are disabed.

Gilles Caulier

Created attachment 104371 [details]
patch version 5 updated with current implementation from git/master

I didn't change anything on the patch, just resolved merge issues with current master (48988b3570740a9c76dd84c147ab2e3f49fd969c).

This looks pretty good and I could not "feel" any performance impact, but I didn't measure in any way.
It would be better if the background indication for expanded groups would span the gap between the individual icons to make it clearer which belong together, but I guess that's pretty hard to do. Also a frame over all images of one group would be good.
I don't like using color for this, as there is already the color label. I think having two meaning for colors would be confusing. Just having one "color" (shade) is IMO clear and elegant.

Hi Simon,

I agree that no performance issue is visible with grouped frame pixmap. The cache is well used...

But look well in patch :

     if (!d->ratingRect.isNull())
     {
-        drawRating(p, index, d->ratingRect, info.rating(), isSelected);
+        int backgroundtype = 0;
+        if(isSelected)
+        {
+            backgroundtype = 1;
+        }
+        else if(isGroupExpanded)
+        {
+            backgroundtype = 2;
+        }
+
+        drawRating(p, index, d->ratingRect, info.rating(), backgroundtype);
     }

Why this kind of code for rating widget ? Why hard coded values in source code ?

As i said previously, there is also a layout problem with icon-view item contents. Go to Setup dialog, and turn on HiDPI compatible thumbnail size (<=512 px).

Now, turn on all icon-view item properties to be visible, and play with thumbnails size slider from status bar. Look the result, for ex, with GPS location indicator. The layout is broken...

Gilles

Created attachment 104642 [details]
patch version 6 updated with current implementation from git/master

New version of patch to compile against current git/master.

Simon, 

The patch version 6 work well. We near to finalize it.

Maik, 

Please test with all icon view options enabled. Change thumbnails size and look if all items info still displayed as expected.

Gilles

Maik,

Outside the current patch, the git/master implementation has a problem with GPS icon overlay. It do not follow the thumbnail size as other icon overlay (rotate, selection, etc).

This problem do not exist with 4.x release. It think this have been introduced with breeze icons patch in source code while 5.2/5.3 releases.

Look my screenshot for details:

https://www.flickr.com/photos/digikam/33396428281/

Can you reproduce ?

Gilles

Maik,

Same problem with patch applied. The position of GPS icon overlay is a little bit different too...

https://www.flickr.com/photos/digikam/33396556291

Gilles

Git commit b425579e132fd21cb3be963bea50e5ab30678316 by Maik Qualmann.
Committed on 19/03/2017 at 09:46.
Pushed by mqualmann into branch 'master'.

fix small size from the globe icon

M  +2    -2    libs/widgets/itemview/itemviewimagedelegate.cpp
M  +2    -2    showfoto/thumbbar/itemviewshowfotodelegate.cpp
M  +2    -2    utilities/importui/items/itemviewimportdelegate.cpp

https://commits.kde.org/digikam/b425579e132fd21cb3be963bea50e5ab30678316

Simon, Maik,

If this patch do not introduce dysfunctions for you, it can be applied to git/master and proposed to end users as testing through pre-release bundles.

Gilles

No, the patch is not ready yet. The geolocation icon position is not yet correct. I know why, but I want to optimize some things.

Maik

Created attachment 104647 [details]
patch version 7

- visual appearance of non-grouped thumbnails is now unchanged
- geolocalization icon at the correct position

What I do not like is the background color for opened grouped thumbnails. I am still searching for a better idea.

Maik

MAik,

Look also that an enum must be used here, about background color :

+        if (isSelected)
+        {
+            backgroundtype = 1;
+        }
+        else if (isGroupExpanded)
+        {
+            backgroundtype = 2;

The background color is a good idea, if there is only one group open in an album. In case of multiple groups expanded, this become unsuitable.

I thinking about a special color label, but this will break color label feature with grouped images.

Gilles

Gilles

How about we visually (!) treat an opened group like a subalbum at the respective position in the list of images? the master image should then also be in the subalbum and must still be highlighted somehow but this can be done with E. g. a thicker border frame (instead of another color). 

Yes, this would require a rearrangement of all thumbnails in the album (since a row is probably being split up) but it would be consistent.

Another idea is to reduce the opacity of opened grouped thumbnails to e. g. 50 percent  but only(!) while the mouse is not over one of the group. this does not affect visibility while relevant, it indicates clearly what is grouped, and it does not interfere with color schemes. Also, it creates the possibility of correctly sorting grouped images in between non grouped ones when the group is opened.

Maik,

I like the "mouse over with 50% transparency" idea. What do you think about ?

Gilles

Git commit 2a54e446e2a9cf04d9b79360d1d12405a188a2c3 by Maik Qualmann.
Committed on 20/03/2017 at 19:49.
Pushed by mqualmann into branch 'master'.

apply visual patch for grouped images without background color function

M  +3    -1    app/items/imagedelegate.cpp
M  +22   -6    libs/widgets/itemview/ditemdelegate.cpp
M  +1    -1    libs/widgets/itemview/ditemdelegate.h
M  +31   -6    libs/widgets/itemview/itemviewimagedelegate.cpp
M  +20   -20   libs/widgets/itemview/itemviewimagedelegate.h
M  +34   -2    libs/widgets/mainview/thumbbardock.cpp
M  +2    -1    libs/widgets/mainview/thumbbardock.h

https://commits.kde.org/digikam/2a54e446e2a9cf04d9b79360d1d12405a188a2c3

Currently the "grouping indication" (rotated frames in background) are visible whether the group is expanded or not. I think it should only be visible when the group is not expanded.

Git commit 5cd5b23b6ddb9eab927f1e7a68e593c138c7c538 by Maik Qualmann.
Committed on 23/03/2017 at 20:18.
Pushed by mqualmann into branch 'master'.

draw grouped border only if the group closed

M  +4    -1    app/items/imagedelegate.cpp

https://commits.kde.org/digikam/5cd5b23b6ddb9eab927f1e7a68e593c138c7c538

I would really like to see/review this change. Is there a chance to replace 1-2 compiled files in the appimage (libraries etc) or will you compile new appimages anyway in a few days?
Thanks!

I will recompile the AppImage this week end as well...

Gilles Caulier

Anyway, yesterday i recompiled the AppImage. It do not have teh last commit from Maik, but all the rest is already inside. It's at usual place in GDrive repository.

Gilles Caulier

Is the transparency / opacity patch already applied to git master? See comment 76, idea 2.

No this idea is not implemented. This can be interesting effectively...

Q: Why this bug is still open as patch was applied to git/master ? As the original idea is implemented in current 5.6.0 version, i think this file can be closed and another one open with the idea 2 from comment 76.

Gilles Caulier

Agreed. Will open a new bug.

Jens,

For a reason that i don't know, the digiKam 5.6.0 bundles and tarball still in pending queue to be published on KDE download server. Typically it must be published one week ago. Nothing happen, KDE admin do not process files on queue, and digiKam is not alone in this case.

So, i published the official 5.6.0 files on GDrive repository, at least...

https://drive.google.com/drive/folders/0BzeiVr-byqt5Y0tIRWVWelRJenM

Gilles Caulier