Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Idea: Add shadow background to covers #68

Closed
fossfreedom opened this issue Oct 16, 2012 · 13 comments
Closed

Idea: Add shadow background to covers #68

fossfreedom opened this issue Oct 16, 2012 · 13 comments
Milestone

Comments

@fossfreedom
Copy link
Owner

tip

issue #24

subtle but nice - investigate if possible to implement this without slowing down load. May have to wait until progressive load issue is resolved.

@ghost ghost assigned fossfreedom Oct 18, 2012
@ghost ghost assigned asermax Dec 13, 2012
@asermax
Copy link
Contributor

asermax commented Dec 13, 2012

Here is the code used on Noise to create the shadow. Probably gonna do something similar bu trying to stay with Cairo and GDK only (Granite seems to be a vala framework for this kind of stuff).

@fossfreedom
Copy link
Owner Author

well found - that should be a very useful reference.

Previously we were worried about the performance hit this could/may make doing this additional graphics processing.

I note in the progressive issue you mentioned the idea of a new "ShowingPolicy" class - I presume this would concentrate on changing the display for just the covers in the main visible part of the window?

@asermax
Copy link
Contributor

asermax commented Dec 13, 2012

The initial idea was to create another showing policy that would choose when an album's cover should be loaded/showed, but given the good results the new structure had (at least for me, although I think @jrbastien had good results too), I don't think it's necessary to implement for now.
I was thinking on creating a subclass/wrapper/decorator around the Cover class that would take care of adding the shadow to it. The CoverManager will need a new factory method to create the Cover for an album with our without shadow depending on a setting.

@asermax
Copy link
Contributor

asermax commented Dec 13, 2012

Woot, can't believe I got it on the first try. It doesn't slow down the loading but it uses a considerable extra amount of memory, I'm guessing it's because the intermediary pixbufs :/

@fossfreedom
Copy link
Owner Author

\o/ ... °º¤ø,¸¸,ø¤º°°º¤ø,¸,ø¤°º¤ø,¸¸,ø¤º°°º¤ø,¸ \o/

very nice - will look more closely at the code later ... looks great :)

EDIT:

if you are looking for that extra bit of speed, maybe this trick would be useful

that was for gtk2 - I presume similarly for gtk3 - if during the ALBUM_LOADER_CHUNK stuff that is populating the model, if its possible to get hold of the iconview - maybe by disconnecting and reconnecting the iconview around the for-loop, the large connection you have will load that little bit faster.

@asermax
Copy link
Contributor

asermax commented Dec 13, 2012

That's the same as using the nay filter, since new albums are not shown (because they are filtered out). But that's not a problem tho, since the filter model fails to thrown row-changed signals, the cover_view doesn't know when a cover is updated and hence there's no performance issues; the bad part is that the cover isn't updated on screen even when it haves to, that's what the code on the ShowingPolicy takes care of, throwing the row-changed signal for the filtered store when the updated album is within the cover_view viewport.

@fossfreedom
Copy link
Owner Author

had a little play with deleting pixbufs after they were used, putting do_delete_thyself methods on classes that handled pixbufs trying to cleanup (deleting pixbufs that were created). No joy :(

Also with the above tried to force garbage collection (gc.collect() ) - again no joy :((

Apart from that, noticed a couple of issues.

  1. the coversize slider seems to have become disconnected.
  2. CoverManager class throws a _shadow unitialised error on start - probably because line 1070 _connect_properties is before _shadow creation - when the bind on notify::coversize is made is calls _on_notify_cover_size which in-turn tries to resize _shadow which hasnt been created yet.

EDIT: noticed you've got there and fixed that already :)

@asermax
Copy link
Contributor

asermax commented Dec 13, 2012

I've already fixed the first issue in my latests commits, gonna checkout the second. EDIT: Fixed

@asermax
Copy link
Contributor

asermax commented Dec 14, 2012

Well, I think this is done, I don't see any misbehavings so I gonna merge it. If anyone sees some problem that may be related to this, please reopen this issue~~

@fossfreedom
Copy link
Owner Author

looks good.

quick question -

Imgur

the difference size between a cover with shadow and something without a cover is intentional? Just need to know I need to explain this in the README.

@asermax
Copy link
Contributor

asermax commented Dec 14, 2012

Nup, I hadn't noticed. I'll fix it as soon as the energy comes back to my house. Whenever is a little hot around here, everyone turn on their ACs and collapse the energy net u_u

asermax added a commit that referenced this issue Dec 14, 2012
@jrbastien
Copy link
Contributor

Would it be possible to offset the shadow? This creates a more realistic effect.
The left picture is the current shadow, the right shows the shadow offset to the bottom right.

Drop Shadow Offset

@asermax
Copy link
Contributor

asermax commented Dec 15, 2012

It's posible, but I think is more a matter of taste than anything, I don't look for the shadow to be realistic, just to give the image some volume.
Neverthless, you can customize the shadow by modifying the album-shadow.png file on the img directory (conserving it's dimensions).
Here's an example that produces the effect you're looking for:
example
You can create your own shadow using the shadow filter on gimp (I'm guessing that's what you used for your image).
One thing to take into account: the cover is always drawn in the center, so the shadow should be made taking that as reference point.

If you think it's worthed to add a configuration option to select the "light source", do us a favor an open another issue so we can take care of that later (I'm trying to work out older issues first xD). If you could provide the diferent shadows for each light source, it would be even better :]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants