Opened 16 years ago

Closed 16 years ago

Last modified 7 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 16 years ago.
2byte_charset.diff (14.2 KB) - added by SF/wonst719 16 years ago.
DIG_VIDEO.ZIP (106.7 KB) - added by SF/wonst719 16 years ago.
The DIG Korean data files
cmi_resource.ZIP (227.8 KB) - added by SF/wonst719 16 years ago.
COMI Korean data files
2byte_charset.2.diff (16.0 KB) - added by fingolfin 16 years ago.
rev 3
comi_kor_20051105.zip (112.0 KB) - added by cyxx 14 years ago.

Download all attachments as: .zip

Change History (17)

Changed 16 years ago by SF/wonst719

Attachment: 2byte_charset.cpp added

Changed 16 years ago by SF/wonst719

Attachment: 2byte_charset.diff added

comment:1 Changed 16 years ago by fingolfin

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 Changed 16 years ago by fingolfin

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?

Changed 16 years ago by SF/wonst719

Attachment: DIG_VIDEO.ZIP added

The DIG Korean data files

Changed 16 years ago by SF/wonst719

Attachment: cmi_resource.ZIP added

COMI Korean data files

comment:3 Changed 16 years ago by SF/wonst719

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 Changed 16 years ago by fingolfin

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 Changed 16 years ago by fingolfin

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 Changed 16 years ago by fingolfin

Owner: set to fingolfin

comment:7 Changed 16 years ago by fingolfin

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

Changed 16 years ago by fingolfin

Attachment: 2byte_charset.2.diff added

rev 3

comment:8 Changed 16 years ago by fingolfin

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 Changed 16 years ago by fingolfin

Status: newclosed

Changed 14 years ago by cyxx

Attachment: comi_kor_20051105.zip added

comment:10 Changed 14 years ago by cyxx

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 Changed 7 months ago by digitall

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