Merge lp:~codeforger/simplegc/ProgressBar into lp:simplegc

Proposed by Michael Rochester
Status: Superseded
Proposed branch: lp:~codeforger/simplegc/ProgressBar
Merge into: lp:simplegc
Diff against target: 206 lines (+180/-0)
3 files modified
docs/sgc.widgets.rst (+13/-0)
sgc/__init__.py (+1/-0)
sgc/widgets/progress.py (+166/-0)
To merge this branch: bzr merge lp:~codeforger/simplegc/ProgressBar
Reviewer Review Type Date Requested Status
Sam Bull Approve
Review via email: mp+178027@code.launchpad.net

This proposal supersedes a proposal from 2013-06-01.

This proposal has been superseded by a proposal from 2014-04-20.

Description of the change

Implimented a progress bar widget

To post a comment you must log in.
Revision history for this message
Sam Bull (dreamsorcerer) wrote : Posted in a previous version of this proposal

Images - 'overlay':
  This is not a valid image. It should simply draw the text onto the widget. Also, the text could be used as a string with an argument which you format with the current percentage, e.g. "Percent: %s%%".

Documentation is incomplete, there is no documentation for pulse() or percent. Try compiling the documentation and viewing it before finalising it.

can_focus defaults to False, so that line can simply be removed.

bar_percent and percent should be renamed to fraction, to better portray their 0-1 value.

throbber_speed:
  Should not move throbber once each time pulse is called. A smooth animation should be achieved if pulse() is called, say once every 250ms. You could instead use a throbber_step to decide how far it should move for each pulse, but the pulse animation should continue for 250ms after the last pulse() call. throbber_step should also be a fraction (0-1), not a pixel number.
  Note: GTK defaults to 0.1. Perhaps have the progress bar take 1 second to get from one side to the other, and use the step to define how far it will travel before it stops. This means if people have slow update loops, they can set the step to say, 0.5 to last half a second between updates.

Looking at GtkProgressBar, it makes more sense to switch between activity/percentage mode automatically when pulse() or fraction are used, thus losing the activity_mode config argument.

throbber_x can be a class attribute...

throbber_velocity seems to be the same variable as throbber_speed, there is no reason for this to exist.

Do the images need to be hidden everytime config() is called? Looks like something that should be in init.

Bar image is not drawn to full image:
  draw.rect(image, self._settings["col"], (2, 2, size[0] - 4, size[1] - 4))
Instead, you should set the size to be smaller, fill the whole image, and set it's position to the offset.
"bar": ((1, 0), (1, 0)) should be "bar": ((1, -4), (1, -4))

Same goes for throbber image.

update() is non-standard:
  This function doesn't get documented as it should be the same as all others, so you're warning will not be visible to users. I also don't understand why they would be clipped in 'pulse' (activity) mode, or why it would be unusual in 'default' (percentage) mode.

  Why is everything being blitted here? This should be done automatically by the widget, the only thing that should be happening here is moving the throbber's position when in activity mode.

self._throbber_velocity *= -1
Shouldn't be doing multiplication, if you want to invert it, just negate it:
self._throbber_velocity = -self._throbber_velocity

review: Needs Fixing
Revision history for this message
Sam Bull (dreamsorcerer) wrote :

Code's looking good, just sort out a few formatting problems:

46: *Displays
104: No reason for line wrapping.
117: Huh? What has that to do with rounding?
138: Pointless docstring, which is not displayed anyway.
162/197: Docstring should be lined up, not indented further.
191: Remove leading/trailing space.

Switching modes is a significant activity and should not be done in a property method. Leave the fraction property as read-only.

Does the text property make sense? Under what case would you need to retrieve the text? I can't think of a reason, and then setting the text makes more sense as a config argument.

review: Needs Fixing
Revision history for this message
Michael Rochester (codeforger) wrote :

On Wed, Aug 7, 2013 at 10:49 AM, Sam Bull <email address hidden> wrote:

> 46: *Displays
> 104: No reason for line wrapping.
> 117: Huh? What has that to do with rounding?
> 138: Pointless docstring, which is not displayed anyway.
> 162/197: Docstring should be lined up, not indented further.
> 191: Remove leading/trailing space.
>

These line numbers bear no relevance to the code, most of them point to
empty lines, and furthur more the entire file is only 165 lines long :/

lp:~codeforger/simplegc/ProgressBar updated
361. By Michael Rochester

made requested tweaks

362. By Michael Rochester

made requested tweaks

Revision history for this message
Sam Bull (dreamsorcerer) wrote :

Set the fraction of the bar, then run pulse(). The progress bar is still displayed in addition to the throbber.

review: Needs Fixing
lp:~codeforger/simplegc/ProgressBar updated
363. By Michael Rochester

fixed bug where text did not show.

364. By Michael Rochester

fixed issue where if bar was displayed in fraction mode and then switched to pulse mode, the bar remained visible.

Revision history for this message
Michael Rochester (codeforger) wrote :

> Set the fraction of the bar, then run pulse(). The progress bar is still
> displayed in addition to the throbber.

This is fixed in revision 364

Revision history for this message
Sam Bull (dreamsorcerer) wrote :

Merged to 0.2 branch.

review: Approve

Unmerged revisions

364. By Michael Rochester

fixed issue where if bar was displayed in fraction mode and then switched to pulse mode, the bar remained visible.

363. By Michael Rochester

fixed bug where text did not show.

362. By Michael Rochester

made requested tweaks

361. By Michael Rochester

made requested tweaks

360. By Michael Rochester

finalized tweaks that were requested

359. By Michael Rochester

made all requested changed

358. By Michael Rochester

Improved efficiency of pulse().

357. By Michael Rochester

Changed behaviour to be more like the GTK toolkit. Also some usability tweaks.

356. By Michael Rochester

implimented a progress bar widget

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'docs/sgc.widgets.rst'
2--- docs/sgc.widgets.rst 2013-03-22 17:46:43 +0000
3+++ docs/sgc.widgets.rst 2014-04-20 13:44:42 +0000
4@@ -116,6 +116,19 @@
5
6 .. automethod:: config
7
8+:mod:`progress` Module
9+--------------------------
10+
11+.. automodule:: sgc.widgets.progress
12+
13+ .. autoclass:: sgc.widgets.progress.ProgressBar
14+ :members:
15+ :undoc-members:
16+ :show-inheritance:
17+ :exclude-members: update, groups, selected
18+
19+ .. automethod:: config
20+
21 :mod:`radio_button` Module
22 --------------------------
23
24
25=== modified file 'sgc/__init__.py'
26--- sgc/__init__.py 2013-03-23 16:16:35 +0000
27+++ sgc/__init__.py 2014-04-20 13:44:42 +0000
28@@ -31,6 +31,7 @@
29 from widgets.fps_counter import FPSCounter
30 from widgets.input_box import InputBox
31 from widgets.label import Label
32+from widgets.progress import ProgressBar
33 from widgets.radio_button import Radio
34 from widgets.scroll_box import ScrollBox
35 from widgets.settings import Keys
36
37=== added file 'sgc/widgets/progress.py'
38--- sgc/widgets/progress.py 1970-01-01 00:00:00 +0000
39+++ sgc/widgets/progress.py 2014-04-20 13:44:42 +0000
40@@ -0,0 +1,166 @@
41+# Copyright 2013 the SGC project developers.
42+# See the LICENSE file at the top-level directory of this distribution
43+# and at http://program.sambull.org/sgc/license.html.
44+
45+"""
46+Progress Bar widget. Displays a bar to the user with either a percent filled
47+or activity throbber.
48+
49+"""
50+
51+import pygame
52+from pygame.locals import *
53+from pygame import draw
54+
55+from _locals import *
56+from base_widget import Simple
57+
58+
59+class ProgressBar(Simple):
60+ """
61+ A Progress Bar.
62+
63+ Images:
64+ 'image': The background of the bar.
65+ 'bar': The image to be used for the percent bar
66+ (bar will be cut to size, not stretched)
67+ 'throbber': The image used for the 'activity' throbber.
68+ 'overlay': This image will always be displayed on top of the widget
69+ regardless of mode, it is by default blank, however passing the
70+ keyword 'text' to the config function will draw this text onto this
71+ surface.
72+ """
73+
74+ _default_size = (100, 20)
75+ _extra_images = {"bar": ((1, -4), (1, -4)),
76+ "throbber": ((.2, -6), (1, -6)),
77+ "overlay": ((1, 0), (1, 0))}
78+ _settings_default = {"col": (118, 45, 215), "font": Font["widget"],
79+ "text": "", "text_col": (10, 10, 10), "total_time": 1,
80+ "pulse_duration": .1}
81+ _surf_flags = SRCALPHA
82+
83+ _bar_fraction = 1.
84+ _throbber_move_timer = 0
85+ _activity_mode = False
86+
87+ def _config(self, **kwargs):
88+ """
89+ col: ``tuple`` (r,g,b) The color to be used for the bar, to avoid
90+ saturation of the throbber use values below 200
91+ pulse_duration: ``float`` time in seconds each pulse should last,
92+ useful if your update loop is slower than the default of .25
93+ total_time: ``float`` time in seconds the throbber should take to
94+ travel the full distance of the bar.
95+ font: font to be used for the text
96+ text: ``str`` the text to be shown in the center of the widget
97+ text_col: the color of the text
98+ """
99+ for key in ("col", "font", "total_time",
100+ "pulse_duration", "text_col"):
101+ if key in kwargs:
102+ self._settings[key] = kwargs[key]
103+
104+ if "text" in kwargs:
105+ self._settings["text"] = kwargs["text"]
106+ overlay = self._images["overlay"]
107+ overlay.image.fill((0, 0, 0, 0))
108+ label = self._settings["font"].render(self._settings["text"], True,
109+ self._settings["text_col"])
110+ overlay.image.blit(label, (overlay.rect.w / 2 -
111+ label.get_rect().w / 2,
112+ overlay.rect.h / 2 -
113+ label.get_rect().h / 2))
114+
115+ if "total_time" in kwargs or "pulse_duration" in kwargs:
116+ total_time_ms = self._settings["total_time"] * 1000.
117+ self._throbber_velocity = self.rect.w / total_time_ms
118+
119+ if "init" in kwargs:
120+ self._images["bar"]._show = False
121+ self._images["throbber"]._show = False
122+ self._images["throbber"].pos = [0, 3]
123+
124+ total_time_ms = self._settings["total_time"] * 1000.
125+ self._throbber_velocity = self.rect.w / total_time_ms
126+
127+ self._throbber_x = 0
128+
129+ def _draw_base(self):
130+ draw.rect(self._images["image"], (100, 100, 100),
131+ (0, 0, self.rect.w, self.rect.h))
132+ draw.rect(self._images["image"], (240, 240, 240),
133+ (1, 1, self.rect.w - 2, self.rect.h - 2))
134+
135+ def _draw_bar(self, image, size):
136+ draw.rect(image, self._settings["col"],
137+ (0, 0, size[0], size[1]))
138+
139+ def _draw_throbber(self, image, size):
140+ brightened_color = [x + 20 for x in self._settings["col"]]
141+ draw.rect(image, brightened_color, (0, 0, size[0], size[1]))
142+
143+ def _draw_overlay(self, image, size):
144+ pass
145+
146+ def update(self, time):
147+ if self._activity_mode:
148+ self._images["bar"]._show = False
149+ if self._throbber_move_timer > 0:
150+ self._throbber_move_timer -= time
151+ # Move throbber.
152+ self._throbber_x += self._throbber_velocity * time
153+ self._images["throbber"].rect.x = self._throbber_x
154+ # If outside box, change direction and move back in.
155+ left_edge = self._images["throbber"].rect.x - 3
156+ right_edge = left_edge + self._images["throbber"].rect.w + 6
157+ if left_edge < 0 or right_edge > self.rect.w:
158+ self._throbber_velocity = - self._throbber_velocity
159+ self._throbber_x += self._throbber_velocity * time
160+ self._images["throbber"].rect.x = self._throbber_x
161+ else:
162+ bar_rect = (0, 0, self.rect.w * self._bar_fraction, self.rect.h)
163+ draw.rect(self.image, (100, 100, 100),
164+ (0, 0, self.rect.w, self.rect.h))
165+ draw.rect(self.image, (240, 240, 240),
166+ (1, 1, self.rect.w - 2, self.rect.h - 2))
167+ self.image.set_clip(bar_rect)
168+ self.image.blit(self._images["bar"].image, (2, 2))
169+ self.image.set_clip(None)
170+
171+ def pulse(self):
172+ """
173+ Moves the activity mode throbber
174+
175+ The throbber will move for the number of seconds described by the
176+ pulse_duration config argument.
177+
178+ Note: This will cause the Widget to change to activity mode if it
179+ was in percent mode before.
180+ """
181+ if not self._activity_mode:
182+ self._activity_mode = True
183+ bar_rect = (0, 0, self.rect.w * self._bar_fraction, self.rect.h)
184+ draw.rect(self.image, (100, 100, 100),
185+ (0, 0, self.rect.w, self.rect.h))
186+ draw.rect(self.image, (240, 240, 240),
187+ (1, 1, self.rect.w - 2, self.rect.h - 2))
188+ self._images["throbber"]._show = True
189+ self._throbber_move_timer = self._settings["pulse_duration"] * 1000
190+
191+ @property
192+ def fraction(self):
193+ """Returns the current fraction the bar is at as a float."""
194+ return self._bar_fraction
195+
196+ def set_fraction(self, fraction):
197+ """
198+ Sets the fraction the bar should be as a float.
199+
200+ Note: This will cause the Widget to change to percent mode if it
201+ was in activity mode before.
202+ """
203+ if self._activity_mode:
204+ self._activity_mode = False
205+ self._images["throbber"]._show = False
206+ self._bar_fraction = min(max(fraction, 0.), 1.)

Subscribers

People subscribed via source and target branches