Rockbox

  • Status Unconfirmed
  • Percent Complete
    0%
  • Task Type Patches
  • Category Plugins
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version Daily build (which?)
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by csavery - 2010-05-24

FS#11310 - PictureFlow - mono tracklist for mono displays

This very small mod changes the tracklist so it doesn't fade lines when on a mono display (bit depth 1). Tested against r26241 in Sim only (because I don't have a mono display device).
Please report back if problems.

This does noticeably improve the display, but the text of the unselected track titles continues to flicker faintly even so; it appears that all text is flickering at the same rate rather than in accordance with its distance from the selected track, but certainly at a much reduced rate and readability has certainly been improved regardless. Also worth noting is that the highlight bar also flickers as it is not a solid colour; perhaps an inverted colour system (bar coloured, text black, as Rockbox does in its own menus) would improve that.

It's difficult to demonstrate either issue given that the simulator doesn't replicate mono screens very accurately and you can't show flickering in a photo, but attached is a picture demonstrating the potential readabiity issue caused by the selection bar; it's more legible when not being captured by a low-quality webcam, but could be a problem, particularly for users with poor eyesight.

I'd suggest you make a manual change to the patch and let me know if it looks better.
Go to the few lines changed and you'll see the value 250 is used for text colour except for the current line which is 255.
Try making them all 255 and see how it looks. It may be that anything but 0 and 255 get represented with dithering and cause flicker.
I kept the 250 value as it offers a slight brightness change to indicate highlight but if it cannot show anything but on/off then 0 and 255 may be better.
Let me know if that looks better and I'll make a new patch. Also the highlight may look better as a inverse (black lettering on white bar).
If you're not sure what I mean or what to change then just let me know and I'll upload one for you to test out.

Indeed, setting the brightness to 255 has it working perfectly with no flicker; I did try to do an inverse bar but only got as far as making the text black before losing my way. Sorry this isn't a patch, but my pictureflow.c already has several other patches applied and also I have no idea how to make a patch; this is as far as I got:

      if ( track_i == selected_track ) {

#if LCD_DEPTH > 1

          draw_gradient(titletxt_y, titletxt_h);
          MYLCD(set_foreground)(G_BRIGHT(255));

#else

          MYLCD(set_foreground)(G_BRIGHT(0));

#endif

It can get a bit confusing with several patches and I've made mistakes editing out bits sometimes.
Here is the above patch updated with the color 255 change for others who may want it.
It's against r26251 now.

And here is the patch updated with trial inverted menu bar code.
Check if it looks better and post here so others know which one is better to use.
Also against r26251 and tested here only in Sim.

Compiled from a clean, up-to-date pictureflow.c with that latest patch applied; the highlight bar is still not a solid colour and so has the flickering/dithering effect. Since the bar is ostensibly "grey" it's about as legible now as it was with "white" text, so I really wouldn't want to say whether one or the other was superior, it'd really come down to a matter of preference.

Looking at the patch, I think the problem might be that draw_gradient is being called on all devices, when it really should only be called on devices with bit depth > 1. Unfortunately, the fix is not as simple as putting the draw_gradient behind the #if LCD_DEPTH > 1 line; this simply results in no bar appearing at all meaning that the currently selected track is black-on-black and entirely invisible.

Ah yes, that would be likely. It's too bad I don't have a mono device to see this type of thing before uploading a patch.
Here is a pair of updates with the draw_gradient modified to degrade to solid color when bit depth is 1.
Hopefully this will hot the spot.

Getting closer;

pictureflow-mono-list.patch gives a solid bar which I suspect is overlaid over the text; if the gradient isn't a background but an overlay, setting a solid colour overlay just results in obscuring the text–that's my best guess as to what's happening here. Incidentally, this patch also causes an error while compiling:

CC apps/plugins/pictureflow/pictureflow.c
/home/vague/rockbox/apps/plugins/pictureflow/pictureflow.c: In function ‘draw_gradient’:
/home/vague/rockbox/apps/plugins/pictureflow/pictureflow.c:2251: warning: unused variable ‘c2’
LD pictureflow.rock

pictureflow-mono-list-inv.patch looks the same as the previous version; black text on flickerbar.

Ya, something like that is probably happening.
I knew about the compiler warning but didn't want to add a few more defines to remove.
Just an unused variable hanging around from color version code - causes no problem.
I'll get back to have another look a little later.

Here's an updated version that shows the menu bar as black text on white bar.
Confirmed to work ok on Clip+ and in USim - against r26251.

I think this should be only for the clip, since the greylib works properly for other mono displays.

When using greylib does LCD_DEPTH have value 1 or some higher value dependent on greyscale bit depth?

Hmmm. Just had a look a various config headers. It seems these devices have LCD_DEPTH == 1 (other than Clip variants):
archos*, iriverifp7xx, logikdax, mrobe100, sansam200.
If we're sure that these players display fine with current support then I need some other value to key off.
I don't see a "Clip family" define.
Suggested solution? Add #define LCD_STRICT_MONO for Clip family devices?

Updates to this patch are combined in with reflection in FS#11300.
See that task for new patch just posted.

#if CONFIG_LCD == LCD_CLIP should work

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing