• Status New
  • Percent Complete
  • Task Type Patches
  • Category Operating System/Drivers
  • 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 Maurus Cuelenaere - 2010-09-07

FS#11615 - Dynamic screen size

This patch converts LCD_{WIDTH,HEIGHT} into variables, allowing Rockbox to change screen size at runtime.

I’ve tried to make the changes as less intrusive as possible, but still, there will be some binsize increases.

Most of the #if’s were converted to C if()’s which should be functionally equivalent (and if the compiler is smart enough, no increase in binsize should occur).

In order to use this, just define DYNAMIC_LCD_SIZE in your target config and compile without plugins (if the preprocessor spits errors, try clearing ENABLEDPLUGINS in your Makefile).

What still needs to be done:
* figure a way out how to compile several rockbox logos into the binary and choose the correct one at runtime
* do something about the hacks in apps/bitmaps/SOURCES and apps/settings_list.c
* get the Android port working

Jonathan Gordon commented on 2010-11-04 10:32

I’m looking into getting this finished, a few comments:
1) why the changes to apps/recorder/bmp.c? Seems the end result is the same?
2) ditto firmware/bidi.c ?
3) scroll engine… we aren’t memory constrained on these targets so lets just set LCD_SCROLLABLE_LINES to something massive like 64 or so?
4) built in bitmaps: we could load them at runtime instead of compile time with a nice function to grab the right size (i.e <image name>.widthxheightxdepth.bmp )

I think I’ve freed up the skin engine which could have caused big problems so I’m going to apply this now and see how it goes (come on irc if you can :) )

Jonathan Gordon commented on 2010-11-04 12:02

Synced and updated version:
- scroll engine is totally stuffed
- loads backdrops and skins with the widthxheightxdepth extension so builds will need to have all the cabbie theme files for the dimensions we want to support.

I’ve only fixed the sdl app for now… “–display <width>x<height>” on the command line will set the size. sims still seem to work but I’m not sure if it is correct.

to test it with the themes “cp -R backdrops wps /usr/local/share/rockbox” from your rockbox tree

Jonathan Gordon commented on 2010-11-04 13:32

this is my crude attempt to fix android (apply to above patch)…. it segfaults after init and I don;t know where/why

Thomas Martitz commented on 2010-11-04 17:21

The last patch is more complicated than needed. You can get the display metrics in the Framebuffer class too, then you can query the dimensions after creating the object.

Maurus Cuelenaere commented on 2010-11-05 09:54

1) BM_MAX_WIDTH probably depends on LCD_{HEIGHT,WIDTH}, so ba.buf can’t be statically alloced
2) same reason (allocation is on-stack this time though)
3) why not just allocate this dynamically? That way, there are no limits to run into in the future and it shouldn’t result in a large binsize increase (talking about firmware/scroll_engine.c changes)
4) I was thinking of modifying bmp2rb to output an array of possible images, but dynamically loading them at runtime seems OK too

P.S.: your apps/gui/skin_engine/skin_parser.c change looks unrelated + you have some whitespace changes in apps/plugin.c

Jonathan Gordon commented on 2010-11-06 10:39

1,2) ah ok
3) do you mean LCD_SCROLLABLE_LINES should be dynamically alloced based on the screen size? or properly dynamically allocated as needed? the first seems a bit pointless because you still have a artificial limit (so using a hardcoded large value is just as good and simpler).
4) we don’t want to load too many images into the binary if we can avoid it, what could be fun is loading them all into the end of the *audiobuffer* region and then copying the one which is actually used to some buffer_alloced() block so the ram is reclaimed.

the skin_parser change is relevant, (It needs a comment). skin images are loaded from a directory with the same name as the skin file (i.e cabbiev2.wps → wps/cabbiev2/) now with the above changes we could be loading cabbiev2.60x480x16.wps but we want the images to still be in the wps/cabbiev2 folder.. so that change does this.

Thomas Martitz commented on 2010-11-06 11:48

Aren’t the images in the wps’ subfolder also screen size dependent? I.e. the 480×800 cabbie has larger icons (and a backdrop) in the wps/cabbiev2/ folder than the 320×480 one. So I’d think if we load cabbiev2.640x6480x16.wps (assuming there’s another .wps for a different screen size in the same folder) we also want the suitable images from wps/cabbiev2.640x480x16.wps.

I think we should package cabbiev2 separate, somehow. How about we put cabbiev2 into (either libcabbiev2 which has it for all dimensions, or one for each) and extract that separatenly, possibly to the sdcard directly? Or we make it so that the right cabbiev2 is downloaded on the first run?

Jonathan Gordon commented on 2010-11-06 12:00

well the skin images all have their screen size on the filenames anyway which is why it works. packaging up cabbie into a zip seems like more effort than needed, and will break if/when we do remote screen widgets which will need them also and wont be know which to use untill widget creation which could be after rockbox is up and running.
I thought we really didnt want to be downloading anything on first run? Althoughnot having to package the fonts into the zip would be very helpful

Maurus Cuelenaere commented on 2010-11-06 16:09

3) LCD_SCROLLABLES_LINES *is* dynamically based on the screen size. LCD_WIDTH & LCD_HEIGHT are dynamic now, remember? Basically, all changes in my original patch address static allocations based on LCD_{WIDTH,HEIGHT} (even if that isn’t very clear from the patch POV).
4) INIT_ATTR & friends could be used for that, but if loading the bitmaps at runtime is an easier way out I won’t disagree (there would be a need for default bitmaps compiled into the binary though, so Rockbox still is “usable” without any additional files).

I’m for the “download the correct default theme at first start”-idea, if we don’t want to waste binsize by putting all possible resolutions in the APK (not sure how big that would be?).

Jonathan Gordon commented on 2010-11-06 23:34

3) right, so there is still an artificial limit here, so we don’t actually gain anything by making it based on the detected screen size instead of just using a static size which is excessively large

with all the cabbie files (all screen sizes and depths so more than required) and all fonts, rockbox.apk ends up at about 9MB which is too big. however having to download 5MB of fonts/themes on first run sucks just as much.

very busy week at work so unlikely to be able to touch this till Wednesday night :(

Thomas Martitz commented on 2010-11-07 01:22

I think we should concentrate on the dynamic screen size first. removing artificial limits should be a separate patch.

Jonathan Gordon commented on 2010-11-07 12:13

I got android working :) still could be neater though.scrolling is still broken. is prebuilt with a manually hacked so it works. it contains most of the big fonts and all cabbie files. comes to about 7MB which is probbaly too big, we can work out how to fix that later.

Jonathan Gordon commented on 2010-11-07 12:41

and this fixes the scroll engine.

Jonathan Gordon commented on 2011-03-09 13:24

bump time. I’ve stripped out the android changes and only enabling this for the sdl app (change the screen size with –display WIDTHxHEIGHT).

scrolling is broken but otherwise looks good (scrolling is broken on sim also)

Jonathan Gordon commented on 2011-03-09 13:38

micor cleanup of LCD_HEIGHT/WIDTH, scrolling and plugins still problematic

Jonathan Gordon commented on 2011-03-13 10:03

fixed scrolling.

I’d really like to commit this in the next few days, so comments welcome. This does disable some plugins again but I really dont think it is a big deal (people that actually care about the missing plugins can disable the config option.) I’m going to look into fixing the plugins later.

Maurus Cuelenaere commented on 2011-03-13 12:00

Is the change in apps/gui/skin_engine/skin_parser.c related to this patch?
Otherwise this looks fine (also see my comments on the mailing list).

Jonathan Gordon commented on 2011-03-13 12:07

pretty sure that is an artifact from the sync.. need to double check that is still needed.

Jonathan Gordon commented on 2011-03-22 11:28

synced, android lcd is broken, no idea why though. Needs someone to take a look

Maurus Cuelenaere commented on 2011-03-24 00:42

I quickly hacked this up, seems to work on the emulator..

Jonathan Gordon commented on 2011-03-24 01:29

I cant actually look at your changes now but going by the diffstats I assume you made it work for android? there is opposition to enabling this for anything ubt sdl for now, so did you notice anything obvious why it wasnt working with HAVE_DYNAMIC_SCREEN_SIZE was not enabled?

Maurus Cuelenaere commented on 2011-03-24 09:25

I didn’t test that case, only made it work with HAVE_DYNAMIC_SCREEN_SIZE enabled. With it disabled, there shouldn’t be much additional changes though, just setting lcd_framebuffer to _lcd_framebuffer and doing some s/lcd_WIDTH/LCD_WIDTH/ etc.

Will look at it later.

Jonathan Gordon commented on 2011-03-27 11:19

my version causes wierdness in the android sim and outright crashes on target, your version seems to work so im inclined to look into making it compile for non-HAVE_DYNAMIC_SCREEN_SIZE and commit :)

Jonathan Gordon commented on 2011-03-30 12:14

fixed android and disabled it by default to not stire the hornets nest…

Maurus Cuelenaere commented on 2011-03-30 12:45

I’d move the

+#if !defined(HAVE_DYNAMIC_LCD_SIZE) && \
+ lcd_framebuffer = &_lcd_framebuffer[0][0];

hunk to firmware/target/hosted/android/lcd-android.c

You’ve also left some DEBUGF statements in lcd-bitmap-common and lcd-android.c

And the “lcd_framebuffer = malloc(sizeof(fb_data) * lcd_width * lcd_height);” line in lcd-android.c can be moved into the HAVE_DYNAMIC_LCD_SIZE #ifdef (you overwrite it later on anyways).

Jonathan Gordon commented on 2011-03-30 23:42

that code you pasted is for not android… maybe its unecessary, I’ll clean that up before commitign

Maurus Cuelenaere commented on 2011-03-31 10:57

Hmm right, I misread that line.

Anyway, if the alloc’ing of the framebuffer in lcd-android.c is properly excluded on #ifdef HAVE_DYNAMIC_LCD_SIZE, there should be no need to have that special case for Android.

Jonathan Gordon commented on 2011-03-31 11:04

its only needed because android mallocs the buffer regardless, but that shuold be changed to clean it up.
it looks like a bunch of other lcd drivers need fiddling because of the lcd_framebuffer arrray→pointer change which is mildly annoying… i’ll get it done over the weekend hopefully

Jonathan Gordon commented on 2011-04-20 13:57

sync and make ipd video compile, but it whitescreen and data aborts on boot

Björn Stenberg commented on 2011-10-11 22:10

Synced against r30742.
Changed to only enable dynamic code for targets with HAVE_DYNAMIC_LCD_SIZE. Other targets are (should be) unaffected.
sdlapp (with dynamic size) builds and runs. sansac200, ipodvideo and archosplayer (without dynamic) all build, haven’t run them yet.
A few malloc calls still remain.

Björn Stenberg commented on 2011-10-15 22:37

This patch (against r30759) is tested and runs fine on sdlapp, clip and c200.


Available keyboard shortcuts


Task Details

Task Editing