Rockbox

Slightly different, basically your patch but with other parameters for draw_progressbar and no +1 in line_num (what was that for?).

   pb_fix.diff (4.2 KiB)

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

“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).”

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.