Rockbox

This is the bug/patch tracker for Rockbox. Click here for more information.

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#4817 - Remove duplicated scrolling code in the LCD drivers.

Attached to Project: Rockbox
Opened by Fredrik Öhrn (ohrn) - Saturday, 11 March 2006, 18:03 GMT+2
Last edited by Andrew Mahone (Unhelpful) - Thursday, 13 August 2009, 10:05 GMT+2
Task Type Patches
Category Drivers
Status Closed
Assigned To No-one
Player Type All players
Severity Low
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

This patch merges the scrolling code from all the bitmap LCD drivers (except the H100 remote) into a single file.
The copyright notice needs to be fixed, I was unable to figure out from whom the scrolling code actually originated.
(application/octet-stream)    remove_duplicate_scrolling.diff (32.1 KiB) 
 firmware/SOURCES                 |    1 
 firmware/drivers/lcd-16bit.c     |  209 +-------------------------------------
 firmware/drivers/lcd-2bit-horz.c |  214 +--------------------------------------
 firmware/drivers/lcd-h100.c      |  206 +------------------------------------
 firmware/drivers/lcd-recorder.c  |  213 +-------------------------------------
 firmware/export/lcd.h            |    4 
 6 files changed, 43 insertions(+), 804 deletions(-)

Closed by  Andrew Mahone (Unhelpful)
Thursday, 13 August 2009, 10:05 GMT+2
Reason for closing:  Accepted
Additional comments about closing:  Committed as r22289.
Comment by pillo (pillo79) - Saturday, 11 March 2006, 19:08 GMT+2
It seems you forgot to include in the patch the scrolling.c file you just created. This is probably because you used "cvs diff" on a tree with non-yet-added cvs files (see first line of your .diff ;-)
Comment by Fredrik Öhrn (ohrn) - Saturday, 11 March 2006, 19:23 GMT+2
Doh! I spent ages fighting with cvs over that misfeature and then I manage to upload the wrong file.
Here's the correct diff with scrolling.c included.
(application/octet-stream)    correct_patch.diff (39.8 KiB) 
 firmware/SOURCES                 |    1 
 firmware/drivers/lcd-16bit.c     |  209 +-------------------------------
 firmware/drivers/lcd-2bit-horz.c |  214 +--------------------------------
 firmware/drivers/lcd-h100.c      |  206 +-------------------------------
 firmware/drivers/lcd-recorder.c  |  213 +--------------------------------
 firmware/drivers/scrolling.c     |  250 +++++++++++++++++++++++++++++++++++++++
 firmware/export/lcd.h            |    4 
 7 files changed, 293 insertions(+), 804 deletions(-)

Comment by Fredrik Öhrn (ohrn) - Monday, 13 March 2006, 10:45 GMT+2
OK, I managed to leave some junk in a function declaration. Here's a new patch.
(application/octet-stream)    correct_patch2.diff (39.8 KiB) 
 firmware/SOURCES                 |    1 
 firmware/drivers/lcd-16bit.c     |  209 +-------------------------------
 firmware/drivers/lcd-2bit-horz.c |  214 +--------------------------------
 firmware/drivers/lcd-h100.c      |  206 +-------------------------------
 firmware/drivers/lcd-recorder.c  |  213 +--------------------------------
 firmware/drivers/scrolling.c     |  250 +++++++++++++++++++++++++++++++++++++++
 firmware/export/lcd.h            |    4 
 7 files changed, 293 insertions(+), 804 deletions(-)

Comment by Jonathan Gordon (jdgordon) - Tuesday, 31 July 2007, 15:26 GMT+2
quick attempt to merge the bits of the lcd drivers which wernt done last week.
untested because stupid gcc complains about no sysfont.h with this, hopefully someone else has more luck
   lcd_update.diff (23.1 KiB) 
 firmware/SOURCES                      |    1 
 firmware/drivers/lcd-1bit-vert.c      |   80 ------------
 firmware/drivers/lcd-2bit-vert.c      |   83 -------------
 firmware/drivers/lcd-2bit-horz.c      |   82 -------------
 firmware/drivers/lcd-remote-2bit-vi.c |   83 -------------
 firmware/drivers/lcd-generic.c        |  210 ++++++++++++++++++++++++++++++++++
 firmware/drivers/lcd-16bit.c          |   90 --------------
 firmware/drivers/lcd-remote-1bit-v.c  |   83 -------------
 8 files changed, 211 insertions(+), 501 deletions(-)

Comment by Peter D'Hoye (petur) - Thursday, 02 August 2007, 23:14 GMT+2
Jonathan?
Comment by Jonathan Gordon (jdgordon) - Saturday, 11 August 2007, 16:56 GMT+2
forgot about this one...

something causes sysfont.h to not be created...
Comment by Barry Wardell (barrywardell) - Saturday, 11 August 2007, 17:29 GMT+2
The problem isn't really with sysfont.h. The problem is your change in firmware/SOURCES. The added line should be 'drivers/lcd-generic.c'. For some reason, whenever a file from SOURCES doesn't exist, we get an error about sysont.h not being created instead of an error about the missing file.

I made the change to firmware/SOURCES, but there are a load of errors in compiling lcd-generic.c so I didn't go any further with testing it.

Edit: I've opened a task for the sysfont.h problem and added it as a related task here.
Comment by Jonathan Gordon (jdgordon) - Sunday, 12 August 2007, 02:38 GMT+2
doh, thanks Barry.

here is a partial sync.. it now compiles fine on 16 bit non-lcd-remote targets.. if this is wanted the rest of the lcd targets can be fixed up also
   lcd_update.diff (25.7 KiB) 
 firmware/export/lcd.h                 |   10 +
 firmware/SOURCES                      |    1 
 firmware/drivers/lcd-1bit-vert.c      |   80 ------------
 firmware/drivers/lcd-2bit-vert.c      |   83 ------------
 firmware/drivers/lcd-2bit-horz.c      |   82 ------------
 firmware/drivers/lcd-remote-2bit-vi.c |   83 ------------
 firmware/drivers/lcd-generic.c        |  225 ++++++++++++++++++++++++++++++++++
 firmware/drivers/lcd-16bit.c          |  102 ---------------
 firmware/drivers/lcd-remote-1bit-v.c  |   83 ------------
 9 files changed, 242 insertions(+), 507 deletions(-)

Comment by Andrew Mahone (Unhelpful) - Saturday, 18 July 2009, 12:23 GMT+2
This is more of a rewrite than a resync, moving pretty much all of the text drawing code into a new lcd-bitmap-common.c, merging the styled text draw code used by puts_style_offset and puts_scroll_style_offset, and merging lcd_gradient_rect and lcd_gradient_rect_scroll. There are binsize savings on the order of 500-600B on color targets, with smaller or no improvement on greyscale targets. I also have a version in a branch which avoids including lcd-bitmap-common.c, but this is more invasive (because the viewports must be made non-static, and therefore must have unique names between main and remote) and causes binsize increase on some targets compared to this version (probably because of long calls).
EDIT: new diff, previous one missed the new file
   lcd-bitmap-common-20090718072655.diff (51.2 KiB) 
 b/firmware/drivers/lcd-16bit.c         |  347 ---------------------------------
 b/firmware/drivers/lcd-1bit-vert.c     |  224 ---------------------
 b/firmware/drivers/lcd-2bit-horz.c     |  221 ---------------------
 b/firmware/drivers/lcd-2bit-vert.c     |  226 ---------------------
 b/firmware/drivers/lcd-2bit-vi.c       |  225 ---------------------
 b/firmware/drivers/lcd-bitmap-common.c |  314 +++++++++++++++++++++++++++++
 6 files changed, 321 insertions(+), 1236 deletions(-)

Comment by Andrew Mahone (Unhelpful) - Friday, 24 July 2009, 06:49 GMT+2
Updated, mostly just line-length fixes
   lcd-bitmap-common-20090724004529.diff (51.5 KiB) 
 b/firmware/drivers/lcd-16bit.c         |  347 ---------------------------------
 b/firmware/drivers/lcd-1bit-vert.c     |  224 ---------------------
 b/firmware/drivers/lcd-2bit-horz.c     |  221 ---------------------
 b/firmware/drivers/lcd-2bit-vert.c     |  226 ---------------------
 b/firmware/drivers/lcd-2bit-vi.c       |  225 ---------------------
 b/firmware/drivers/lcd-bitmap-common.c |  322 ++++++++++++++++++++++++++++++
 6 files changed, 329 insertions(+), 1236 deletions(-)

Loading...