On Fri, Mar 25, 2011 at 5:20 PM, Sascha Silbe <[email protected]>wrote:
> Excerpts from godiard's message of Wed Mar 23 19:40:47 +0100 2011: > > > Thanks for your patch! I can't wait for an accelerated launcher screen. ;) > Thanks for review this patch. I can't wait for faster startup in activities :) I attach a new patch and reply your comments here > > > This render the SVG image only one time, and then use alpha and scaling > > of the rendered icon and will speed up the overall activity startup. > > Any patch that claims improved performance needs to include proof of > that, i.e. test results plus a description of the test hardware and test > procedure so the results can be reproduced by the reviewer and/or a > third party. > We compared two XO with and without the patch and reading the lines like "a4a52fc2a33b5356bcce073513223d7a62fe38af launched in 2.128857 seconds." in shell.log All the activities started faster, in many cases 2 seconds faster. The effect is more visible in XO-1 I don't know if this information must be included in the comment of the patch. > > > Another side effect is that the ticket #2080 effects are reduced greatly. > > Which side effects exactly? #2080 mentions a) empty window getting > displayed for several seconds and b) the launch window getting shown > after the activity was _closed_. > The effect we are talking is, in many activities, the startup zoom was not visible at all, because the svg rendering takes more time than the 0.1 second available before the next image must be displayed. With the patch you can see the zoom effect even in XO-1 > > > Signed-off-by: Gonzalo Odiard <[email protected]> > > Acked-By: Simon Schampijer <[email protected]> > > Did he (Ack your patch)? > > Of course. I will not add this line on my own. > > [src/jarabe/view/launcher.py] > > I like that you're getting rid of the HippoCanvas based class. I'm not > so sure about not using sugar.graphics.animator.Animation anymore, > though. The old code used an exponential formula while yours is purely > linear. We might want to check with the Design Team whether that makes > a difference to the user. > > [LaunchWindow.__destroy_cb(self, box)] > > + self._activity_icon.props.pulsing = False > > + self._home.disconnect_by_func(self.__active_activity_changed_cb) > > If the launcher and with it the icon gets destroyed anyway, why do we > need to tell it to stop pulsing? (Yes, I'm aware this was already the > case in the old code - just wondering whether we really need it). > > I think the old code is stopping the animation because the destroy can be delayed by python. Does not harm anyway. > > [src/jarabe/view/pulsingicon.py] > > +_MINIMAL_ALPHA_VALUE = 0.2 > > How was this value determined? > > Experimentation and testing :) Anyway is a visual value, can be checked with Design Team too. > > [Pulser.__init__(self, icon)] > > - self._level = 0 > > + self._level = 1.0 > > self._phase = 0 > > + self._start_scale = 1.0 > > + self._end_scale = 1.0 > > I would prefer to use None for indicating no scaling is to take place > (this applies to the other patch as well). It's probably ok in this > particular place, but in general (in)equality comparisons of floating > point numbers are tricky at best. > > Ok. I agree. But in this case, the code is simplest in this way. > > + self._zoom_steps = 1 > > + self._current_zoom_step = 1 > > + self._scale = 1.0 > > + self._current_scale_step = 1 > > self._current_zoom_step seems to be a counter, duplicating self._phase > (though scaling by 1/_STEP). self._current_scale_step OTOH seems to be a > scaling factor. Please rename the variables appropriately and if > possible eliminate some of them (self._phase and self._level would be > good candidates). > > self._current_zoom_step is not the same than self._phase, because the zoom can stop and the pulsing continues. Also you can have have a pulsing icon without zooming at all (like in the neighborhood view). You are righth about self._phase and self._level, can be simplified, i did it. > Do we need to keep a copy of the current scale (i.e. self._scale)? > AFAICT assigning directly to self._icon.scale should suffice. > Ok. > [Pulser] > > + def set_zooming_properties(self, start_scale, end_scale, > zoom_steps): > > + self._start_scale = start_scale > > + self._end_scale = end_scale > > + self._zoom_steps = zoom_steps > > + self._current_scale_step = abs(self._start_scale - > self._end_scale) / \ > > + self._zoom_steps > > + self._scale = self._start_scale > > + self._icon.scale = self._scale > > Please provide a docstring explaining what this function is about (see > PEP 257). I also wonder if the caller isn't more interested in > specifying a time interval rather than the number of steps. > > Ok > set_zooming_properties sounds a bit strange, but I can't think of a much > better name. set_zooming might be a start - it should at least get rid > of some of the line continuations. :) > > If you are happy .... :) > > [Pulser.__pulse_cb(self)] > > - self._level = (math.sin(self._phase) + 1) / 2 > > + self._level = _MINIMAL_ALPHA_VALUE + \ > > + (1 - _MINIMAL_ALPHA_VALUE) * (math.sin(self._phase) + 1) > / 2 > > + if self._current_zoom_step <= self._zoom_steps: > > + if self._start_scale != self._end_scale: > > These two ifs can be combined to a single one, thus removing one level > of indentation. > > Ok. > [PulsingIcon] > > + def set_zooming_properties(self, start_size=style.SMALL_ICON_SIZE, > > + end_size=style.XLARGE_ICON_SIZE, > > + zoom_steps=10): > > + if start_size > end_size: > > + start_scale = 1.0 > > + end_scale = float(end_size) / float(start_size) > > You don't need to convert the second operand to float. If the first > operand is float, the result will always be float. > Ok > > > + if end_size > start_size: > > + start_scale = float(start_size) / float(end_size) > > + end_scale = 1.0 > > Replacing the second if with an else would avoid throwing a NameError if > start_size == end_size. > Ok > > Sascha > > -- > http://sascha.silbe.org/ > http://www.infra-silbe.de/ > -- Gonzalo
From d50dd23c20804300ce64c651e324fba7e3fa8576 Mon Sep 17 00:00:00 2001 From: Gonzalo Odiard <[email protected]> Date: Mon, 28 Mar 2011 00:29:36 -0300 Subject: [PATCH] Do startup animation of the activity icon using scaling and alpha - v3 This render the SVG image only one time, and then use alpha and scaling of the rendered icon and will speed up the overall activity startup. Another side effect is that the startup zoom not visible in many activities, is visible now - #2080 Signed-off-by: Gonzalo Odiard <[email protected]> Acked-By: Simon Schampijer <[email protected]> --- src/jarabe/view/launcher.py | 83 +++++++++------------------------------ src/jarabe/view/pulsingicon.py | 75 +++++++++++++++++++----------------- 2 files changed, 58 insertions(+), 100 deletions(-) diff --git a/src/jarabe/view/launcher.py b/src/jarabe/view/launcher.py index 89251e5..5c645c4 100644 --- a/src/jarabe/view/launcher.py +++ b/src/jarabe/view/launcher.py @@ -18,16 +18,13 @@ import logging from gettext import gettext as _ import gtk -import hippo import gobject from sugar import wm from sugar.graphics import style -from sugar.graphics import animator -from sugar.graphics.xocolor import XoColor from jarabe.model import shell -from jarabe.view.pulsingicon import CanvasPulsingIcon +from jarabe.view.pulsingicon import PulsingIcon class LaunchWindow(gtk.Window): @@ -51,12 +48,15 @@ class LaunchWindow(gtk.Window): canvas.pack_start(header, expand=False) self._activity_id = activity_id - self._box = LaunchBox(activity_id, icon_path, icon_color) - box = hippo.Canvas() - box.modify_bg(gtk.STATE_NORMAL, style.COLOR_WHITE.get_gdk_color()) - box.set_root(self._box) - box.show() - canvas.pack_start(box) + + self._activity_icon = PulsingIcon(file=icon_path, + pixel_size=style.XLARGE_ICON_SIZE) + self._activity_icon.set_base_color(icon_color) + self._activity_icon.set_zooming(style.SMALL_ICON_SIZE, + style.XLARGE_ICON_SIZE, 10) + self._activity_icon.set_pulsing(True) + self._activity_icon.show() + canvas.pack_start(self._activity_icon) footer = gtk.VBox(spacing=style.DEFAULT_SPACING) footer.set_size_request(-1, bar_size) @@ -78,11 +78,16 @@ class LaunchWindow(gtk.Window): screen = gtk.gdk.screen_get_default() screen.connect('size-changed', self.__size_changed_cb) + self._home = shell.get_model() + self._home.connect('active-activity-changed', + self.__active_activity_changed_cb) + + self.connect('destroy', self.__destroy_cb) + self._update_size() def show(self): self.present() - self._box.zoom_in() def _update_size(self): self.resize(gtk.gdk.screen_width(), gtk.gdk.screen_height()) @@ -95,65 +100,15 @@ class LaunchWindow(gtk.Window): def __size_changed_cb(self, screen): self._update_size() - -class LaunchBox(hippo.CanvasBox): - - def __init__(self, activity_id, icon_path, icon_color): - gobject.GObject.__init__(self, orientation=hippo.ORIENTATION_VERTICAL) - - self._activity_id = activity_id - self._activity_icon = CanvasPulsingIcon( - file_name=icon_path, - pulse_color=icon_color, - background_color=style.COLOR_WHITE.get_gdk_color()) - self.append(self._activity_icon, hippo.PACK_EXPAND) - - # FIXME support non-xo colors in CanvasPulsingIcon - self._activity_icon.props.base_color = \ - XoColor('%s,%s' % (style.COLOR_BUTTON_GREY.get_svg(), - style.COLOR_TRANSPARENT.get_svg())) - - self._animator = animator.Animator(1.0) - - self._home = shell.get_model() - self._home.connect('active-activity-changed', - self.__active_activity_changed_cb) - - self.connect('destroy', self.__destroy_cb) - - def __destroy_cb(self, box): - self._activity_icon.props.pulsing = False - self._home.disconnect_by_func(self.__active_activity_changed_cb) - - def zoom_in(self): - self._activity_icon.props.size = style.STANDARD_ICON_SIZE - - self._animator.remove_all() - self._animator.add(_Animation(self._activity_icon, - style.STANDARD_ICON_SIZE, - style.XLARGE_ICON_SIZE)) - self._animator.start() - self._activity_icon.props.pulsing = True - def __active_activity_changed_cb(self, model, activity): if activity.get_activity_id() == self._activity_id: self._activity_icon.props.paused = False else: self._activity_icon.props.paused = True - -class _Animation(animator.Animation): - - def __init__(self, icon, start_size, end_size): - animator.Animation.__init__(self, 0.0, 1.0) - - self._icon = icon - self.start_size = start_size - self.end_size = end_size - - def next_frame(self, current): - d = (self.end_size - self.start_size) * current - self._icon.props.size = int(self.start_size + d) + def __destroy_cb(self, box): + self._activity_icon.props.pulsing = False + self._home.disconnect_by_func(self.__active_activity_changed_cb) def setup(): diff --git a/src/jarabe/view/pulsingicon.py b/src/jarabe/view/pulsingicon.py index 392a404..2c106b0 100644 --- a/src/jarabe/view/pulsingicon.py +++ b/src/jarabe/view/pulsingicon.py @@ -16,73 +16,65 @@ import math -import gtk import gobject from sugar.graphics.icon import Icon, CanvasIcon - +from sugar.graphics import style _INTERVAL = 100 _STEP = math.pi / 10 # must be a fraction of pi, for clean caching +_MINIMAL_ALPHA_VALUE = 0.2 class Pulser(object): def __init__(self, icon): self._pulse_hid = None self._icon = icon - self._level = 0 self._phase = 0 + self._start_scale = 1.0 + self._end_scale = 1.0 + self._zoom_steps = 1 + self._current_zoom_step = 1 + self._current_scale_step = 1 + + def set_zooming(self, start_scale, end_scale, zoom_steps): + """ Set start and end scale and number of steps in zoom animation """ + self._start_scale = start_scale + self._end_scale = end_scale + self._zoom_steps = zoom_steps + self._current_scale_step = abs(self._start_scale - self._end_scale) / \ + self._zoom_steps + self._icon.scale = self._start_scale def start(self, restart=False): if restart: self._phase = 0 if self._pulse_hid is None: self._pulse_hid = gobject.timeout_add(_INTERVAL, self.__pulse_cb) + if self._start_scale != self._end_scale: + self._icon.scale = self._start_scale + \ + self._current_scale_step * self._current_zoom_step def stop(self): if self._pulse_hid is not None: gobject.source_remove(self._pulse_hid) self._pulse_hid = None self._icon.xo_color = self._icon.get_base_color() + self._icon.alpha = 1.0 def update(self): - if self._icon.get_pulsing(): - base_color = self._icon.get_base_color() - pulse_color = self._icon.get_pulse_color() - - base_stroke = self._get_as_rgba(base_color.get_stroke_color()) - pulse_stroke = self._get_as_rgba(pulse_color.get_stroke_color()) - base_fill = self._get_as_rgba(base_color.get_fill_color()) - pulse_fill = self._get_as_rgba(pulse_color.get_fill_color()) - - self._icon.set_stroke_color( - self._get_color_string(base_stroke, pulse_stroke)) - self._icon.set_fill_color( - self._get_color_string(base_fill, pulse_fill)) - else: - self._icon.xo_color = self._icon.base_color - - def _get_as_rgba(self, html_color): - if html_color == 'none': - return 1.0, 1.0, 1.0 - else: - color = gtk.gdk.color_parse(html_color) - return color.red / 65535.0, \ - color.green / 65535.0, \ - color.blue / 65535.0 - - def _get_color_string(self, orig_color, target_color): - r = orig_color[0] + self._level * (target_color[0] - orig_color[0]) - g = orig_color[1] + self._level * (target_color[1] - orig_color[1]) - b = orig_color[2] + self._level * (target_color[2] - orig_color[2]) - - return '#%02x%02x%02x' % (int(r * 255), int(g * 255), int(b * 255)) + self._icon.xo_color = self._icon.base_color + self._icon.alpha = _MINIMAL_ALPHA_VALUE + \ + (1 - _MINIMAL_ALPHA_VALUE) * (math.sin(self._phase) + 1) / 2 def __pulse_cb(self): self._phase += _STEP - self._level = (math.sin(self._phase) + 1) / 2 + if self._current_zoom_step <= self._zoom_steps and \ + self._start_scale != self._end_scale: + self._icon.scale = self._start_scale + \ + self._current_scale_step * self._current_zoom_step + self._current_zoom_step += 1 self.update() - return True @@ -118,6 +110,17 @@ class PulsingIcon(Icon): def get_base_color(self): return self._base_color + def set_zooming(self, start_size=style.SMALL_ICON_SIZE, + end_size=style.XLARGE_ICON_SIZE, + zoom_steps=10): + if start_size > end_size: + start_scale = 1.0 + end_scale = float(end_size) / start_size + else: + start_scale = float(start_size) / end_size + end_scale = 1.0 + self._pulser.set_zooming(start_scale, end_scale, zoom_steps) + base_color = gobject.property( type=object, getter=get_base_color, setter=set_base_color) -- 1.7.4
_______________________________________________ Sugar-devel mailing list [email protected] http://lists.sugarlabs.org/listinfo/sugar-devel
