Merge lp://qastaging/~uriboni/unity/unity-drag-shadow into lp://qastaging/unity

Proposed by Ugo Riboni
Status: Rejected
Rejected by: Andrea Azzarone
Proposed branch: lp://qastaging/~uriboni/unity/unity-drag-shadow
Merge into: lp://qastaging/unity
Diff against target: 84 lines (+35/-4)
2 files modified
launcher/LauncherDragWindow.cpp (+34/-4)
launcher/LauncherDragWindow.h (+1/-0)
To merge this branch: bzr merge lp://qastaging/~uriboni/unity/unity-drag-shadow
Reviewer Review Type Date Requested Status
Marco Trevisan (Treviño) Needs Fixing
Review via email: mp+119106@code.qastaging.launchpad.net

Description of the change

Add a shadow below the icon when it's being dragged out from the launcher.

To post a comment you must log in.
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Thanks for the branch, however it simply does not work as expected here: when I drag an icon out from the launcher, I get nothing painted on the screen, but one small pixel.
The problems seems to be in
SetBaseSize(DROP_SHADOW_SIZE * widthFactor, DROP_SHADOW_SIZE * heightFactor);

Also:
31 + glib::Error error;
32 + glib::Object<GdkPixbuf> pbuf(gdk_pixbuf_new_from_file_at_size(PKGDATADIR "/ordering_shadow.png",
33 + DROP_SHADOW_SIZE, DROP_SHADOW_SIZE, &error));

Since you don't LOG_ERROR the error, you can avoid to use it... Just pass nullptr to gdk_pixbuf_new_from_file_at_size

Also, instead of adding a new pixbuf, what about using the PKGDATADIR"/launcher_icon_shadow_200.png" or PKGDATADIR"/launcher_icon_shadow_62.png" textures that the AbstractIconRender is alredy using? So they will always match...

34 + if (GDK_IS_PIXBUF(pbuf.RawPtr()))

Use the nwe pbuf.IsType(GDK_TYPE_PIXBUF)

+ nux::BaseTexture* texture = nux::CreateTexture2DFromPixbuf(pbuf, true);

This should be unreferenced... You can just use a nux::ObjectPtr to handle it and forget.

PS: instead of checking != nullptr we prefer to have simple (check) or (!check) ;)

review: Needs Fixing
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Ugo, FYI this diff fixes the issues above: http://paste.ubuntu.com/1144653/ (the size problem I had was caused by a division with an integer).

However, I'm still not getting the correct design when dragging the icon outside the launcher: http://i.imgur.com/ALpcH.png

2546. By Ugo Riboni

Merge changes from trunk

2547. By Ugo Riboni

Use proper flat math and properly handle adopted textures. Thanks Treviño.

Revision history for this message
Andrea Azzarone (azzar1) wrote :

Fixed in trunk.

Unmerged revisions

2547. By Ugo Riboni

Use proper flat math and properly handle adopted textures. Thanks Treviño.

2546. By Ugo Riboni

Merge changes from trunk

2545. By Ugo Riboni

Remove another unnecessary include

2544. By Ugo Riboni

Minimize whitespace changes

2543. By Ugo Riboni

Use same indent style as the rest of the file

2542. By Ugo Riboni

Remove useless includes and use constants instead of defines

2541. By Ugo Riboni

Add a drop shadow below the icon when it's dragged out from the launcher

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.