Opened 5 months ago

Closed 5 months ago

#12796 closed defect (fixed)

SCUMM: COMI: Object label line printed on the wrong spot

Reported by: AndywinXp Owned by: athrxx
Priority: normal Component: Engine: SCUMM
Version: Keywords:
Cc: Game: Monkey Island 3

Description

At the current time, object labels are printed 16 pixels below and 1 pixel to the left with respect to where they should be.

This bug exclusively affects the object line which is output when the cursor goes over an object in the room (and NOT in the inventory). Every other blastText (inventory object line, dialogues, subtitles) is placed on the correct spot of the screen.

The blastText pipeline seems to be in line with what the disasm does, but I'm wondering if this is some kind of edge case that the original interpreter treats in a custom way.

The script which handles this is global script 39.

Attachments (2)

original.bmp (900.1 KB ) - added by AndywinXp 5 months ago.
scummvm.bmp (900.1 KB ) - added by AndywinXp 5 months ago.

Download all attachments as: .zip

Change History (4)

by AndywinXp, 5 months ago

Attachment: original.bmp added

by AndywinXp, 5 months ago

Attachment: scummvm.bmp added

comment:1 by athrxx, 5 months ago

Thanks for the report. I'll clue you in what the cause of this issue is, so you can decide whether you want to fix it or not. The diagnosis is not the real problem here, the problem is the fix :-)

The original has only one text printing method for everything. Unfortunately, our ScummVM forefathers made the decision to split it up: one accurate version for the Smush movies (found in smush/smush_font.cpp) and the rest shared through the existing SCUMM <=6 code. I fixed up the smush font code last year to make it pixel perfect (of course I am not going to make any promises, but all my tests were good :-). It just so happens that my fixes included the code for exactly what we have here: Just look at that:

https://github.com/scummvm/scummvm/blob/master/engines/scumm/smush/smush_font.cpp#L343

This would lift up the text exactly 16 pixels...

COMI uses a 10, 10, 630, 470 clip rect:

https://github.com/scummvm/scummvm/blob/master/engines/scumm/smush/smush_player.cpp#L649

With the horizontally wrong pixel it's the same thing, really. If you look at the rest of that function SmushFont::drawStringWrap() you can see that it has unique methods to measure the horizontal and vertical dimensions of the whole text block. Sticking to exactly that method ensures the correct placement.

How to fix this?
For the vertical fix it could be hacked into here:

https://github.com/scummvm/scummvm/blob/master/engines/scumm/string.cpp#L152

As you can see there are other hacks already there for CJK texts which I actually took from disasm. So this is apparently the place where even the original developers saw fit to place such hacks.

The issue with a possible fix is that we're missing all the width and height measuring logic from smush font (which would have been correct to have for all SCUMM7/8 texts).
If it is safe to assume that the COMI object descriptions are the only case we're treating here then we could probably live without the wrapping and height measuring logic and just make an per-case fix.

With the horizontally off-by-one thing, that's difficult without the correct measuring.

Btw., I think the problem you're trying to solve in your PR is kind of the same thing, too.

Please let us know whether you want to try to make a fix or whether we should do it ourselves...

Last edited 5 months ago by athrxx (previous) (diff)

comment:2 by athrxx, 5 months ago

Owner: set to athrxx
Resolution: fixed
Status: newclosed

I have fixed the vertical positioning with a small hack.
It does not fix the fact that we're missing the entire measuring logic, but I don't feel that needs to happen before the upcoming release.
Also, it doesn't do anything about the horizontal off-by-1-pixel glitch. I assume this would be aligned properly, if we had the correct measuring (the correct handling of every ' ', '\n' or '\r' matters here). But this is at least less visible than the wrong vertical positioning.

I close this for now...

Note: See TracTickets for help on using tickets.