[Scummvm-tracker] [ScummVM] #9857: SCI: GK2: Character Graphic Glitches in Opera basement

[Colin Snover]

#9857: SCI: GK2: Character Graphic Glitches in Opera basement
-------------------------------+-------------------------
  Reporter:  dafioram          |      Owner:  csnover
      Type:  defect            |     Status:  new
  Priority:  low               |  Component:  Engine: SCI
Resolution:                    |   Keywords:  sci32
      Game:  Gabriel Knight 2  |
-------------------------------+-------------------------

Comment (by csnover):

 So, here is a write-up of things so far:

 SCI engine uses versioning to manage palette updates. This is generally an
 optimisation to avoid unnecessary palette updates when a palette has
 already been merged into the main (source) palette.

 There are two ways that palettes make it into GfxPalette32. The first way
 is by an explicit call by a game script to `kPalette` to load a specific
 palette resource. The second way is implicit: during rendering, when a
 screen item ends up in the draw list, its cel’s baked-in palette is
 submitted by `GfxFrameout::frameOut` or `palMorphFrameOut` to the palette
 manager.

 In both cases, the palette data contains a version field that is updated
 by GfxPalette32 to record the last version that the given palette was
 submitted. If the current version in GfxPalette32 is the same as the
 version in the palette data, the palette manager skips applying the
 palette.

 If the version in the palette data does ''not'' match the version of
 GfxPalette32, the palette is merged into the source palette, and if this
 causes a change to the colours in the source palette, the
 `GfxPalette32::_needsUpdate` flag is set, and the GfxPalette32 version is
 incremented. Once the `_needsUpdate` flag is set, all palettes with a
 different version are merged, but the GfxPalette32 version is not updated
 again until the `_needsUpdate` flag is cleared (by a call to
 `GfxPalette32::updateForFrame` or `updateFFrame`).

 What seems to be happening here is complicated and seems to revolve around
 explicit calls to submit palette 252.

 When loading the attached save 86 and walking up the stairs, the order of
 operation is:

 * The game plays the VMD of walking up the stairs. This submits palettes
 for the VMD + view 126 (the background for the bar at the top of the
 screen with the room name & score), and both are applied successfully.
 (version is now 6)
 * After the VMD ends, palette 252 is submitted by a game script, and is
 also applied successfully. (version is now 7)
 * The game plays the VMD of the man talking about the show starting. This
 submits palettes for the VMD + view 126, again applied successfully, but
 version remains 7 because nothing has cleared the `_needsUpdate` flag.
 * After the VMD, palette 252 is submitted again, but since GfxPalette32
 says we are still at version 7, it is not applied. This breaks Grace's
 palette, which comes from palette 252.

 If we mess with GfxPalette32 by calling `updateForFrame` to clear the
 `_needsUpdate` flag before calling `frameOut` in `VMDPlayer::renderFrame`,
 when `frameOut` submits all the draw list palettes, this will
 coincidentally trigger a version update when view 126’s palette is
 submitted, and then when palette 252 is submitted again later, it will be
 updated to the new version. But this seems to rely entirely on the
 coincidence that there is a cel waiting to render when `frameOut` is
 called that causes a change to the colours in the source palette.

 I am not sure yet what is the correct way to ensure this all works
 properly. It is possible that SSCI relied on this coincidental situation
 with the draw list but I am still looking to figure it out. I have tried
 changing the ScummVM implementation to put the palette from the VMD
 decoder into the SciBitmap that is used to render the video (so it is
 processed by the call to `submitPalette` in `frameOut`, instead of being
 submitted directly), but this does not seem to change the situation when
 it comes times to resubmit palette 252.

 As I write this description, I realise that it does, however, seem to
 provide a basis for guaranteeing that at least one palette will be
 submitted through the draw list to cause a version update: the VMD’s own
 screen item. So it may actually just be the case that the implementation
 has to be updated to do this, but also with a call to `updateForFrame`
 before `frameOut` in `VMDPlayer::renderFrame` so that the VMD’s palette
 update does not get batched up with any earlier set of palette changes.

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