This is the bug/patch tracker for Rockbox. Click here for more information.
Quick links: Bugs · Patches · Rockbox frontpage
FS#8385 - Viewports
Attached to Project:
Rockbox
Opened by Dave Chapman (linuxstb) - Saturday, 29 December 2007, 21:14 GMT+1
Last edited by Dave Chapman (linuxstb) - Friday, 21 March 2008, 20:39 GMT+1
Opened by Dave Chapman (linuxstb) - Saturday, 29 December 2007, 21:14 GMT+1
Last edited by Dave Chapman (linuxstb) - Friday, 21 March 2008, 20:39 GMT+1
|
DetailsAttached is my work-in-progress implementation of viewports.
Currently, they are only implemented in the lcd-16bit.c LCD driver - so this patch only compiles on targets with a colour screen and no LCD remote. This patch adds a test_viewports.c plugin, plus a new %V WPS tag to allow use of viewports in the WPS. The format of the %V tag is: %V|x|y|width|height|fgcolor|bgcolor| e.g. %V|100|50|125|125|FF0000|00FF00| All lines in the WPS up to the first %V tag are displayed in the default (full-screen) viewport. Lines following a %V are drawn in the new viewport - giving pixel-accurate positioning, left/right scrollmargins and custom colours. Note that if the WPS specifies a backdrop, the bgcolor is ignored - backdrops are still global and the viewport will be transparent. As an example of how viewports can be used, I'm attaching a modified version of jClix_Night adapted to use viewports - note that this is 220x176, so will currently only work on the iPod Color (the H300 has a remote). Work left to do: Finalise API - this implementation is still a work-in-progress Adapt other LCD drivers Modify the core Rockbox UI code to use viewports |
This task depends upon
Closed by Dave Chapman (linuxstb)
Friday, 21 March 2008, 20:39 GMT+1
Reason for closing: Accepted
Additional comments about closing: Patch v4.1 committed to SVN - r16733
Friday, 21 March 2008, 20:39 GMT+1
Reason for closing: Accepted
Additional comments about closing: Patch v4.1 committed to SVN - r16733
It also includes various bugfixes and API enhancements/changes - this is likely to be the final API, so work can now start on implementing viewport changes to the rest of the apps/ code.
My current to-do list (apart from implementing the remaining 5 LCD drivers):
* Validate the Viewport position/size when reading a WPS (it is the apps/ code's responsibility to ensure that viewports are wholly contained on the screen - the LCD drivers don't check).
* WPS images are still being drawn in the full-screen viewport, regardless of where in the .wps file they appear. Should this be changed?
* There are now three different functions to stop scrolling lines (lcd_stop_scroll, lcd_scroll_stop(vp), lcd_scroll_stop_line(vp,line)) - can these be consolidated/named better?).
* Currently the fg/bg colours in the %V tag are always mandatory, even for mono WPSs - this needs fixing.
My intention is to commit the completed version of this patch (LCD driver, scrolling and WPS changes) as soon as possible (hopefully within the next couple of days), without changing any more of the apps/ code. Work can then start on adapting the rest of the apps code to exploit viewports.
So all comments are welcome, preferably sooner than later... But if anyone wants to seriously review this patch before I commit it, but doesn't have time immediately, please let me know.
Also very useful would be testing on various targets to ensure that no current functionality is broken - everything (including all WPSs) should work the same as without this patch.
I'll try to keep them up to date, but watch the "Last updated" date to make sure you get a version with the latest patch applied.
What exactly happens when you try it? Which sim did you use?
Nice work on the patch so far!
Both in the sim and on the player - whenever the volume is changed, a transparent bar appears across the AlbumArt image. Nothing else seems to be affected.
as for the playlist view in the wps, if what I want to do happens then it will be possible, but untill apps/ starts getting converted, we dont know what will and wont happen.
I think your problem is the same as I found when porting jClix - if you have a line of conditional images, then whenever that line is updated, the text-line the conditional appears on is cleared, and the images redisplayed. I'm assuming WPSes have always behaved this way.
The solution is either to do what I did in jClix (which I don't like...) and create a 1x1 pixel viewport and use that for all your bitmaps (bitmaps are drawn based on full-screen co-ordinates, not inside the current viewport), or put your conditional image lines at the end of the WPS, so that they are drawn off the bottom of the screen.
Does anyone who knows the WPS code better than me have any other ideas/solutions?
This also includes various little cleanups everywhere, plus a fix for GodEater's fg/bg colour bug (untested, but I'm confident...)
Please test and provide feedback...
Using arm-elf-gcc 4.0.3 (400)
Using arm-elf-ld 2.16.1
CC test_viewports.c
test_viewports.c:252: error: redefinition of ‘__header’
test_viewports.c:22: error: previous definition of ‘__header’ was here
test_viewports.c:262:7: warning: extra tokens at end of #else directive
test_viewports.c:268: error: redefinition of ‘vp0’
test_viewports.c:26: error: previous definition of ‘vp0’ was here
test_viewports.c:289: error: redefinition of ‘vp1’
test_viewports.c:47: error: previous definition of ‘vp1’ was here
test_viewports.c:310: error: redefinition of ‘vp2’
test_viewports.c:68: error: previous definition of ‘vp2’ was here
test_viewports.c:332: error: redefinition of ‘vp3’
test_viewports.c:90: error: previous definition of ‘vp3’ was here
test_viewports.c:391: error: redefinition of ‘plugin_start’
test_viewports.c:149: error: previous definition of ‘plugin_start’ was here
test_viewports.c:561: error: redefinition of ‘__header’
test_viewports.c:22: error: previous definition of ‘__header’ was here
test_viewports.c:571:7: warning: extra tokens at end of #else directive
test_viewports.c:577: error: redefinition of ‘vp0’
test_viewports.c:268: error: previous definition of ‘vp0’ was here
test_viewports.c:598: error: redefinition of ‘vp1’
test_viewports.c:289: error: previous definition of ‘vp1’ was here
test_viewports.c:619: error: redefinition of ‘vp2’
test_viewports.c:310: error: previous definition of ‘vp2’ was here
test_viewports.c:641: error: redefinition of ‘vp3’
test_viewports.c:332: error: previous definition of ‘vp3’ was here
test_viewports.c:700: error: redefinition of ‘plugin_start’
test_viewports.c:149: error: previous definition of ‘plugin_start’ was here
test_viewports.c:870: error: redefinition of ‘__header’
test_viewports.c:22: error: previous definition of ‘__header’ was here
test_viewports.c:880:7: warning: extra tokens at end of #else directive
test_viewports.c:886: error: redefinition of ‘vp0’
test_viewports.c:577: error: previous definition of ‘vp0’ was here
test_viewports.c:907: error: redefinition of ‘vp1’
test_viewports.c:598: error: previous definition of ‘vp1’ was here
test_viewports.c:928: error: redefinition of ‘vp2’
test_viewports.c:619: error: previous definition of ‘vp2’ was here
test_viewports.c:950: error: redefinition of ‘vp3’
test_viewports.c:641: error: previous definition of ‘vp3’ was here
test_viewports.c:1009: error: redefinition of ‘plugin_start’
test_viewports.c:149: error: previous definition of ‘plugin_start’ was here
test_viewports.c:1179: error: redefinition of ‘__header’
test_viewports.c:22: error: previous definition of ‘__header’ was here
test_viewports.c:1189:7: warning: extra tokens at end of #else directive
test_viewports.c:1195: error: redefinition of ‘vp0’
test_viewports.c:886: error: previous definition of ‘vp0’ was here
test_viewports.c:1216: error: redefinition of ‘vp1’
test_viewports.c:907: error: previous definition of ‘vp1’ was here
test_viewports.c:1237: error: redefinition of ‘vp2’
test_viewports.c:928: error: previous definition of ‘vp2’ was here
test_viewports.c:1259: error: redefinition of ‘vp3’
test_viewports.c:950: error: previous definition of ‘vp3’ was here
test_viewports.c:1318: error: redefinition of ‘plugin_start’
test_viewports.c:149: error: previous definition of ‘plugin_start’ was here
make[2]: *** [/home/sander/rockbox-svn/build/apps/plugins/test_viewports.o] Error 1
make[1]: *** [rocks] Error 2
make: *** [build] Error 2
The syntax is now as follows:
%V|x|y|width|height|font|fgcolor|bgcolor|
Specifying "0" for font means that that viewport draws text using the built-in (6x8) system font (FONT_SYSFIXED). Note that this font only includes ASCII characters, so isn't suitable for displaying information from tags.
Specifying "1" for font will use the currently selected user font (FONT_UI).
Other values for font are not rejected, but are replaced with FONT_UI. The intention is that the multifont patch can be updated to work with viewports, and (until that patch is suitable for and committed to SVN), themes using multifonts can be designed to degrade gracefully in the official builds.
I'm also attaching an updated version of jClix_Night_VP, which also removes the "dummy viewport hack" and instead puts all the bitmap conditionals on a single line with some dynamic content, as suggested by Nico_P. Screenshot also attached.
The batt[5cp].bmp in jClix should be renamed to Batt* for the sim to find them. Also stop.bmp seems to be missing completely?
Thanks - I'm using a Mac, so didn't notice the filenames were wrong. I've now renamed everything to all lower-case and changed the .wps appropriately.
You're right that stop.bmp is missing (it was missing in the original jClix), but the WPS is never displayed when music is stopped...
I would be happy to help debug any problems you have with porting the multifonts patch. But I don't think you'll have the same problem, as long as you stick to the intended design of one font per viewport.
Regarding customlists - see the comment I've just posted on that task. This patch is just the start...
my current thinking is along the lines of moving the current screens struct (screen_access.[ch]) into firmware and calling it lcd_funcs (or something) than in apps/ get a new sturct going which would have a lcd_funcs pointer and a viewports struct, and then some extra stuff, although the only thing I can think of for the extras is the line count in the viewport...
although, typing this is turning me off that idea a bit also :(
I was simply thinking that each screen (let's take the list widget as an example) would have a set of viewports defined (per screen), and the code would just draw into those viewports. e.g.
struct list_viewports = {
struct viewport title;
struct viewport scrollbar;
struct viewport cursor;
struct viewport main;
struct viewport icons;
};
(or maybe less viewports, I know we've discussed that before)
and then the list code would define an array of these, one per screen:
static struct list_viewports list_vp[NB_SCREENS];
You could then have a function to initialise a list_viewports struct based on a display:
static void init_viewports(struct list_viewports* vp, struct screen* display);
This function could also take overall x,y,width,height values as parameters, to draw the list within that area.
And finally, the list drawing code would draw the elements into those viewports, calling display->set_viewport() appropriately.
i have noticed a few issues with this patch while making a WPS.
Attached is a screendump showing the issues and the test WPS for reproduction.
1. In the viewport only lines that actually contain text use the specified backgroundcoulour (i would have expected, that the whole viewport uses it).
2. The peakmeter inside the viewport is at the wrong position (it should appear between the lines test3 and test4) and uses the height of the user font although the viewport uses the systemfont.
3. When an image is loaded after a viewport then the image is shown in the main viewport (like expected), but the clearing of the area for the image happens in the other viewport. In the test WPS this causes that when enabling HOLD the image appears in the main viewport, but parts of the first line in the other viewport get cleared. And when disabling HOLD again, the image doesn't dissappear.
When moving the WPScode that shows the image before the %V line, then it works as expected.
H300 uisimulator r16011 with the v8 patch.
This patch also includes some optimisations to the WPS code (to reduce binsize a little), but doesn't yet address PaulJam's issues (thanks for the report).
"struct viewport title = { .x = parent->x, .y=parent->y,
.width = parent->width,
.height = display->get_font(FONT_UI)->height,
.font = FONT_UI,
.drawmode = DRMODE_DEFAULT,
....
"
for now i'll stick em in the list file, but a proper place is needed. not sure if it should go apps/viewports.c or firmware/viewports.c...
edit: the other thing which I just thought of which may turn out to be pointless is whenever the current_vp changes, the old value should be stored so we can just do lcd_viewport_prev()
edit2:
ok, http://rafb.net/p/2es7LV31.html is the completly unchecked code to draw the title area of the list. does this look about like how we want viewports used? (and I just realised I havnt updated the viewport there, but I may just do the one viewport_update() on the parent once all the drawing is done...
the drawing for the individual items will be very similar..
But (as I described in an earlier comment) I was thinking it would work differently to your patch - you've combined the initialisation of the viewports with the actual drawing. I was imagining that these would be separate - you would define all the viewports for the list centrally, and then the draw_title() function would take the viewport(s) to draw into as parameters, and do the drawing.
You also can't define viewports on the stack - the scrolling code stores a pointer to the viewport used at the time the scrolling was set up, so it needs to be statically declared, or at least at a higher-level function.
as for the viewport_copy().. you cant use = when one is a pointer... what I was trying to show is how I eventually want the general drawing functions to look, its given a parnt viewport and told to draw inside it...
http://img258.imageshack.us/my.php?image=snapshot2he6.png is an earlier version of the patch... just me showing off for irc :p
Have a play with the values of the parent viewport sturct in list.c if you want to see why I'm excited about viewports... (the code to decide what the first item to draw - and so making sure the selection is on the screen - hasnt been fixed yet, so if you change the height to be much smaller than the height of the full lcd it will go funny.. but it will still be drawn correctly. :)
I dont know how you were thinking about doing it... but what I was thinkg about is having the whole vp read in from a .vp file s only the filename needs to be stored in settings....
menu_viewport: 0,0,100,100,0,ff0000,00ffff
i.e. add a new "type" to the settings code of "viewport", and read all the values in a single line.
The code to parse this could even be merged with the WPS code that parses a %V tag - we would just need to pass the separator (, or |) as a parameter to a generic "parse_viewport" function.
do we want to get this to a commitable state asap? or can we wait a little while so I can fiddle with the list api (I've never liked it and want to change it, and I'd rather change it all once instead of twice...)
Travis, ?
is this the natural restriction of the number of viewports or just a bug?
also i think the viewports should draw their complete "rectangle" with the background colour no matter if anything is displayed at all. atm only the lines where text is are "backgrounded".
i also had a minor issue with overlapping viewports:
when having one viewport spanning a complete textline containing a left-aligned text and a right aligned text, i then made a second viewport on the same y coorinates but between both texts containing a scrolling line.
problem there was that the "middle" viewport was not displayed at all. i had to split the texts up into 3 viewports to make the line work like i wanted it to work.
and i also faught with the previously mentioned problem with conditional graphics in the background viewport clearing a complete textline beneath the graphics (including a existing albumart).
In my own custom WPS, I have the volume level aligned left and the time aligned right on either side of the album art, and it creates a white line through the picture. If I create two viewports on either side that don't invade that space, will that eliminate the line?
1) @dberg918 - Within a viewport, the WPS will behave exactly as it used to behave full-screen without viewports. So if you have a line with text left-aligned and right-aligned, and album-art displayed in the middle, then the album-art will be overwritten. As you say, you should create two viewports - one for the left text, and one for the right text.
2) Sacha - I'm not understanding what you mean by "a viewport called by a conditional". But viewports must be on a line by themselves, it's not (currently) possible to make them conditional.
I'm probably not going to get a chance to work on this patch for at least a couple of weeks, so if anyone else wants to carry on and try to get it to a committable state, please do.
%Cl|21|41|s75|s75|
%X|background.bmp|
%P|progressbar.bmp|
#
%xl|a|stop.bmp|100|163|
%xl|b|play.bmp|100|163|
%xl|c|pause.bmp|100|163|
%xl|d|ff.bmp|100|163|
%xl|e|rew.bmp|100|163|
%xl|f|vol0.bmp|63|163|
%xl|g|vol1.bmp|63|163|
%xl|h|vol2.bmp|63|163|
%xl|i|vol3.bmp|63|163|
%xl|j|vol4.bmp|63|163|
%xl|k|vol5.bmp|63|163|
%xl|l|vol6.bmp|63|163|
%xl|m|vol7.bmp|63|163|
%xl|n|vol8.bmp|63|163|
%xl|o|vol9.bmp|63|163|
%xl|p|vol10.bmp|63|163|
%xl|q|Batt1.bmp|192|5|
%xl|r|Batt2.bmp|192|5|
%xl|s|Batt3.bmp|192|5|
%xl|t|Batt4.bmp|192|5|
%xl|u|Batt5.bmp|192|5|
%xl|y|Battp.bmp|192|5|
%xl|z|Battc.bmp|192|5|
%xl|A|Hold-on.bmp|175|5|
%xl|B|Hold-off.bmp|175|5|
%xl|C|hdd.bmp|156|5|
%wd
%C
%V|10|5|100|14|1|737675|000000|
# NOTE: These conditionals all blank the current line when they update,
# so we place them all on the same line, along with some dynamic content
%?mh<%xdA|%xdB>%?bp<%xdy|>%?bc<%xdy|>%?lh<%xdC|>%?pv<%xdf|%xdg|%xdh|%xdi|%xdj|%xdk|%xdl|%xdm|%xdn|%xdo|%xdp|%xdp>%?bl<%xdz|%xdq|%xdr|%xds|%xdt|%xdu|%xdu>%?mp<%xda|%xdb|%xdc|%xdd|%xde>%t5%?mh<Battery: %bl%%|%cl:%cM %cp>;%t3%?mh<Battery: %bt|%cd/%cm/%cy>
%V|16|124|200|15|1|C8BFB5|000000|
Next Song:
%V|100|68|150|15|0|FFFFFF|000000|
%al%pc%ar%pt
%V|129|69|100|6|1|FFFFFF|000000|
%pb|6|0|59|0|
%V|100|41|150|15|1|FFFFFF|000000|
%s%ac%?ia<%ia|%?d2<%d2|Artist Unknown>>
%V|100|80|150|30|1|FFFFFF|000000|
%s%?id<%id|%?d1<%d1|Album Unknown>>
%s%?it<%it|%fn>
%V|84|124|200|15|1|FFFFFF|000000|
%s%?It<%It|%Fn>
%V|16|158|200|15|1|C8BFB5|000000|
%al%?mp<Stopped|Playing|Paused|Forward|Rewind>%ar%?mm<%?ps<%<S%>| >|%?ps<%<S+All%>|%<All%>>|%?ps<%<S+One%>|%<One%>>|%?ps<%<S+Sfl%>|%<Sfl%>>|%?ps<%<S+A-B%>|%<A-B%>>> %pp of %pe%?mm< |>
i wasnt able to resolve them, so please resync. best would be to bring the wps viewports into a commitable state.
also i hope interest in the vp implementation is not lost.
and if so i hope the devs post it so we can continue to use the "Old" methods of font-usage etc. >-)
@jg: ok, thanks for the info.
Example:
If you state '2' for 'arial.fnt' in .cfg file - all '2' stated fonts will be changed.
With 'FontName.fnt' for font statement this quick fixes will be impossible. But personal opinion full names will be easier for theme developers.
thus the font must only be changed on one position instead of multiple position (in each vp).
the more vp's we get (wps, lists, whateverwethinkofnext) the more places there will be where fonts may need to be changed.
as a theme imho will use the same or similar font for many of its vp's, the fontchange can be easily done via theme-cfg compared to changing all cfg-files of the theme.
also the lines are shorter in the wps-files and have a fixed length so you can easily compare settings.
example:
%V|16|158|200|15|1|C8BFB5|000000|
%V|84|124|200|15|2|FFFFFF|000000|
compared to
%V|16|158|200|15|nimbus-14|C8BFB5|000000|
%V|84|124|200|15|hevetica-bold-0815|FFFFFF|000000|
but on the other hand of course the fontname is more "readable" in the cfg's.
Just a thought... thanks!
Sorry...!
you can easily use switch with an integer :-)
an argument for font naming:
you can easily generate a global function mapping font-identifiers or font names to numbers for use with switch.
another question would be:
will we really want to use fontnames in wps's or vp definitions?
or will we rather use "font identifiers" indexing on the lines in the cfg-file and allow the user to customize all possible fonts via theme-setting menu?
example:
will we use "nimbus-12" and "nimbus-14" directly in the vp setting-line and the wps, or will we rather use:
user1 and user2 to index on the config-file values "userfont1: nimbus-12" and "userfont2: nimbus-14" and allow to user to freely choose all possible fonts (userfont1-15, menufont, .....).
Why not just defining the fonts used in the same way as bitmats? Something like
%Font1|[/.rockbox/fonts/]font.fnt
%Font2|[/.rockbox/fonts/]another_font.fnt
And in the viewport it would be like this:
%V|x|y|width|height|[%]Font1|fgcolor|bgcolor|
This is more logical for me than the current solution. And of course this should go hand in hand with a multifont solution.
@kugel - If I understand your suggestion, fonts will be defined in the .wps file, and not globally in the global settings (.cfg). Why? How does this go hand-in-hand with multifont, when multifont is intended to be used in the whole of Rockbox, not just the WPS?
At least, that's how I want to work on this WPS patch.
But for now I'm ignoring fonts (keeping the existing 0/1 numeric implementation) and working on the other issues.
My idea basically moves the userfont part of the multifont patch into the wps, which has more sense imo.
I just said it should go hand in hand with a multifont solution since those font definitions make only sense when one can actually choose between fonts.
we first need to get agreement on how to enable multiple fonts in the config and theme files and how to work with fonts wherever we use them.
if we agree on anything there, we can work on getting a multifont patch submitted which enables all needed functions for other parts of rockbox (loading of fonts, font usage in viewports, font caching, settings management, userUI to select the fonts etc.).
after that is commited we can work on enabling mf for wps and other screens using viewports.
maybe it would be better to take this discussion (especially the discussion how to use fonts and which functions need to be in the mf-patch to get it commited) into the forum or irc.
atm all 3 patches are a bit stalled because the discussion is not finished (mf needs wps-vp-fontsupport, list-vp needs mf, and all must wait for outstanding decissions).
WPSes with viewports can be designed not to care about font height at all. And even if they aren't, users can still use a .cfg to load an alternative set of fonts without having to directly modify the .wps, if they want to use a variant of that .wps.
It also allows a clever user to create a .wps that is mostly font independent, then use a simple .cfg to load an alternative set of fonts to make the same .wps more usable in the car (large fonts) but show less information (it would push certain optional information out of the displayed viewport).
I can see several reasons why having them in the .cfg would mean a much more flexible player overall, for me.
http://www.rockbox.org/twiki/bin/view/Main/MultiFontSupport
As much as possible, can we keep this task focused on the other aspects of WPS viewports patch - it's getting hard to find WPS-specific posts here... Discussion of how multi-font should be implemented affects much more than just the WPS.
Changes compared to viewports-wps-v1.diff:
1) Images are now displayed within the current viewport (see my jClix example for how this is working). This will break all existing WPSs written to use earlier versions of my viewports patch but I think it's the more logical approach.
2) Fix the bugs reported by PaulJam on 7 Jan 2008 (thanks for the clear report) - the test WPS posted with that bug report now displays correctly.
3) Other small bug fixes and cleanups
Still to do before I would consider committing it:
1) Clean up the parsing of the %V tag - reduce code size and ensure viewports are fully validated before using. This parser could be shared with other parts of Rockbox that will be parsing viewport structs (e.g. custom lists).
2) Decide on how fonts should be specified - see http://www.rockbox.org/twiki/bin/view/Main/MultiFontSupport
3) More testing - help needed! Please try using these new features in your WPSs and report any issues.
The normal method of conditionals won't work because of the way I've implemented viewports. Previously, the WPS consisted of a single array of lines. With my viewports patch, a WPS is now an array of viewports, with each viewport containing an array of lines.
So we need some way to specify whether each viewport should or should not be displayed - suggestions for a syntax to accomplish this are welcome.
Given that the only use case I can think of is for album art, maybe we should just hard-code something for that (e.g. %V for viewports which are always displayed, %VC for display when there is album art, and %Vc for display when no album art), but it would be nicer to have a more flexible system.
Or do we need the ability to make the co-ordinates in a viewport conditional? That would be much more complex, but I can see the usefulness...
would it make sense to define one viewport inside the wps which will be used for list-display?
so for example if i use VP5 for this "combined VP", i could define AA to show there, and as soon as i press "select" the AA is not displayed in VP5 any more but the resulting list (for example context menu) is displayed inside VP5.
if i return to the WPS from the list the AA will be displayed in VP5.
if we then manage to update the WPS information during list-display, we could easily allow menu navigation, playlist editing etc during playback inside a wps ;-)
oh well, maybe we should first get the rest commited *g*
* remember to convert the coordinates of the pictures to viewport-related coordinates :-)
* do NOT use overlapping viewports
mf patch got synced to this one, and aa-resize also works with it.
Also improves the viewport parser. Please continue testing and reporting any problems.
Still left to do:
1) Fix the checkwps tool - this won't compile due to the validation of viewports based on the device's LCD dimension/depth. So we need to add some way to specify the target we're checking for to checkwps.
2) Decide what to do about conditionals
3) Decide what to do about fonts
EDIT: The v4 patch previously attached to this comment had a line missing, please use the attached v4.1 instead.