diff options
author | Colin Snover | 2017-10-05 21:41:29 -0500 |
---|---|---|
committer | Colin Snover | 2017-10-06 22:10:50 -0500 |
commit | ff3503abdee09fcd35d86297dfb79879d0ecc8b8 (patch) | |
tree | 4ff273c6d75ca8c988c40eb8565851cd369fbc16 /engines/sci | |
parent | 0ac5d84062e430dff0f102788015dbb609fcdaa0 (diff) | |
download | scummvm-rg350-ff3503abdee09fcd35d86297dfb79879d0ecc8b8.tar.gz scummvm-rg350-ff3503abdee09fcd35d86297dfb79879d0ecc8b8.tar.bz2 scummvm-rg350-ff3503abdee09fcd35d86297dfb79879d0ecc8b8.zip |
SCI32: Clean up GfxTransitions32
* Use containers where appropriate
* Re-wrap doxygen comments to 80 columns
* Clarify comments for parts of the engine that are understood now
but were not understood at the time of the initial
implementation
Diffstat (limited to 'engines/sci')
-rw-r--r-- | engines/sci/engine/kgraphics32.cpp | 17 | ||||
-rw-r--r-- | engines/sci/graphics/transitions32.cpp | 56 | ||||
-rw-r--r-- | engines/sci/graphics/transitions32.h | 229 |
3 files changed, 125 insertions, 177 deletions
diff --git a/engines/sci/engine/kgraphics32.cpp b/engines/sci/engine/kgraphics32.cpp index b7fae2e05d..d093c553e6 100644 --- a/engines/sci/engine/kgraphics32.cpp +++ b/engines/sci/engine/kgraphics32.cpp @@ -368,10 +368,10 @@ reg_t kSetShowStyle(EngineState *s, int argc, reg_t *argv) { const uint16 type = argv[0].toUint16(); reg_t planeObj = argv[1]; int16 seconds = argv[2].toSint16(); - // NOTE: This value seems to indicate whether the transition is an - // “exit” transition (0) or an “enter” transition (-1) for fade - // transitions. For other types of transitions, it indicates a palette - // index value to use when filling the screen. + // This value indicates whether the transition is an "exit" transition (0) + // or an "enter" transition (-1) for fade transitions. For other types of + // transitions, it indicates a palette index value to use when filling the + // screen. int16 back = argv[3].toSint16(); int16 priority = argv[4].toSint16(); int16 animate = argv[5].toSint16(); @@ -404,9 +404,8 @@ reg_t kSetShowStyle(EngineState *s, int argc, reg_t *argv) { error("Illegal show style %d for plane %04x:%04x", type, PRINT_REG(planeObj)); } - // NOTE: The order of planeObj and showStyle are reversed - // because this is how SCI3 called the corresponding method - // on the KernelMgr + // The order of planeObj and showStyle are reversed because this is how + // SSCI3 called the corresponding method on the KernelMgr g_sci->_gfxTransitions32->kernelSetShowStyle(argc, planeObj, (ShowStyleType)type, seconds, back, priority, animate, refFrame, pFadeArray, divisions, blackScreen); return s->r_acc; @@ -881,8 +880,8 @@ reg_t kSetScroll(EngineState *s, int argc, reg_t *argv) { const int16 deltaY = argv[2].toSint16(); const GuiResourceId pictureId = argv[3].toUint16(); const bool animate = argv[4].toUint16(); - // NOTE: speed was accepted as an argument, but then never actually used - // const int16 speed = argc > 5 ? (bool)argv[5].toSint16() : -1; + // argv[5] was some speed argument, but it was not actually used by SSCI, so + // we ignore it here const bool mirrorX = argc > 6 ? (bool)argv[6].toUint16() : false; g_sci->_gfxTransitions32->kernelSetScroll(plane, deltaX, deltaY, pictureId, animate, mirrorX); diff --git a/engines/sci/graphics/transitions32.cpp b/engines/sci/graphics/transitions32.cpp index df9c2da82b..8be9e63965 100644 --- a/engines/sci/graphics/transitions32.cpp +++ b/engines/sci/graphics/transitions32.cpp @@ -126,7 +126,7 @@ void GfxTransitions32::processShowStyles() { g_sci->_gfxFrameout->frameOut(true); throttle(); } - } while(continueProcessing && doFrameOut); + } while (continueProcessing && doFrameOut); } void GfxTransitions32::processEffects(PlaneShowStyle &showStyle) { @@ -249,14 +249,13 @@ void GfxTransitions32::kernelSetShowStyle(const uint16 argc, const reg_t planeOb if (createNewEntry) { entry = new PlaneShowStyle; - // NOTE: SCI2.1 engine tests if allocation returned a null pointer - // but then only avoids setting currentStep if this is so. Since - // this is a nonsensical approach, we do not do that here + // SSCI2.1 tests if allocation returned a null pointer but then only + // avoids setting currentStep if this is so. Since this nonsensical, we + // do not do that here entry->currentStep = 0; entry->processed = false; entry->divisions = hasDivisions ? divisions : _defaultDivisions[type]; entry->plane = planeObj; - entry->fadeColorRangesCount = 0; if (getSciVersion() < SCI_VERSION_2_1_MIDDLE) { // for pixel dissolve @@ -267,32 +266,26 @@ void GfxTransitions32::kernelSetShowStyle(const uint16 argc, const reg_t planeOb entry->screenItems.clear(); entry->width = plane->_gameRect.width(); entry->height = plane->_gameRect.height(); - } else { - entry->fadeColorRanges = nullptr; - if (hasFadeArray) { - // NOTE: SCI2.1mid engine does no check to verify that an array is - // successfully retrieved, and SegMan will cause a fatal error - // if we try to use a memory segment that is not an array - SciArray &table = *_segMan->lookupArray(pFadeArray); - - uint32 rangeCount = table.size(); - entry->fadeColorRangesCount = rangeCount; - - // NOTE: SCI engine code always allocates memory even if the range - // table has no entries, but this does not really make sense, so - // we avoid the allocation call in this case - if (rangeCount > 0) { - entry->fadeColorRanges = new uint16[rangeCount]; - for (size_t i = 0; i < rangeCount; ++i) { - entry->fadeColorRanges[i] = table.getAsInt16(i); - } + } else if (hasFadeArray) { + // SSCI2.1mid does no check to verify that an array is successfully + // retrieved + SciArray &table = *_segMan->lookupArray(pFadeArray); + + const uint32 rangeCount = table.size(); + + // SSCI always allocates memory even if the range table has no + // entries, but this does not really make sense, so we avoid the + // allocation call in this case + if (rangeCount > 0) { + entry->fadeColorRanges.reserve(rangeCount); + for (uint32 i = 0; i < rangeCount; ++i) { + entry->fadeColorRanges.push_back(table.getAsInt16(i)); } } } } - // NOTE: The original engine had no nullptr check and would just crash - // if it got to here + // SSCI had no nullptr check and would just crash if it got to here if (entry == nullptr) { error("Cannot edit non-existing ShowStyle entry"); } @@ -397,10 +390,9 @@ ShowStyleList::iterator GfxTransitions32::deleteShowStyle(const ShowStyleList::i break; case kShowStyleFadeIn: case kShowStyleFadeOut: - if (getSciVersion() > SCI_VERSION_2_1_EARLY && showStyle->fadeColorRangesCount > 0) { - delete[] showStyle->fadeColorRanges; - } - break; + // SSCI manually allocated the color ranges for fades and deleted that + // memory here, but we use a container so there is no extra cleanup + // needed case kShowStyleNone: case kShowStyleMorph: case kShowStyleHShutterIn: @@ -944,8 +936,8 @@ bool GfxTransitions32::processFade(const int8 direction, PlaneShowStyle &showSty percent *= 100; percent /= showStyle.divisions - 1; - if (showStyle.fadeColorRangesCount > 0) { - for (int i = 0, len = showStyle.fadeColorRangesCount; i < len; i += 2) { + if (showStyle.fadeColorRanges.size()) { + for (uint i = 0, len = showStyle.fadeColorRanges.size(); i < len; i += 2) { g_sci->_gfxPalette32->setFade(percent, showStyle.fadeColorRanges[i], showStyle.fadeColorRanges[i + 1]); } } else { diff --git a/engines/sci/graphics/transitions32.h b/engines/sci/graphics/transitions32.h index c4c52283d1..4dbacf41b9 100644 --- a/engines/sci/graphics/transitions32.h +++ b/engines/sci/graphics/transitions32.h @@ -28,7 +28,7 @@ #include "sci/engine/vm_types.h" namespace Sci { -enum ShowStyleType /* : uint8 */ { +enum ShowStyleType { kShowStyleNone = 0, kShowStyleHShutterOut = 1, kShowStyleHShutterIn = 2, @@ -48,14 +48,12 @@ enum ShowStyleType /* : uint8 */ { }; /** - * Show styles represent transitions applied to draw planes. - * One show style per plane can be active at a time. + * A show style represents a transition applied to a Plane. One show style per + * plane can be active at a time. */ struct PlaneShowStyle { /** - * The ID of the plane this show style belongs to. - * In SCI2.1mid (at least SQ6), per-plane transitions - * were removed and a single plane ID is used. + * The ID of the plane this transition applies to. */ reg_t plane; @@ -65,9 +63,8 @@ struct PlaneShowStyle { ShowStyleType type; /** - * When true, the show style is an entry transition - * to a new room. When false, it is an exit - * transition away from an old room. + * When true, the show style is an entry transition to a new room. When + * false, it is an exit transition away from an old room. */ bool fadeUp; @@ -77,38 +74,35 @@ struct PlaneShowStyle { int16 divisions; /** - * The color used by transitions that draw CelObjColor - * screen items. -1 for transitions that do not draw - * screen items. + * The color used by transitions that draw CelObjColor screen items. -1 for + * transitions that do not draw screen items. */ int16 color; - // TODO: Probably uint32 - // TODO: This field probably should be used in order to - // provide time-accurate processing of show styles. In the - // actual SCI engine (at least 2–2.1mid) it appears that - // style transitions are drawn “as fast as possible”, one - // step per loop, even though this delay field exists + /** + * The amount of time, in ticks, between each cycle of the animation. + */ int delay; - // TODO: Probably bool, but never seems to be true? + /** + * If true, GfxTransitions32 will yield back to the main game loop after + * calculating the next frame. Otherwise, GfxTransitions32 takes exclusive + * control over the game loop until the transition has completed. + */ bool animate; /** - * The wall time at which the next step of the animation - * should execute. + * The time at which the next step of the animation should execute. */ uint32 nextTick; /** - * During playback of the show style, the current step - * (out of divisions). + * During playback of the show style, the current step (out of `divisions`). */ int currentStep; /** - * Whether or not this style has finished running and - * is ready for disposal. + * Whether or not this style has finished running and is ready for disposal. */ bool processed; @@ -117,32 +111,30 @@ struct PlaneShowStyle { // /** - * A list of screen items, each representing one - * block of a wipe transition. + * A list of screen items, each representing one block of a wipe transition. + * These screen items are owned by GfxFrameout. */ Common::Array<ScreenItem *> screenItems; /** - * For wipe transitions, the number of edges with a - * moving wipe (1, 2, or 4). + * For wipe transitions, the number of edges with a moving wipe (1, 2, or + * 4). */ uint8 numEdges; /** - * The dimensions of the plane, in game script - * coordinates. + * The dimensions of the plane, in game script coordinates. */ int16 width, height; /** - * For pixel dissolve transitions, the screen item - * used to render the transition. + * For pixel dissolve transitions, the screen item used to render the + * transition. This screen item is owned by GfxFrameout. */ ScreenItem *bitmapScreenItem; /** - * For pixel dissolve transitions, the bitmap used - * to render the transition. + * For pixel dissolve transitions, the bitmap used to render the transition. */ reg_t bitmap; @@ -152,15 +144,13 @@ struct PlaneShowStyle { uint32 dissolveMask; /** - * The first pixel that was dissolved in a pixel - * dissolve transition. + * The first pixel that was dissolved in a pixel dissolve transition. */ uint32 firstPixel; /** - * The last pixel that was dissolved. Once all - * pixels have been dissolved, `pixel` will once - * again equal `firstPixel`. + * The last pixel that was dissolved. Once all pixels have been dissolved, + * `pixel` will once again equal `firstPixel`. */ uint32 pixel; @@ -169,21 +159,15 @@ struct PlaneShowStyle { // /** - * The number of entries in the fadeColorRanges array. - */ - uint8 fadeColorRangesCount; - - /** - * A pointer to an dynamically sized array of palette - * indexes, in the order [ fromColor, toColor, ... ]. + * An array of palette indexes, in the order [ fromColor, toColor, ... ]. * Only colors within this range are transitioned. */ - uint16 *fadeColorRanges; + Common::Array<uint16> fadeColorRanges; }; /** - * PlaneScroll describes a transition between two different - * pictures within a single plane. + * PlaneScroll describes a transition between two different pictures within a + * single plane. */ struct PlaneScroll { /** @@ -197,28 +181,26 @@ struct PlaneScroll { int16 x, y; /** - * The distance that should be scrolled. Only one of - * `deltaX` or `deltaY` may be set. + * The distance that should be scrolled. Only one of `deltaX` or `deltaY` + * may be set. */ int16 deltaX, deltaY; /** - * The pic that should be created and scrolled into - * view inside the plane. + * The pic that should be created and scrolled into view inside the plane. */ GuiResourceId newPictureId; /** - * The picture that should be scrolled out of view - * and deleted from the plane. + * The picture that should be scrolled out of view and deleted from the + * plane. */ GuiResourceId oldPictureId; /** - * If true, the scroll animation is interleaved - * with other updates to the graphics. If false, - * the scroll will be exclusively animated until - * it is finished. + * If true, GfxTransitions32 will yield back to the main game loop after + * calculating the next frame. Otherwise, GfxTransitions32 takes exclusive + * control over the game loop until the transition has completed. */ bool animate; @@ -235,6 +217,7 @@ class GfxTransitions32 { public: GfxTransitions32(SegManager *_segMan); ~GfxTransitions32(); + private: SegManager *_segMan; @@ -264,209 +247,185 @@ public: inline bool hasShowStyles() const { return !_showStyles.empty(); } /** - * Processes all active show styles in a loop - * until they are finished. + * Processes all active show styles in a loop until they are finished. */ void processShowStyles(); /** - * Processes show styles that are applied - * through `GfxFrameout::palMorphFrameOut`. + * Processes show styles that are applied through + * `GfxFrameout::palMorphFrameOut`. */ void processEffects(PlaneShowStyle &showStyle); - // NOTE: This signature is taken from SCI3 Phantasmagoria 2 - // and is valid for all implementations of SCI32 void kernelSetShowStyle(const uint16 argc, const reg_t planeObj, const ShowStyleType type, const int16 seconds, const int16 direction, const int16 priority, const int16 animate, const int16 frameOutNow, reg_t pFadeArray, int16 divisions, const int16 blackScreen); /** - * Sets the range that will be used by - * `GfxFrameout::palMorphFrameOut` to alter - * palette entries. + * Sets the range that will be used by `GfxFrameout::palMorphFrameOut` to + * alter palette entries. */ void kernelSetPalStyleRange(const uint8 fromColor, const uint8 toColor); /** - * A map of palette entries that can be morphed - * by the Morph show style. + * A map of palette entries that can be morphed by the Morph show style. */ int8 _styleRanges[256]; private: /** - * Default sequence values for pixel dissolve - * transition bit masks. + * Default sequence values for pixel dissolve transition bit masks. */ int *_dissolveSequenceSeeds; /** - * Default values for `PlaneShowStyle::divisions` - * for the current SCI version. + * Default values for `PlaneShowStyle::divisions` for the current SCI + * version. */ int16 *_defaultDivisions; /** - * The list of PlaneShowStyles that are - * currently active. + * The list of PlaneShowStyles that are currently active. */ ShowStyleList _showStyles; /** - * Finds a show style that applies to the given - * plane. + * Finds a show style that applies to the given plane. */ PlaneShowStyle *findShowStyleForPlane(const reg_t planeObj); /** - * Finds the iterator for a show style that - * applies to the given plane. + * Finds the iterator for a show style that applies to the given plane. */ ShowStyleList::iterator findIteratorForPlane(const reg_t planeObj); /** - * Deletes the given PlaneShowStyle and returns - * the next PlaneShowStyle from the list of - * styles. + * Deletes the given PlaneShowStyle and returns the next PlaneShowStyle from + * the list of styles. */ ShowStyleList::iterator deleteShowStyle(const ShowStyleList::iterator &showStyle); /** - * Initializes the given PlaneShowStyle for a - * horizontal wipe effect for SCI2 to 2.1early. + * Initializes the given PlaneShowStyle for a horizontal wipe effect for + * SCI2 to 2.1early. */ void configure21EarlyHorizontalWipe(PlaneShowStyle &showStyle, const int16 priority); /** - * Initializes the given PlaneShowStyle for a - * horizontal shutter effect for SCI2 to 2.1early. + * Initializes the given PlaneShowStyle for a horizontal shutter effect for + * SCI2 to 2.1early. */ void configure21EarlyHorizontalShutter(PlaneShowStyle &showStyle, const int16 priority); /** - * Initializes the given PlaneShowStyle for an - * iris effect for SCI2 to 2.1early. + * Initializes the given PlaneShowStyle for an iris effect for SCI2 to + * 2.1early. */ void configure21EarlyIris(PlaneShowStyle &showStyle, const int16 priority); /** - * Initializes the given PlaneShowStyle for a - * pixel dissolve effect for SCI2 to 2.1early. + * Initializes the given PlaneShowStyle for a pixel dissolve effect for SCI2 + * to 2.1early. */ void configure21EarlyDissolve(PlaneShowStyle &showStyle, const int16 priority, const Common::Rect &gameRect); /** - * Processes one tick of the given - * PlaneShowStyle. + * Processes one tick of the given PlaneShowStyle. */ bool processShowStyle(PlaneShowStyle &showStyle, uint32 now); /** - * Performs an instant transition between two - * rooms. + * Performs an instant transition between two rooms. */ bool processNone(PlaneShowStyle &showStyle); /** - * Performs a transition that renders into a room - * with a horizontal shutter effect. + * Performs a transition that renders into a room with a horizontal shutter + * effect. */ bool processHShutterOut(PlaneShowStyle &showStyle); /** - * Performs a transition that renders to black - * with a horizontal shutter effect. + * Performs a transition that renders to black with a horizontal shutter + * effect. */ void processHShutterIn(const PlaneShowStyle &showStyle); /** - * Performs a transition that renders into a room - * with a vertical shutter effect. + * Performs a transition that renders into a room with a vertical shutter + * effect. */ void processVShutterOut(PlaneShowStyle &showStyle); /** - * Performs a transition that renders to black - * with a vertical shutter effect. + * Performs a transition that renders to black with a vertical shutter + * effect. */ void processVShutterIn(PlaneShowStyle &showStyle); /** - * Performs a transition that renders into a room - * with a wipe to the left. + * Performs a transition that renders into a room with a wipe to the left. */ void processWipeLeft(PlaneShowStyle &showStyle); /** - * Performs a transition that renders to black - * with a wipe to the right. + * Performs a transition that renders to black with a wipe to the right. */ void processWipeRight(PlaneShowStyle &showStyle); /** - * Performs a transition that renders into a room - * with a wipe upwards. + * Performs a transition that renders into a room with a wipe upwards. */ void processWipeUp(PlaneShowStyle &showStyle); /** - * Performs a transition that renders to black - * with a wipe downwards. + * Performs a transition that renders to black with a wipe downwards. */ void processWipeDown(PlaneShowStyle &showStyle); /** - * Performs a transition that renders into a room - * with an iris effect. + * Performs a transition that renders into a room with an iris effect. */ bool processIrisOut(PlaneShowStyle &showStyle); /** - * Performs a transition that renders to black - * with an iris effect. + * Performs a transition that renders to black with an iris effect. */ bool processIrisIn(PlaneShowStyle &showStyle); /** - * Performs a transition that renders between - * rooms using a block dissolve effect. + * Performs a transition that renders between rooms using a block dissolve + * effect. */ void processDissolveNoMorph(PlaneShowStyle &showStyle); /** - * Performs a transition that renders between - * rooms with a pixel dissolve effect. + * Performs a transition that renders between rooms with a pixel dissolve + * effect. */ bool processPixelDissolve(PlaneShowStyle &showStyle); /** - * SCI2 to 2.1early implementation of pixel - * dissolve. + * SCI2 to 2.1early implementation of pixel dissolve. */ bool processPixelDissolve21Early(PlaneShowStyle &showStyle); /** - * SCI2.1mid and later implementation of - * pixel dissolve. + * SCI2.1mid and later implementation of pixel dissolve. */ bool processPixelDissolve21Mid(const PlaneShowStyle &showStyle); /** - * Performs a transition that fades to black - * between rooms. + * Performs a transition that fades to black between rooms. */ bool processFade(const int8 direction, PlaneShowStyle &showStyle); /** - * Morph transition calls back into the - * transition system's `processEffects` - * method, which then applies transitions - * other than None, Fade, or Morph. + * Morph transition calls back into the transition system's `processEffects` + * method, which then applies transitions other than None, Fade, or Morph. */ bool processMorph(PlaneShowStyle &showStyle); /** - * Performs a generic transition for any of - * the wipe/shutter/iris effects. + * Performs a generic transition for any of the wipe/shutter/iris effects. */ bool processWipe(const int8 direction, PlaneShowStyle &showStyle); @@ -476,8 +435,7 @@ public: inline bool hasScrolls() const { return !_scrolls.empty(); } /** - * Processes all active plane scrolls - * in a loop until they are finished. + * Processes all active plane scrolls in a loop until they are finished. */ void processScrolls(); @@ -490,8 +448,7 @@ private: ScrollList _scrolls; /** - * Performs a scroll of the content of - * a plane. + * Performs a scroll of the content of a plane. */ bool processScroll(PlaneScroll &scroll); }; |