[Scummvm-tracker] [ScummVM] #10748: QFG4: Flipped backgrounds are off by 1px, garbage on right edge

[Vhati]

#10748: QFG4: Flipped backgrounds are off by 1px, garbage on right edge
--------------------------------+-------------------------
  Reporter:  Vhati              |      Owner:  (none)
      Type:  defect             |     Status:  new
  Priority:  low                |  Component:  Engine: SCI
Resolution:                     |   Keywords:  SCI32
      Game:  Quest for Glory 4  |
--------------------------------+-------------------------

Comment (by Vhati):

 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".
 \\
 \\
 [https://github.com/scummvm/scummvm/blob/cfb46da90cb045e029153848652bbd166a5c95b6/engines/sci/graphics/plane32.cpp#L181
 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.
 \\
 \\
 [https://github.com/scummvm/scummvm/blob/cfb46da90cb045e029153848652bbd166a5c95b6/engines/sci/graphics/screen_item32.cpp#L413
 Source]: screen_item32.cpp - ScreenItem::calcRects()
 {{{
 // low resolution coordinates

 # [...]

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

 # [...]

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

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

 That 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.

 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?

--
Ticket URL: <https://bugs.scummvm.org/ticket/10748#comment:12>
ScummVM <https://bugs.scummvm.org>
ScummVM