- Status Closed
- Percent Complete
- Task Type Patches
- Category User Interface → Themes
- Assigned To No-one
- Operating System All players
- Severity Low
- Priority Very Low
- Reported Version Version 3.1
- Due in Version Undecided
-
Due Date
Undecided
- Votes
- Private
FS#9904 - Fix for FS#9894 - Position of the progress bar is not updated after the font is changed
This is a fix for the bug reported in FS#9894
Closed by kugel.
2009-02-16 08:37
Reason for closing: Accepted
Additional comments about closing: Warning: Undefined array key "typography" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 371 Warning: Undefined array key "camelcase" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 407
2009-02-16 08:37
Reason for closing: Accepted
Additional comments about closing: Warning: Undefined array key "typography" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 371 Warning: Undefined array key "camelcase" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 407
Committed slightly modified in r20015,
thanks you!
Loading...
Available keyboard shortcuts
- Alt + ⇧ Shift + l Login Dialog / Logout
- Alt + ⇧ Shift + a Add new task
- Alt + ⇧ Shift + m My searches
- Alt + ⇧ Shift + t focus taskid search
Tasklist
- o open selected task
- j move cursor down
- k move cursor up
Task Details
- n Next task
- p Previous task
- Alt + ⇧ Shift + e ↵ Enter Edit this task
- Alt + ⇧ Shift + w watch task
- Alt + ⇧ Shift + y Close Task
Task Editing
- Alt + ⇧ Shift + s save task
Slightly different, basically your patch but with other parameters for draw_progressbar and no +1 in line_num (what was that for?).
The negated line number must be 1-based (not 0-based) to clearly distinguish the case when it was not set. The comment about it was relevant IMHO. Hence, when computing the Y-pos of the progress bar we have to subtract 1. You patch does not add the necessary delta for the Y. The progress bar should be drawn in the middle of the line, not at the top or bottom.
But if the information about the font is present in the wps viewport it can be used as the parameter as well, it would be even better.
The local variable I used for vieports[v] should make binsize smaller. But I agree that it could/should be a separate patch.
Why does it have to be 1-based? if it’s 0, it doesn’t matter if y was set or not.
Right, I forget about the draw-in-the-middle-of-the-line thing.
I’d still prefer to preserve the information about whether it was explicitly set or not as long as possible. I even wanted to introduce a special flag for that but then went the “dirty” way by using a special value range. Effectively 0 would give the same result, yes. But to have this info is cleaner IMO (for possible changes in the future).
Regarding the parameters of draw_progressbar: if we pass the wps_viewport, we don’t need to pass the progress bar explicitly since it’s contained in the viewport struct. Thus the function gets the meaning “draw the viewport’s progressbar given the specified gwps state”.
Yes, and I removed the progressbar from the parameter list (if you didn’t notice).
a) We don’t code for the future. Nobody can tell what will happen in the future, so we do not prepare the code for it. Especially not, if there’s not even the slightest idea of “what might change”.
b) I don’t consider more complexity as cleaner. It does what we need, not more, with minimal complexity. That is what I consider as clean.
Ok, I thought about it again, and it makes a difference if it’s 0 due to specific setting y or due to line number, since in the latter case we want to center the progress bar in the line.
I’m going to commit your version, but with my parameter change.