#8598 closed patch
Better (?) memory allocation in NUT font renderer
| Reported by: | eriktorbjorn | Owned by: | eriktorbjorn |
|---|---|---|---|
| Priority: | normal | Component: | Engine: SCUMM |
| Version: | Keywords: | ||
| Cc: | Game: | Monkey Island 3 |
Description
Recently, Fingolfin wrote a mail to scummvm-devel about ScummVM's memory footprint. One of the areas he speculated would be a good place to save memory was the NUT font renderer used by The Curse of Monkey Island, since it pre-decodes the entire font.
One possibility (suggested in the mail) would be to store the undecoded font file in memory and decode it on the fly when the font is actually rendered. Unfortunately, the total size of the decoded glyphs is, as far as I can tell, actually smaller than the entire undecoded font file.
Of course, the font file also contains the dimensions for each glyph (which we also read in advance), tags, etc.
So all this patch does is to change the memory allocation so that we'll allocate five larger memory chunks (one for each font) instead of 1280 smaller ones (one for each glyph). Surely that should save the memory allocator some bookkeeping information?
Another possible, and trivial, saving which I didn't want to mix up in this patch might be to 16-bit integers instead of plain int in NutRenderer's _chars[] array.
Ticket imported from: #1635584. Ticket imported from: patches/703.
Attachments (9)
Change History (41)
by , 17 years ago
| Attachment: | nut-allocate.diff added |
|---|
comment:1 by , 17 years ago
| Owner: | set to |
|---|
comment:2 by , 17 years ago
Yet another way to save a more substantial amount of memory would be to - where possible - decode the NUT fonts as bitmaps instead of pixmaps. Note that NutRenderer::drawChar() draws the glyph with only one colour.
Things are slightly complicated by SMUSH and Insane using the NUT renderer, directly or indirectly, to draw bitmap fonts and icons.
I've attached a proof-of-concept patch to do this. I have not tried to be elegant. I haven't tested it outside of Curse of Monkey Island. But it might provide the basis for a prettier version. File Added: nut-allocate2.diff
comment:3 by , 17 years ago
Will look at it. The (ab)use of the NUT font render code by SMUSH/INSANE isn't very nice indeed, and I'd appreciate any attempts to clean that up... :)
comment:6 by , 17 years ago
Could you please elaborate a bit on the "static (=class) methods"? That's one of the many things about C++ I'm not familiar with, I guess. (Simply changing "void codec..." to "static void codec..." in the .cpp and .h files broke compilation for me.)
Also, I thought for sure you would suggest some sub-classing or template trickery to get rid of the _bitmapFont test. Shows how much I know. :-)
comment:7 by , 17 years ago
I've attached an updated patch. Is this what you mean? Either way, the new patch also fills out some missing lines of codec1(), though I'm not sure that function is ever used for fonts that can be treated as bitmaps. File Added: nut-allocate3.diff
comment:9 by , 17 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → closed |
comment:10 by , 17 years ago
In NutRenderer::drawShadowChar(), we are drawing each character glyph 7 times, to achieve the look the original CMI text renderer had. I just committed an update to the comment in that method, *trying* to explain how the original engine did it (I hope the explanation is clear enough...).
My original plan was to modify the code, so that ScummVM would behave exactly like the original (that way we would be able to get rid of NutRenderer::drawShadowChar() among other minor cleanups). But doing so require reverting back to pixmap instead of bitmap.
So I am just wondering, does it really worth it ? Or is it better to leave the code as it is now (probably less performant, but using less memory...) ?
comment:11 by , 17 years ago
Question is, does our code produce identical results in all cases (or "identical enough" results, at least :).
If only four colors (0, 1, 255, 224) occur, we could also use a 2bit plane approach, as some kind of compromise... i.e. we only would need four values for each pixel.
comment:12 by , 17 years ago
To properly answer the question, I think we'll need to write a tool to compare the bitmaps in both scenarii. I'll try to hack one later.
NUT fonts in COMI demo version only used 2 colors (0,1) NUT fonts in COMI retail version uses 4 colors (225, 0, 1, 224). There's no 255, I made a mistake, the original decoding routine handles this case but the datafiles don't use this value.
comment:13 by , 17 years ago
Here are the logs... Looks like many char glyphs have slight differences. I am not sure if font0.nut and font4.nut are ever used, but if they are, they are the only 2 fonts to use colors 224 and 225. The remaining 3 fonts only use colors 0 and/or 1.
So it sounds possible to use a 2bits depth mask (max 2 colors + transparency). File Added: nut_fonts_diff.zip
comment:14 by , 17 years ago
(Re-opening to make it easier to find.)
What about the NUT fonts used by The Dig and Full Throttle? Are they limited to four colours, too? I don't have the time to check right now.
comment:15 by , 17 years ago
| Status: | closed → new |
|---|
comment:16 by , 17 years ago
Not sure about The Dig and FT, as far as I understand, SmushFont objects redefine their own drawing method (without re-using the existing code in NutRenderer).
FT/Insane loads 4 .nut files, but those aren't really "fonts" as it seems.
comment:17 by , 17 years ago
I don't know if it's helpful, but I've attached a patch so that instead of decoding NUT fonts as "bitmap" or "not bitmap", they're decoded using 1, 2 or 8 bits per pixel. File Added: nut-renderer.diff
by , 17 years ago
| Attachment: | nut_no_shadow_hack.diff added |
|---|
Possible patch to get rid of the shadow char hack
comment:18 by , 17 years ago
Your patch looks interesting but it doesn't seem to solve the problem the with the fact we are loosing information by predecoding the data (we can't distinguish if a pixel is transparent or if it's color is equal to 0).
It seems SmushFont::drawChar() tries to cope with that issue by adding extra checks (_original, _new_colors...). So I am not sure what is the correct way of fixing this. For reference, I am attaching the patch I did yesterday which reverts the bitmap/pixmap thing and add special check for codec 44 to handle transparency (that's quite hackish actually, but at least that shouldn't lead to regressions in the smush player).
For information in COMI, AFAICT font0 and font4 are only used during smush cutscenes and colors 224 and 225 are handled in SmushFont::drawChar. I should probably also mention that the font in the demo version of COMI is quite different from the original when rendered under ScummVM (without my patch I mean).
File Added: nut_no_shadow_hack.diff
comment:19 by , 17 years ago
I haven't had a chance to look at those patches yet, but let me just say this: My personal priority list is this: 1) Try to match the original as close as possible 2) Try to unify the SmushFont draw code with the base Nutfont code, if possible 3) Preserve memory as much as possible.
If only four colors are used, we should be able to still do 3, shouldn't we? For 2, it really would be important to understand that code better, to find out which cases occur in which of the games, etc.
comment:20 by , 17 years ago
How is transparency represented? Our current decoder seems to assume that 0 is transparency, but cyx says it's the shadow colour...
comment:21 by , 17 years ago
The current decoder code in ScummVM assumes that 0 is transparency. This seems to be correct for FT and TheDig (which use codec 1 and 21). But for CMI (codec 44), 0 is definitely the shadow color.
comment:22 by , 17 years ago
Looking at cyx's patch, I take it that 2 is the transparency colour for codec 44. I guess I was confused by the earlier statement that there were four colours: 0, 1, 224 and 225.
I assume that for most characters we could still get away with using two bits per pixel, to save memory, but it might take a bit more work. Still, allocating the font as one memory chunk and only loading fonts when they're actually used might have saved us a little too. (Though for in-game use, we may of course end up loading most or all the fonts anyway. Hmm... perhaps unload them when they haven't been used for a while?)
comment:23 by , 17 years ago
Fingolfin seemed to think it would be a pity to throw away the compression, so here's an attempt at combining the cyx's patch with the ideas from my earlier patches. The font is decoded in the usual manner and then, if possible, compressed to 1, 2 or 4 bpp.
I'm a bit confused about drawChar(), though... Don't we need to know which codec was originally used to determine which is the transparent colour?
This may take care of steps 1 and 3, but definitely still leaves step 2: "unify the SmushFont draw code with the base Nutfont code". I didn't even try -- I just added a parameter to the NUT renderer to tell it whether or not to attempt to compress the font. Ideally, that could be removed later. File Added: nut-renderer2.diff
comment:24 by , 17 years ago
Well, in theory, if there are only three colors (black, white and transparent), then why not modify the codecs to automatically convert "their" transparent color to one specific one? I haven't really looked at the code, though, hence this might be total bogus.
comment:25 by , 17 years ago
>> Looking at cyx's patch, I take it that 2 is the transparency colour for codec 44.
Actually, my patch is just a hack, I choose "2" because colors 0 and 1 are already in-use and we need to define a new color to properly handle transparency.
>> I'm a bit confused about drawChar(), though... Don't we need to know which >> codec was originally used to determine which is the transparent colour?
It seems NutRenderer::drawChar is only used when playing CMI, so every glyph rendered by it is using codec 44. If the plan is to unify SmushFont::drawChar and NutRenderer::drawChar, I also think we'll need to add a "codec" field to the NutRenderer class (it should then be possible to get rid of the _new_colors and use_original_colors flags).
comment:26 by , 17 years ago
I've made two slight changes to my patch:
1. The transparency colour is stored for each character, depending on which codec was used to unpack it.
2. I've removed most of codec1() and replaced it with a call to smush_decode_codec1(), almost like we used to do. That's one advantage of doing the decoding and re-compressing as two separate steps.
File Added: nut-renderer3.diff
comment:27 by , 17 years ago
That last patch looks pretty cool to me :)
One minor remark though : what about using the new transparency field in SmushFont::drawChar instead of hard-coding kDefaultTransparentColor and kSmush44TransparentColor ? This would be a first step in trying to write a "generic" SmushFont::drawChar method.
comment:28 by , 17 years ago
> what about using the new transparency field in SmushFont::drawChar
Sounds sensible, though I haven't actually verified that it works. Here's an updated patch which, while it doesn't unify the drawing code (that seems trickier than I had first hoped) does allow SMUSH and INSANE to use compressed fonts. File Added: nut-renderer4.diff
comment:30 by , 17 years ago
Thanks for the kind words. I have committed it, and will re-close this tracker item. (Even if we still haven't unified the NUT and SMUSH font drawing, that will have very little to do with "better memory allocation". ;-)
comment:31 by , 17 years ago
| Status: | new → closed |
|---|
comment:32 by , 6 years ago
| Component: | → Engine: SCUMM |
|---|---|
| Game: | → Monkey Island 3 |
Patch against current SVN