Comment 1 Andrew Flegg 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?

(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 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?

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.

(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.

Created an attachment (id=3315) [details]
Sources of the old and new hildon-homes

Comment 7 Andre Klapper 2011-02-27 20:09:33 UTC

Ricky: Can you please answer comment 3? It is still unanswered... Thanks.

Comment 8 Andrew Flegg 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 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 2011-02-27 20:44:10 UTC

However, the patch could be a starting point for an interested dev to pick it
up.

(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.

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?

Created an attachment (id=3320) [details]
3319 with int shortcuts-back changed to bool shortcuts-hide-back

(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!

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

forgot to say, 
it's already integrated with tk's background patch

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

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

(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

(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!

(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

(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!

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 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.

How are this bug comming on? This is the single reason why i have not installed
the CSSU yet.

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.

(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

(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. ;)

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?

(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

(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?

*** This bug has been confirmed by popular vote. ***