Emblem For (Encrypted) Volume Icons

[tsujan]

GLib may give emblems to volume icons but drawing emblems was not possible because there is no size hint in PlacesModelItem (see #41 (comment)). This PR simply gets the required icon size from PlacesView (through PlacesModel) and draws the GIcon emblem.

In my opinion, the hard task of hacking XdgIcon is an overkill for such a simple job -- we don't need emblems in other places (for now).

[palinek]

I may miss something...
but why not create/use something similar like here: https://github.com/lxde/libfm-qt/blob/master/src/folderitemdelegate.cpp#L102. The FmFileInfo contains information about icon (the GIcon)... so just adding the newRole and implementing the *::data(newRole) and using it in the *ItemDelegate to paint the emblem.

[tsujan]

@palinek
This isn't about files (GFile) but about GVolume.

[tsujan]

@palinek
... And even for GFile, that wouldn't help because FmFileInfo doesn't contain any emblem info: even if you get GFile from FmFileInfo, there will be no emblem info.

[palinek]

OK... I see, I'm missing a lot 😄

[tsujan]

@palinek
It's OK. Yesterday, I learned a lot in the 3 hours I tried to implement file emblems (to no avail).

[tsujan]

@palinek
This isn't related to the PR but I was wrong about GFile above: although FmFileInfo doesn't contain any emblem info, GFileInfo can be derived from it indirectly to provide emblem info. I think the same info could be added to FmFileInfo in libfm.

[PCMan]

@tsujan Thank you for all of the hard work. Reading others' code, understanding it correctly, and trying to enhance its features is never an easy task. Good job so far!
To make your work even better, I'd suggest thinking more about design-pattern at the same time.
The folder view/model stuff is what we called MVC pattern.
Different icon sizes are actually different ways to "view" the same data "model".
So ideally it should not be controlled by the model itself. This PR breaks the design pattern.
The best place to do the emblem hack is definitely inside QIcon, which is not possible at the moment.
As an alternative, I'd suggest:

1. Add Fm::Icon::pixmap(size, state) to render the pixmap, so this code can be reused everywhere when an emblemed icon is needed. Even better, try to cache these pixmaps to some extent since generating it is very expensive. (This is what QIcon does internally) Using g_object_set_qdata() to attach the QPixmap objects to the GIcon object is the most easy and effective way.
2. Make Fm::FolderItemDelegate support PlacesModel (better) or develop another item delegate for the places model (also works). Inside this delegate, you have Fm::FileInfo and then you can get FmIcon of it. Then, call Fm::Icon::pixmap() to render the icon.

Then it's done. No need to spread the emblem code everywhere. In the future if the GIcon/Fm::Icon stuff is changed, nothing outside these classes will be affected as they are well-encapsulated.

[tsujan]

@PCMan
It's about 3-4 months that I don't understand your arguments anymore. Maybe something has got wrong with me or .... So, no further comment.

[PCMan]

@tsujan To sum up. Your code works, which is good and I appreciate your effort. However, adding view information in data model is against the Qt MVC design pattern. Your code is good, but the problem is where to add it.There are many places that you can hack to make it work, but it's better to add it at the right place so we only need to do it once and no further refactoring is needed in the future.
Don't get me wrong. I like your contribution and appreciate that. Just wan to see if there are ways to make the API design better.
If you feel offended, that's probably my problem but I'm not intended to do so. We can also merge your PR first and later do the refactor. This requires frequent changes to library API, though.

[PCMan]

@tsujan Two views with different icon sizes can share the same data model and that's why you should not set icon size on the data model. Though we don't do this in the program at the moment, it's not good API design and might break in the future.
I'm here to help, not to reject all PRs deliberately.
I respect your contribution and hope you can understand that.

[tsujan]

@PCMan
I'm no able to feel offended -- that's part of my personality as a pragmatist. The matter is that, with the attitude you have adopted recently, my contribution would be senseless. Your arguments have become strange and, as I said, incomprehensible to me. Let's not continue this discussion -- it'll go nowhere.

[PCMan]

@tsujan OK, I'll merge your PR first and do the required refactor later before the next release. It's not about attitude. It's about design patterns. Your contribution is always welcomed, but I cannot agree with you on this one. Mixing view stuff with the data model is against the purpose of model/view separation and should generally be avoided.

[tsujan]

@PCMan
Don't merge it if you really think that your argument is correct pracically! What's important to me is not my PR but the final result, provided that there is a result/change.

[PCMan]

@tsujan Merged and did some refactor.

1. Emblem information is now cached in Fm::IconTheme and not built from GIcon everytime.
2. Now icon sizes are only controlled by PlacesView, not PlacesModel. Multiple PlacesViews can have different icon sizes and still share the same model. (but we don't use it now) This API change is still backward compatible and still split model/view/controller cleanly.
3. Reuse FolderItemDelegate to paint the items of PlacesModel to avoid code duplication. All views that need to paint an emblemed icon should reuse this. So we only need to do it once.

Thanks for the contribution. Now the places icon and file icons both support emblems.

[tsujan]

Works as intended.