Bug 11990 - resize patch against PR1.3 hildon-home
: resize patch against PR1.3 hildon-home
Status: NEW
Product: Maemo 5 Community SSU
hildon-home
: unspecified
: N900 Maemo
: Unspecified enhancement with 3 votes (vote)
: ---
Assigned To: unassigned
: general
:
: patch
:
:
  Show dependency tree
 
Reported: 2011-02-27 10:23 UTC by Ricky Tournee
Modified: 2012-06-01 15:02 UTC (History)
5 users (show)

See Also:


Attachments
resize patch against PR1.3 hildon-home (2.77 KB, patch)
2011-02-27 10:23 UTC, Ricky Tournee
Details
Resize and show / hide patch against PR1.3 hildon-home (all files) (15.84 KB, patch)
2011-02-27 11:44 UTC, Ricky Tournee
Details
Sources of the old and new hildon-homes (502.97 KB, application/x-tar)
2011-02-27 11:48 UTC, Ricky Tournee
Details
Patch to hide background of shortcuts via gconf /apps/osso/hildon-home/task-shortcuts-back (3.01 KB, patch)
2011-03-08 02:22 UTC, paiburio
Details
3319 with int shortcuts-back changed to bool shortcuts-hide-back (2.93 KB, patch)
2011-03-08 02:50 UTC, paiburio
Details
a scaling patch for bookmarks and icons, should be used after tk's patch (18.06 KB, patch)
2011-03-09 17:55 UTC, Asaf Elias
Details
a scaling patch that includes tk's background patches (18.38 KB, patch)
2011-03-09 17:58 UTC, Asaf Elias
Details
3320 with reduced widget-size when hiding shortcut-backgrounds (3.36 KB, patch)
2011-03-11 14:54 UTC, paiburio
Details


Note

You need to log in before you can comment on or make changes to this bug.


Description Ricky Tournee (reporter) 2011-02-27 10:23:31 UTC
Created an attachment (id=3313) [details]
resize patch against PR1.3 hildon-home

Patch to make desktop shortcuts resizable and ability to show / hide background
png of them.
Comment 1 Andrew Flegg maemo.org 2011-02-27 10:36:48 UTC
Why does it save pixmaps to /home/user? Ignoring the unfriendliness of that
location, what are they for?
Comment 2 Ricky Tournee (reporter) 2011-02-27 10:39:38 UTC
(In reply to comment #1)
> Why does it save pixmaps to /home/user? Ignoring the unfriendliness of that
> location, what are they for?

This patch makes the homescreen app shortcuts to be resizable via gconf -value
(UI for this is Theme Customizer). Or did I missunderstood your question?
Comment 3 Andrew Flegg maemo.org 2011-02-27 11:21:26 UTC
I didn't make myself clear. What are the following lines for?

+  GdkPixbuf *pixbuf;
+  pixbuf = gdk_pixbuf_new_from_file
("/etc/hildon/theme/images/ApplicationShortcutApplet.png", NULL);
+  pixbuf = gdk_pixbuf_scale_simple(pixbuf, task_shortcut_width,
task_shortcut_width, GDK_INTERP_BILINEAR);
+  gdk_pixbuf_save (pixbuf, "/home/user/applet1.png", "png", NULL, "quality",
"100", NULL);
+
+  GdkPixbuf *pixbuf2;
+  pixbuf2 = gdk_pixbuf_new_from_file
("/etc/hildon/theme/images/ApplicationShortcutAppletPressed.png", NULL);
+  pixbuf2 = gdk_pixbuf_scale_simple(pixbuf2, task_shortcut_width,
task_shortcut_width, GDK_INTERP_BILINEAR);
+  gdk_pixbuf_save (pixbuf2, "/home/user/applet2.png", "png", NULL, "quality",
"100", NULL);
+
+  GdkPixbuf *pixbuf3;
+  pixbuf3 = gdk_pixbuf_new_from_file
("/etc/hildon/theme/images/toolbar_button_disabled.png", NULL);
+  pixbuf3 = gdk_pixbuf_scale_simple(pixbuf3, 1, 1, GDK_INTERP_BILINEAR);
+  gdk_pixbuf_save (pixbuf3, "/home/user/applet3.png", "png", NULL, "quality",
"100", NULL);

If these files are required:
  a) It should be using $HOME, rather than having /home/user hardcoded.
  b) It shouldn't be putting them in the top-level of the user's home
directory.

Where and how does task_shortcuts_back get used?
Comment 4 Ricky Tournee (reporter) 2011-02-27 11:44:27 UTC
Created an attachment (id=3314) [details]
Resize and show / hide patch against PR1.3 hildon-home (all files)

Let's try again with all files in the .patch.
Comment 5 Ricky Tournee (reporter) 2011-02-27 11:45:33 UTC
(In reply to comment #3)
> I didn't make myself clear. What are the following lines for?
> 
> +  GdkPixbuf *pixbuf;
> +  pixbuf = gdk_pixbuf_new_from_file
> ("/etc/hildon/theme/images/ApplicationShortcutApplet.png", NULL);
> +  pixbuf = gdk_pixbuf_scale_simple(pixbuf, task_shortcut_width,
> task_shortcut_width, GDK_INTERP_BILINEAR);
> +  gdk_pixbuf_save (pixbuf, "/home/user/applet1.png", "png", NULL, "quality",
> "100", NULL);
> +
> +  GdkPixbuf *pixbuf2;
> +  pixbuf2 = gdk_pixbuf_new_from_file
> ("/etc/hildon/theme/images/ApplicationShortcutAppletPressed.png", NULL);
> +  pixbuf2 = gdk_pixbuf_scale_simple(pixbuf2, task_shortcut_width,
> task_shortcut_width, GDK_INTERP_BILINEAR);
> +  gdk_pixbuf_save (pixbuf2, "/home/user/applet2.png", "png", NULL, "quality",
> "100", NULL);
> +
> +  GdkPixbuf *pixbuf3;
> +  pixbuf3 = gdk_pixbuf_new_from_file
> ("/etc/hildon/theme/images/toolbar_button_disabled.png", NULL);
> +  pixbuf3 = gdk_pixbuf_scale_simple(pixbuf3, 1, 1, GDK_INTERP_BILINEAR);
> +  gdk_pixbuf_save (pixbuf3, "/home/user/applet3.png", "png", NULL, "quality",
> "100", NULL);
> 
> If these files are required:
>   a) It should be using $HOME, rather than having /home/user hardcoded.
>   b) It shouldn't be putting them in the top-level of the user's home
> directory.
> 
> Where and how does task_shortcuts_back get used?

Sorry.. I'm new with this .patch thing. New attachment uploaded.
Comment 6 Ricky Tournee (reporter) 2011-02-27 11:48:59 UTC
Created an attachment (id=3315) [details]
Sources of the old and new hildon-homes
Comment 7 Andre Klapper maemo.org 2011-02-27 20:09:33 UTC
Ricky: Can you please answer comment 3? It is still unanswered... Thanks.
Comment 8 Andrew Flegg maemo.org 2011-02-27 20:38:47 UTC
It's a mix of other patches (including multiple desktops), and putting files
like book1, book2, bookmask1, applet1, applet2, applet3.png in /home/user would
need to be fixed.

There is commented out code:

+  //priv->bg_image = hd_cairo_surface_cache_get_surface
(hd_cairo_surface_cache_get (), BACKGROUND_IMAGE_FILE);
+  //priv->bg_active = hd_cairo_surface_cache_get_surface
(hd_cairo_surface_cache_get (), BACKGROUND_ACTIVE_IMAGE_FILE);

And the lines following it are inconsistent with indenting.
Comment 9 Andrew Flegg maemo.org 2011-02-27 20:42:24 UTC
However, the patch could be a starting point for an interesting dev to pick it
up.
Comment 10 Andrew Flegg maemo.org 2011-02-27 20:44:10 UTC
However, the patch could be a starting point for an interested dev to pick it
up.
Comment 11 Ricky Tournee (reporter) 2011-02-27 20:54:09 UTC
(In reply to comment #7)
> Ricky: Can you please answer comment 3? It is still unanswered... Thanks.

As far as I know the thing in nutshell is:
- read stored gconf value (height, width or visible / non-visible)
- resize images used in widget and store them into /home/user
- show widget with resized images

I didn't write this stuff so can't provide any more info, but I suppose this
could be easily improved and included with CSSU. Many users like and have got
used to this feature already.
Comment 12 paiburio 2011-03-08 02:22:59 UTC
Created an attachment (id=3319) [details]
Patch to hide background of shortcuts via gconf
/apps/osso/hildon-home/task-shortcuts-back

In the previous patch, the background would be resized to a 1x1 png and stored
as /home/user/applet3.png. Here, bg_image and bg_active is simply set to 0 -
hd_task_shortcut_dispose and hd_task_shortcut_expose_event check for its
existence. I haven't found a problem with that yet.

Problems/Questions:
- The gconf-key task-shortcuts-back, if not set, defaults to 0. On a clean
install this would lead to invisible backgrounds.
The "clean" way IMO would be to name the key task-shortcuts-hide-back (and make
it boolean). But this would break Theme-Customizer...
- Change requires restart. Acceptable?
Comment 13 paiburio 2011-03-08 02:50:01 UTC
Created an attachment (id=3320) [details]
3319 with int shortcuts-back changed to bool shortcuts-hide-back
Comment 14 Ricky Tournee (reporter) 2011-03-08 14:57:53 UTC
(In reply to comment #12)
> The "clean" way IMO would be to name the key task-shortcuts-hide-back (and make
> it boolean). But this would break Theme-Customizer...
> - Change requires restart. Acceptable?

Completely acceptable. I'll just change Theme Customizer to follow that key
naming. Thanks for participating to this!
Comment 15 Asaf Elias 2011-03-08 23:09:24 UTC
I have a version of the scaling working as it should on the cssu version of
hildon-home
scaled images are now placed near the originals in "/etc/hildon/theme/images/"

defines of the image location have moved to hd-task-shortcut.h and
hd-bookmark-shortcut.h to make the code more readable.
instead of using
"/etc/hildon/theme/images/ApplicationShortcutAppletPressed.png" etc one can use
HDTS_BACKGROUND_ACTIVE_IMAGE_FILE where HDTS means "Hildon Desktop Task
Shortcut" same goes to the bookmarks

i just need to change all ints to gints, and find why on small numbers the
bookmarks text is not shown... (probably there is no enough space...)

i will post a patch tommarow...

AsiQue
Comment 16 Asaf Elias 2011-03-08 23:12:06 UTC
forgot to say, 
it's already integrated with tk's background patch
Comment 17 Asaf Elias 2011-03-09 17:55:21 UTC
Created an attachment (id=3321) [details]
a scaling patch for bookmarks and icons, should be used after tk's patch

Ok.. this is a patch that should be applied after tk's patches, 
as i said, i moved the images and size #defines to the header files
also added a default defines for the size key (to be used when there is no such
key instead of hardcoded values)

the scaled images are now created in the theme alongside the originals

i kept the same gconf keys as the old scaling patch

i will also upload another patch witch includes tk's and mine

ps. known bugs: if scaling a bookmark too small, the text doesn't appear
Comment 18 Asaf Elias 2011-03-09 17:58:45 UTC
Created an attachment (id=3322) [details]
a scaling patch that includes tk's background patches

this is the combined patch which includes the scaling patch and tk's background
patches

the text bug (feature :-)) is also here
Comment 19 Ricky Tournee (reporter) 2011-03-09 21:04:10 UTC
(In reply to comment #18)
> Created an attachment (id=3322) [details] [details]
> a scaling patch that includes tk's background patches
> 
> this is the combined patch which includes the scaling patch and tk's background
> patches
> 
> the text bug (feature :-)) is also here

Thanks! However I wasn't able to apply the patch against CSSU hildon-home.
Output is here:
http://www.pastie.org/1652545
Comment 20 paiburio 2011-03-10 08:08:12 UTC
(In reply to comment #17)
> Created an attachment (id=3321) [details] [details]
> a scaling patch for bookmarks and icons, should be used after tk's patch

Nice, Thanks! Mind if I pick a few nits?

> Ok.. this is a patch that should be applied after tk's patches, 

This patch works fine without mine, seems you just missed some definitions and
an include.

> as i said, i moved the images and size #defines to the header files
> also added a default defines for the size key (to be used when there is no such
> key instead of hardcoded values)
>
> the scaled images are now created in the theme alongside the originals

Problem is, they are created 1) at every start 2) even if no scaling is used.
2) could easily be fixed by putting the scaling under the else of if(error ||
!task_..._width).
1) I don't know. Try to open the target files, check their width, generate if
necessary. Is it worth the hassle? Are we writing to root?

> i kept the same gconf keys as the old scaling patch

Ok, while ..._width would be clearer..

> i will also upload another patch witch includes tk's and mine
> 
> ps. known bugs: if scaling a bookmark too small, the text doesn't appear

In hd_bookmark_shortcut_init the label is defined at a fixed offset from top,
which could easily be changed. Problem is, the area in
WebShortcutAppletBackground where the label is drawn is scaled too, and might
become smaller than the label, looking bad.
If you scale the bookmarks up, the label is still drawn at that fixed offset,
looking bad, too.
We might just hide the label when scaling (down). Limit the scale to <=
DEF_SHORTCUT_WIDTH? Good enough for me.
Or we could allow only certain steps, and leave it to the themers to provide
the backgrounds.

Should refactor the scaling into a function, it's the same for bookmarks and
shortcuts. Is that single g_object_unref enough?
AFAIKT, scale_border is not used?
Seems you got some superfluous whitespace and (gasp!) tabs in there:)

Running it now to test with scaled bookmarks. Thanks again!
Comment 21 Asaf Elias 2011-03-10 10:20:22 UTC
(In reply to comment #19)
> (In reply to comment #18)
> > Created an attachment (id=3322) [details] [details] [details]
> > a scaling patch that includes tk's background patches
> > 
> > this is the combined patch which includes the scaling patch and tk's background
> > patches
> > 
> > the text bug (feature :-)) is also here
> 
> Thanks! However I wasn't able to apply the patch against CSSU hildon-home.
> Output is here:
> http://www.pastie.org/1652545

Hey
could you post a link to the .rej files?

don't forget this pathch should be applied after pk's pathes
AsiQue
Comment 22 Asaf Elias 2011-03-10 10:31:10 UTC
(In reply to comment #20)
> Nice, Thanks! Mind if I pick a few nits?

NP

> This patch works fine without mine, seems you just missed some definitions and
> an include.

maybe, but i made the diff after your code was patched... 

> Problem is, they are created 1) at every start 2) even if no scaling is used.
> 2) could easily be fixed by putting the scaling under the else of if(error ||
> !task_..._width).
> 1) I don't know. Try to open the target files, check their width, generate if
> necessary. Is it worth the hassle? Are we writing to root?

i will move the creation of the files into the else {} 
if i think about it, checking the size is way better then recreating the
images, so will probably do that... 

> Ok, while ..._width would be clearer..

i don't have a problem to change that... opinions?

> In hd_bookmark_shortcut_init the label is defined at a fixed offset from top,
> which could easily be changed. Problem is, the area in
> WebShortcutAppletBackground where the label is drawn is scaled too, and might
> become smaller than the label, looking bad.
> If you scale the bookmarks up, the label is still drawn at that fixed offset,
> looking bad, too.
> We might just hide the label when scaling (down). Limit the scale to <=
> DEF_SHORTCUT_WIDTH? Good enough for me.
> Or we could allow only certain steps, and leave it to the themers to provide
> the backgrounds.

i don't have any problems with your suggestions... any more opinions?

> 
> Should refactor the scaling into a function, it's the same for bookmarks and
> shortcuts. Is that single g_object_unref enough?

np i will refactor them.. and i think it's enough... (well the old code didn't
have them at all :-))

> AFAIKT, scale_border is not used?

i know... just wanna be consist.. 

> Seems you got some superfluous whitespace and (gasp!) tabs in there:)

Thats ESBOX's fault :-) i will try to remove them...


> Running it now to test with scaled bookmarks. Thanks again!
Comment 23 paiburio 2011-03-11 14:54:09 UTC
Created an attachment (id=3323) [details]
3320 with reduced widget-size when hiding shortcut-backgrounds

Became aware of a problem while playing with backgroundless bookmarks (anyone
interested?):
Since the widget-size stays the same (on standard shortcuts that's 64x64px for
the icon + 16px at each edge for the background), when you hide the backgrounds
and move the shortcuts closer together, they overlap invisibly, leading to
unintended clicks on the wrong shortcut. Had this bug with the original patched
hildon-home, but I thought I was just clumsy.

This quick patch sets the widget to icon-size instead of shortcut-size when the
background is hidden.
Drawback: All shortcuts shifted 16px to left&top. Works for me, as I want to
use the edge of the screen. Other ideas?
Can post/mail merged patch with AsiQue's if needed.

PS: Is this getting out of hand? Should move to TMO?
Comment 24 Andrew Flegg maemo.org 2011-03-11 14:55:23 UTC
(In reply to comment #23)
> 
> PS: Is this getting out of hand? Should move to TMO?

No. This issue, or maemo-developers (with a "[CSSU]" tag) is the right place.
Comment 25 Daniel Sandman 2011-04-19 20:23:23 UTC
How are this bug comming on? This is the single reason why i have not installed
the CSSU yet.
Comment 26 paiburio 2011-05-06 19:55:16 UTC
Sorry, been quite busy lately. Anyway, I created a repo on
https://gitorious.org/~paiburio/community-ssu/pks-hildon-home, using the
patches found here and then some (AsiQue, hope you are ok with that), and sent
a merge-request.
Comment 27 Asaf Elias 2011-05-06 20:47:57 UTC
(In reply to comment #26)
> Sorry, been quite busy lately. Anyway, I created a repo on
> https://gitorious.org/~paiburio/community-ssu/pks-hildon-home, using the
> patches found here and then some (AsiQue, hope you are ok with that), and sent
> a merge-request.

Yeha no problem.... soryy for my absence, been busy for some time...
pk, do you remember whats left to fix on this subject?

Asique
Comment 28 Daniel Sandman 2011-05-06 21:06:32 UTC
(In reply to comment #26)
> Sorry, been quite busy lately. Anyway, I created a repo on
> https://gitorious.org/~paiburio/community-ssu/pks-hildon-home, using the
> patches found here and then some (AsiQue, hope you are ok with that), and sent
> a merge-request.

That is awesome! Will use that as for now. Finally i can use the CSSU. ;)
Comment 29 Mohammad Abu-Garbeyyeh 2011-05-30 01:43:08 UTC
I've merged the patch and tested hildon-home, I'm either doing it wrong or the
patch doesn't work correctly.

Setting /apps/osso/hildon-home/task-shortcuts-hide-bg to either true or false
keeps the background hidden for me.
The same applies to bookmarks.

Looking at the code, it doesn't seem to be listening to gconf changes, but even
a killall didn't change that behaviour.

Can you check if the patches are indeed working?
Comment 30 Asaf Elias 2011-05-30 08:24:49 UTC
(In reply to comment #29)

Yeah sure I will check it later today...
but I'm pretty sure they work... I've tested them multiple times (before pk
moved them to his repo's)

Asique
Comment 31 paiburio 2011-05-31 11:56:51 UTC
(In reply to comment #29)
> ...
> Looking at the code, it doesn't seem to be listening to gconf changes, but even
> a killall didn't change that behaviour.
> 
> Can you check if the patches are indeed working?

Well, it does not listen for changes. Do you think this is needed? I think it's
not changed that often, so is it worth the effort?

To test, I built a clean clone of CSSU/hildon-home, dsmetool'd it alive on the
device, and it works for me.

Daniel, did it work for you?
Comment 32 Joni Lampio 2012-06-01 15:02:10 UTC
*** This bug has been confirmed by popular vote. ***