Opened 14 months ago

Closed 4 months ago

#10748 closed defect (fixed)

QFG4: Mirrored room Pics are off by 1 pixel, garbage on right edge

Reported by: Vhati Owned by: sluicebox
Priority: low Component: Engine: SCI
Keywords: SCI32 Cc:
Game: Quest for Glory 4

Description

ScummVM 2.1.0git3770-g15306581ab (Oct 18 2018 04:27:32)
Windows 7 64bit
QFG4 CD (English)

In the forest (from the rickety bridge that spans the stream, 1W-1S), there's a tall single-pixel-wide line that looks out of place. It paints a multi-hued green stripe on a rock and seems to extend up the side of a tree. Could be an off-by-1 mistake when a sprite was cropped?

File - 5kb MD5 - Full MD5
RESOURCE.000 - 263dce4aa34c49d3ad29bec889007b1c - 1364ba69e3c0abb68cc0170650a56692
RESOURCE.AUD - c39521bffb1d8b19a57394866184a0ca - 71098b9e97e20c8941c0e4812d5f906f
RESOURCE.MAP - aba367f2102e81782d961b14fbe3d630 - 801a04cc6aa5d437681a2dd0b6545248
RESOURCE.SFX - 3cf95e09dab8b11d675e0537e18b499a - 7c858d7253f86dab4cc6066013c5ecec

Attachments (6)

sci.005 (51.0 KB ) - added by Vhati 14 months ago.
SavedGame - Green Line
GreenLine.png (127.1 KB ) - added by Vhati 14 months ago.
Screenshot - Green Line
DOSBox-NoGreenLine.png (52.6 KB ) - added by Vhati 14 months ago.
DOSBox - No Line
Screenshot (CD) - Room 558 (OP restored).png (55.4 KB ) - added by Vhati 13 months ago.
Screenshot (CD) - Room 558 (OP restored)
Screenshot (CD) - Room 558 (Pic only).png (52.1 KB ) - added by Vhati 13 months ago.
Screenshot (CD) - Room 558 (Pic only)
Screenshot (CD) - Room 558 (deltaX+=1).png (55.3 KB ) - added by Vhati 13 months ago.
Screenshot (CD) - Room 558 (deltaX+=1)

Download all attachments as: .zip

Change History (23)

by Vhati, 14 months ago

Attachment: sci.005 added

SavedGame - Green Line

by Vhati, 14 months ago

Attachment: GreenLine.png added

Screenshot - Green Line

comment:1 by digitall, 14 months ago

@Vhati: Thanks for the bug report. Could you try testing this with the original SCI interpreter to see if this occurs in the original as well?

comment:2 by Vhati, 14 months ago

The original interpreter has no line.

comment:3 by digitall, 14 months ago

Vhati: OK. Thanks for checking.

Will try and replicate with my copy of QFG4 and see if I can work out exactly what resource reference that is exhibiting the issue.

I am not a SCI engine developer, but it may be a script bug which does not manifest on the original, but will try to get enough information for the SCI engine developers to look at this and likely add a fix or workaround.

by Vhati, 14 months ago

Attachment: DOSBox-NoGreenLine.png added

DOSBox - No Line

comment:4 by Vhati, 14 months ago

There's another such tree two screens south of town (reused sprite). Scene looks the same, but with red bushes swapped in for the background rocks.

And another (same sprite again). In an area one screen north of Baba Yaga's waterfall (SE corner of the map). A scene with those red bushes, but this time the boulder the tree rests on is cluttered by a pile of rocks and flowers.

comment:5 by digitall, 14 months ago

Have checked with my copy of QFG4-CD (DOS). I can replicate this with the attached savegame on Linux x86_64 so the bug is stable and replicable.

comment:6 by Vhati, 14 months ago

It's in the floppy edition under ScummVM, too.


work out exactly what resource reference [...] it may be a script bug

  • In the floppy edition, I brought up the console (ctrl-shift-d), used 'room' to get the room number (562).
  • Bumbled through my first time tinkering with SCI Companion, disassembled script 562.
  • Pic 410 is the base background image, including the tree trunk, except roots are conspicuously cut away to reveal green grass.
  • View 414, cell 6 is a pair of detached roots wrapping either side of the boulder.
  • The green line is where one root ought to meet the trunk when the pair is superimposed, but it doesn't, letting a sliver of that background grass through.


ScummVM 2.1.0git3770-g15306581ab (Oct 18 2018 04:27:32)
Windows 7 64bit
QFG4 Floppy 1.1a + note patch (English)

File - 5kb MD5 - Full MD5
RESOURCE.000 - f64fd6aa3977939a86ff30783dd677e1 - ff42260a665995a85aeb277ad80aac8a
RESOURCE.MAP - d10a4cc177d2091d744e2ad8c049b0ae - 3695b1b0a1d15f3d324ea9f0cc325245
RESOURCE.SFX - 3cf95e09dab8b11d675e0537e18b499a - 7c858d7253f86dab4cc6066013c5ecec

comment:7 by Vhati, 14 months ago

Repeating that procedure on my CD edition...

Room 558 (different from floppy)
Pic 410
View 414, cell 6

Will all the room numbers involving Pic 410 need to be collected?

Version 0, edited 14 months ago by Vhati (next)

comment:8 by Vhati, 13 months ago

I think the entire background pic is off by one in rooms with the glitched tree, and it's even leaving unpainted pixels!

There's a line of garbage all along the right edge of the screen. Sometimes the pixels change color as hero walks around within the room. I restored the OP savegame, and the colors are completely different.

by Vhati, 13 months ago

Screenshot (CD) - Room 558 (OP restored)

comment:9 by Vhati, 13 months ago

Unflipping the background (picture=410, style=0) removes the garbage edge.

  • send rm558 drawPic 410 0

Flipping it again (style=1024 aka 0x0400) returns the garbage edge.

  • send rm558 drawPic 410 1024

Exploring other rooms...

  • Get the room object's name (rm123).
    • vv g 2
  • Get the pic and flip state (substituting the room number).
    • vo rm??? picture
    • vo rm??? style

If a room has garbage on the edge, it was flipped.
If a room does not have garbage, it was not flipped. Flipping adds garbage to it.

Last edited 13 months ago by Vhati (previous) (diff)

comment:10 by Vhati, 13 months ago

Summary: QFG4: Green line in the forestQFG4: Flipped backgrounds are off by 1px, garbage on right edge

comment:11 by Vhati, 13 months ago

In room 558, I hid all the clutter Views and took another screenshot.
send atp1 hide
send atp2 hide
send atp3 hide
send atp4 hide
send atp5 hide
send atp6 hide

SCI Companion agrees that what remains is Pic 410, flipped of course.
With the clutter removed, the garbage line is revealed to be the full height of the Pic.

by Vhati, 13 months ago

Screenshot (CD) - Room 558 (Pic only)

comment:12 by Vhati, 13 months ago

Investigating drawPic() in the scripts led to the UpdatePlane() kernel func.


script 64917 - Plane

(method (drawPic param1 param2)
	(= picture param1)
	(if (> argc 1) (= style param2))
	(= mirrored (if (& style $0400) 1 else 0))
	(UpdatePlane self)
	(Styler doit: self style)
)



I searched the ScummVM source for "mirror".


Source: plane32.cpp - Plane::addPicInternal()

ScreenItem *screenItem = new ScreenItem(_object, celObj->_info);
screenItem->_pictureId = pictureId;
screenItem->_mirrorX = mirrorX;
screenItem->_priority = celObj->_priority;
screenItem->_fixedPriority = true;

I gingerly inverted that with "!mirrorX". and compiled.

Rooms that used to have garbage no longer did.
Rooms that used to be fine suddenly had garbage.

I reverted my changes afterward and dug deeper into how "_mirrorX" was being used.

Pics were getting treated differently, incurring garbage, just because their mirror flag was set.


Source: screen_item32.cpp - ScreenItem::calcRects()

// low resolution coordinates

# ...

if (_mirrorX != celObj._mirrorX && _celInfo.type == kCelTypePic) {
	Common::Rect temp(_insetRect);

	if (!scaleX.isOne()) {
		mulinc(temp, scaleX, Ratio());
		temp.right -= 1;
	}

	CelObjPic *celObjPic = dynamic_cast<CelObjPic *>(_celObj.get());
	if (celObjPic == nullptr) {
		error("Expected a CelObjPic");
	}
	temp.translate(celObjPic->_relativePosition.x - (originX * scaleX).toInt(), celObjPic->_relativePosition.y - (celObj._origin.y * scaleY).toInt());

	// TODO: This is weird.
	int deltaX = plane._gameRect.width() - temp.right - 1 - temp.left;

	_scaledPosition.x += deltaX;
	_screenItemRect.translate(deltaX, 0);
}

That TODO comment could be interesting.


I played around editing that and compiling.
I stuck a few a debug("%d", interestingNumber); lines in there.

This code was acting on several screenItems.
One with _screenItemRect.width()==320, height()==190. Could be the background?

The big item was getting a deltaX of -1. That made _scaledPosition.x == -1.

Weird indeed.
If you take the screen width, subtract the width of a full screen image, and subtract 1, you get -1.

scaleX.isOne() is true, so the little IF block that decrements temp.right is not happening.


After some tinkering with forced values of deltaX exclusively for that big screenItem, I realized ALL these items were elements of the Pic. The small ones had been carved out, leaving black silhouettes in the big one. Stuff hero could walk behind. Priority.

SCI Companion's Pic editor confirmed this (switching from "V"isual to "P"riority).

I inserted "deltaX += 1" unconditionally, so it'd apply to all these mirrored Pic screenItems.

That shifted the entire Pic to the right (e.g., the big item's x == 0)... got rid of the garbage... and the trunk matched up with the View of roots!

I walked to a few other rooms. They all looked fine, flipped or not.


Obviously that is not a legitimate fix.

Maybe it indicates this is the place that needs fixing?

Last edited 12 months ago by Vhati (previous) (diff)

by Vhati, 13 months ago

Screenshot (CD) - Room 558 (deltaX+=1)

comment:13 by Vhati, 12 months ago

Summary: QFG4: Flipped backgrounds are off by 1px, garbage on right edgeQFG4: Mirrored room Pics are off by 1 pixel, garbage on right edge

comment:14 by Vhati, 12 months ago

LSL6 hires had the left hallway rooms mirrored.
Garbage on the left edge. That was in its original interpreter, too. Accurate.
It had hires coordinates, which are handled in a different block than the one I quoted above for QFG4.

comment:16 by Filippos Karapetis <bluegr@…>, 4 months ago

In 68843495:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:17 by bluegr, 4 months ago

Owner: set to sluicebox
Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.