Opened 17 years ago

Closed 17 years ago

Last modified 20 months ago

#8240 closed patch

DIG&CMI: 2byte charset support (experimental)

Reported by: SF/wonst719 Owned by: fingolfin
Priority: normal Component: Engine: SCUMM
Keywords: Cc:
Game: The Dig

Description

Korean, Japanese, Chinese(Taiwan) version of The Dig (v8.0) & COMI have external font file, which contains non-ascii (2byte) font bitmap. So, this adds external font charset support for The DIG and COMI. It should fix the bug #626302.

Ticket imported from: #747021. Ticket imported from: patches/345.

Attachments (6)

2byte_charset.cpp (4.6 KB ) - added by SF/wonst719 17 years ago.
2byte_charset.diff (14.2 KB ) - added by SF/wonst719 17 years ago.
DIG_VIDEO.ZIP (106.7 KB ) - added by SF/wonst719 17 years ago.
The DIG Korean data files
cmi_resource.ZIP (227.8 KB ) - added by SF/wonst719 17 years ago.
COMI Korean data files
2byte_charset.2.diff (16.0 KB ) - added by fingolfin 17 years ago.
rev 3
comi_kor_20051105.zip (112.0 KB ) - added by cyxx 15 years ago.

Download all attachments as: .zip

Change History (17)

by SF/wonst719, 17 years ago

Attachment: 2byte_charset.cpp added

by SF/wonst719, 17 years ago

Attachment: 2byte_charset.diff added

comment:1 by fingolfin, 17 years ago

First off, thanks for the patch!

It's a start from which we might add proper support, but the patch as it is right now is a bit too hackish for my test. In particular, it uses global variables instead of adding member variables to the appropriate classes; and the abstraction level is not chosen well (i.e. none is present, actually :-)

Some more concrete remarks: - don't use static/global vars - add memebers to the appropraite classes to store that state - your "maskidx" is the same as our existing "revBitMask" - why have get2byteCharWidth / get2byteCharHeight as seperate functions if you only call them in a single place each anyway (i.e. in getCharWidth/ get2byteCharHeight) - the various draw2byte() present unnecessary code duplication. Take as example CharsetRendererClassic::draw2byte and CharsetRendererClassic::drawBits: they are mostly the same, only in drawBits we pass in a pointer to the char data, the width, and the height; in draw2byte, you pass in the char and then from that compute the char data location, and the char height/width. The logical way to abstract hence would be to always use drawBits, but rather compute the height/width/char pointer passed in properly... (well actually you modified draw2byte, compared to drawBits: it assumes _bpp==1, and it draws each pixel twice, once in its proper position, and once shifted by 1 to the right. Not sure why you do that, though, please explain :-)

i'd rewrite the code myself but I don't have any korean data files to test with.

comment:2 by fingolfin, 17 years ago

For the NUT/Smush font renders, it seems (looking at your code) that the korean fonts are in 1bpp format (contrary to the "normal" ones which are 8bpp), probably to save space. I'd be curious to know if that information is stored somewhere in the font header (that would seem most likely to me). In that case, it should be possible to modify both classes to use a single drawChar method each, too. Although here, too, you do dst[i + 1] = 0; dst[i] = color; which seems odd to me - has that any good reason, or is that just to get a black border at the right side of the chars?

Also I wonder about the hardcoded char dimensions. Are they really hardcoded, or just stored differently in the font data files?

The clipping logic in CharsetRendererClassic::draw2byte seems to be flawed, or at least odd BTW, did you ever test that part of it?

by SF/wonst719, 17 years ago

Attachment: DIG_VIDEO.ZIP added

The DIG Korean data files

by SF/wonst719, 17 years ago

Attachment: cmi_resource.ZIP added

COMI Korean data files

comment:3 by SF/wonst719, 17 years ago

OK. I've attached korean data files. First, Yes. CJK fonts are in 1bpp. they have no header except width and height(third and fourth byte). Korean have 02 00 (512 or 2) in the first two bytes, but I don't know about it. Well. dst[i + 1] = 0; is just a drop shadow as Loom's one, because korean font files have any shadow datas. Char dimensions are hardcoded in the SPU. And I didn't test about clipping logic. maybe it's wrong.

Er... sorry. I'm poor at English...

comment:4 by fingolfin, 17 years ago

Thanks Won Star, that seems very useful. I'll look at it when I am back home.

Don't worry about your english, either, it's definitely stellar compared to my (non existent) skills in the korean language :-)

comment:5 by fingolfin, 17 years ago

I slightly cleaned up the patch (the seperate .cpp is not needed anymore). This is by far not ready to be applied, though - I'll work on this when I have time tomorrow or wednesday...

comment:6 by fingolfin, 17 years ago

Owner: set to fingolfin

comment:7 by fingolfin, 17 years ago

Updated the patch to work against latest CVS; also cleaned up charset.cpp code a little

by fingolfin, 17 years ago

Attachment: 2byte_charset.2.diff added

rev 3

comment:8 by fingolfin, 17 years ago

I put a heavily modified version of this into CVS. There are quite some more changes I want to make, but it's easier to make them incrementally in CVS instead of keeping to maintain .diff files

comment:9 by fingolfin, 17 years ago

Status: newclosed

by cyxx, 15 years ago

Attachment: comi_kor_20051105.zip added

comment:10 by cyxx, 15 years ago

With the recent subtitles/blasttexts change for v7/v8, I am not sure the characters are at the right positions compared to the original.

Wonstar, do you have the possibility to run the same scenes in the original korean version attach screenshots ? Thanks.

comment:11 by digitall, 20 months ago

Component: Engine: SCUMM
Game: The Dig
Note: See TracTickets for help on using tickets.